Code review - Parsing Function

Chris Kelleher

Administrator
Staff member
This function takes a string (in this case, a comma delimited string)
and strips out the commas. I has as an input parameter what the
delimiter is and if you want the delimiter to be replaced with blanks.

Is this a good approach for a function? What are the different ways to
do this?

What (if any) is the performance hit on using functions. This one would
most likey be would be external and run from a persistent procedure.

<BLOCKQUOTE><font size="1" face="Arial, Verdana">code:</font><HR><pre>
/**********************************************************************************\
Function: ParseString

Desc: This function receives, as input, a char string (string
being parsed),
the string delimiter and whether or not you want spaces to
replace
the delimiter. It then returns a new string minus the
delimiters.
For instance, DISPLAY ParseString(cWord,",",1) will result
in

"This is my string."

cWord was "This,is,my,string."

Mods: 11/28/99 JAM Created
\**********************************************************************************/

FUNCTION ParseString RETURNS CHARACTER
(INPUT cString AS CHAR,
INPUT cRemoveChar AS CHAR,
INPUT iAddSpace AS INT).

DEF VAR cParsedString AS CHAR NO-UNDO.
DEF VAR i AS INT NO-UNDO.

CASE iAddSpace:
WHEN 1 THEN /* Add space between words */

DO i = 1 TO NUM-ENTRIES(cString):
ASSIGN
cParsedString = IF cParsedString = "" THEN
ENTRY(i,cString,cRemoveChar)
ELSE cParsedString + " " +
ENTRY(i,cString,cRemoveChar).
END.
OTHERWISE /* No space added */

DO i = 1 TO NUM-ENTRIES(cString):
ASSIGN
cParsedString = IF cParsedString = "" THEN
ENTRY(i,cString)
ELSE cParsedString +
ENTRY(i,cString).
END.
END CASE.

RETURN cParsedString.
END FUNCTION. /* ParseString */

DISP ParseString(DBPARAM(1),",",1) FORM "X(55)".

[/code]
 

Chris Kelleher

Administrator
Staff member
From: Jay Martin <martine1@earthlink.net>


> Is this a good approach for a function? What are the different ways to
> do this?

This is the general idea. However, in this particular case, PROGRESS has the
following built-in function:

REPLACE (source-string, from-string, to-string)

> For instance, DISPLAY ParseString(cWord,",",1) will result in
>
> "This is my string."

With a default format of "x(8)", DISPLAY will show only "This is ".

> cWord was "This,is,my,string."

Straight DISPLAY REPLACE (cWord, ",", " ") would do the same thing.

> FUNCTION ParseString RETURNS CHARACTER
> (INPUT cString AS CHAR,
> INPUT cRemoveChar AS CHAR,
> INPUT iAddSpace AS INT).

Instead of INPUT iAddSpace AS INT, it might be more useful
to have INPUT iAddString AS CHAR
that way you could replace a "," with "whatever"
where "whatever" could be "" or a single character or multiple characters

> DEF VAR cParsedString AS CHAR NO-UNDO.
> DEF VAR i AS INT NO-UNDO.
>
> CASE iAddSpace:
> WHEN 1 THEN /* Add space between words */
>
> DO i = 1 TO NUM-ENTRIES(cString):
> ASSIGN
> cParsedString = IF cParsedString = "" THEN
> ENTRY(i,cString,cRemoveChar)
> ELSE cParsedString + " " +
> ENTRY(i,cString,cRemoveChar).
> END.
> OTHERWISE /* No space added */
>
> DO i = 1 TO NUM-ENTRIES(cString):
> ASSIGN
> cParsedString = IF cParsedString = "" THEN
> ENTRY(i,cString)
> ELSE cParsedString +
> ENTRY(i,cString).
> END.
> END CASE.

Using my iAddString and the fact the cString is a local variable and already
initialized to "", all of the above could be simplified to something along
the lines of:

DO i = 1 to NUM-ENTRIES (cString):

cParsedString = (IF i = 1 THEN "" ELSE iAddString)
+ ENTRY (i, cString).
END.

If the string was delimited by something else, like "|" or even better an
input parameter iDelimiter then the following 2 changes would apply:
NUM-ENTRIES (cString, iDelimiter)
ENTRY (i, cString, iDelimiter).

The compiler does not complain when iDelimiter is more than one character
but appears to use only the first one. This therefore limits the "delimited
list" approach to replacement of single characters by none, one or more
characters.

If you needed to replace multiple characters with none, one or more
characters then you could use the INDEX function to give you the position of
a string (one or more characters) inside of another string. The whole thing
could go inside a DO WHILE <condition> or an "endless loop" that you would
LEAVE when INDEX returns a zero.

HOWEVER, none of this should be used and instead should be replaced with the
already mentioned REPLACE function. Wrapping any PROGRESS function within
an identical user defined function would also be quite pointless.

> END FUNCTION. /* ParseString */

You are on the right path with that function name as a comment on the END
FUNCTION line.
Congratulations !!!

While I am a lazy comment-ator, I tend to put even needles "block-labels:"
on blocks with more than a handful of lines - just so that I can stick them
on the END of blocks. This makes reading COMPILE LISINGs almost a pleasure
to read. Another motivating factor is having a block label to stick on
NEXT, LEAVE, etc. statements. This way there is never a question what block
is being ended, iterated or left.

HTH,
Frank

"Homework is due Monday" - Greg Higgins
 

Chris Kelleher

Administrator
Staff member
OOOPS !
I made a typo.

> DO i = 1 to NUM-ENTRIES (cString):
>
> cParsedString = (IF i = 1 THEN "" ELSE iAddString)
> + ENTRY (i, cString).
> END.

cParsedString = cParsedString
+ (IF i = 1 THEN "" ELSE iAddString)
+ ENTRY (i, cString).

Frank
 

Chris Kelleher

Administrator
Staff member
Frank Z. Gligic <zGligic@cgoCable.net> writes
<big snip>
>If you needed to replace multiple characters with none, one or more
>characters then you could use the INDEX function to give you the position of
>a string (one or more characters) inside of another string. The whole thing
>could go inside a DO WHILE <condition> or an "endless loop" that you would
>LEAVE when INDEX returns a zero.
>
>HOWEVER, none of this should be used and instead should be replaced with the
>already mentioned REPLACE function. Wrapping any PROGRESS function within
>an identical user defined function would also be quite pointless.
>

Unless, of course, the inbuilt Progress function performs just one of a
range of required actions, as is the case here.

>"Homework is due Monday" - Greg Higgins

It's Monday. Here's mine
smile.gif


Cheers--
Mike Brennan
Softline Ltd.
mailto:mike@softline.demon.co.uk
Tel: +44 1535 645789
Fax: +44 8700 558144 http://www.softline.demon.co.uk
 

Chris Kelleher

Administrator
Staff member
> >Wrapping any PROGRESS function within
> >an identical user defined function would also be quite pointless.
>
> Unless, of course, the inbuilt Progress function performs just one of a
> range of required actions, as is the case here.

What additional "required actions" did Jay's user defined function do that
can not be done with the built in REPLACE function ?

> >"Homework is due Monday" - Greg Higgins
>
> It's Monday. Here's mine
smile.gif


If I missed something, I will write "Homework is due Monday" - 100 times.
wink.gif


Frank
 

Chris Kelleher

Administrator
Staff member
EXCELLENT!

This is exactly the type of feedback I was looking for. I did not know
the REPLACE function existed - must be new to version 8.

Anyway two different ways to accomplish the same thing.

Jay A. Martin
Galaxy Aerospace, LP
jmartin@galaxyaero.com
 

Chris Kelleher

Administrator
Staff member
Jay Martin <jmartin@galaxyaero.com> writes
> I did not know
>the REPLACE function existed - must be new to version 8.
>
And the moral of the story is...
Have a good look at the new keywords when you get your hands on a new
version. Beats coding anyday
smile.gif


Cheers--
Mike Brennan
Softline Ltd.
mailto:mike@softline.demon.co.uk
Tel: +44 1535 645789
Fax: +44 8700 558144 http://www.softline.demon.co.uk
 
Top