Programmers! Can you improve my programming?

Problem: I want to convert this string “foo|bar&foobar” to “foo|bar&foobar”. Fact: Following code works. Question: Is this the best way to do this or can this code be improved? String := 'foo|bar&foobar'; NumOfWildcards := STRLEN(DELCHR(String,'=',DELCHR(String, '=', '|&'))); FOR i := 1 TO NumOfWildcards DO BEGIN CASE TRUE OF STRPOS(String,'|') <> 0 : BEGIN SubStrNoString := '%1'; WildCardPos := STRPOS(String,'|'); END; STRPOS(String,'&') <> 0 : BEGIN SubStrNoString := '%2'; WildCardPos := STRPOS(String,'&'); END; END; String := DELSTR(String,WildCardPos,1); String := INSSTR(String,SubStrNoString,WildCardPos); END; String := STRSUBSTNO('*%1*',STRSUBSTNO(String,'*|*','*&*')); Vars: String:Text, SubStrNoString:Text, NumOfWildcards:Integer, i:Integer, WildCardPos:Integer.

What about: NewString := '*'; FOR i := 1 TO STRLEN(String) DO IF String[i] IN ['|','&'] THEN NewString := NewString + '*' + FORMAT(String[i]) + '*' ELSE NewString := NewString + FORMAT(String[i]); String := NewString + '*';

Emiel, Assuming that the two rules which define the string modification in you example are 1. Add a wildcard character to the start and end of the string 2. Wrap any logical operators (i.e the | or &) in wildcard characters. the the following function could be used… The function, called ChangeString has a text input parameter and a text return value. The logical operators are defined as a CSV so you could define this as you like (even use a setup value). It works on the principal that a string can be referenced as an array of characters. ChangeString(VtxtInputString : Text[50]) : Text[152] //define a CSV string for the characters you want to wrap in wildcards LtxtWildCardCandidates := ‘|,&’; //iterate through your inputstring, building the outputstring and adding wildcards when appropriate LtxtOutputString := ‘’;//add the leading wildcard FOR LintCounter := 1 TO STRLEN(VtxtInputString) DO BEGIN IF STRPOS(LtxtWildCardCandidates,FORMAT(VtxtInputString[LintCounter])) >0 THEN LtxtOutputString := LtxtOutputString + '’ + FORMAT(VtxtInputString[LintCounter]) + ‘’ ELSE LtxtOutputString := LtxtOutputString + FORMAT(VtxtInputString[LintCounter]); END; LtxtOutputString := LtxtOutputString + '’;//add the last wildcard EXIT(LtxtOutputString); If you call the function like… MESSAGE(ChangeString(‘foo|bar&foobar’)); you should get the desired result. Chris.

Guys, Good thread! [:D] I couldn’t see anything wrong with Emiel’s code but if Heinz’s works, it looks a better solution. Will go away and test both! [;)]

Heinz, You must have posted just before me… Nice to see a validation of the priciple. Chris.

Chris, 9 minutes, to be precise [;)] But your solution seems to be identical to mine, except that I used the IN operator where you used STRPOS.

ooooh darn guys… I forgot something… It could be possible the sting must be converted to “foo*|bar*&foobar*” with minimal modification. In my code this will be only one extra statement. But as far as I can see, both your code samples need minimal modification too. The art of simplicity. That what I like about your code Heinz although I don’t like looping the complete text (Which resulted in my code, minimal looping). But still, the code is much more compact. Anymore guys? (I’m looking for a really fast whay to convert 1 char to more then one. So | can be converted to |).

Emiel, what do you mean by “modification”? In Chris’ and my own code, the original string is not modified at all, except for my last statement, which could be easily replaced by NewString := … Regarding looping over the string: Our code examines each character of the original string exactly once (well, you could add another local variable for containing String[i]). It’s true, your code does not have an explicit loop over each character of the source string. But you have a lot of STRPOS statements, which of course do nothing else but looping over the string’s characters. Same for DELCHR and INSSTR. As a matter of fact, you won’t be able to solve your problem without examining each character of the string at least once, whether explicitly or implicitly. After all, each single character could be a | or a &, so - you have to look at each character!

quote:


Originally posted by eromein
(I’m looking for a really fast whay to convert 1 char to more then one. So | can be converted to |).


You added this requirement just while I was writing my answer above [;)] I think using C/AL string functions won’t buy you any extra speed here. These functions won’t know any better way to search for these special characters than you, the human programmer: Examine each single character from left to right (or right to left [:D]) until you find one that matches. Binary search, tree search etc. can only be used if the data to be searched is sorted or organized in some way, so they won’t help you here. So, all you do by using C/AL functions is to move the required loops from explicit C/AL code into these routines. There they will probably execute a bit faster, depending on the language the library was written in. But as you will end up calling these functions repeatedly to find all occurrences, you will lose this performance gain.

Aight… So, if nobody comes up with a better solution, yours is the one Heinz!

Hi Emiel, taking a wild guess, I assume you are doing something to allow the user to enter filters. Please keep in mind precedence:

quote:


Operator Hierarchy The precedence order of the C/AL operators: 1 .(fields in records), (indexing), () (parentheses), :: (scope) 2 NOT, - (unary), + (unary) 3 *, /, DIV, MOD, AND, XOR 4 +, -, OR 5 >, <, >=, <=, =, <>, IN 6 … (range)


So you may have some problems here. Probably just a case of training the users, but watch out.

Here’s a variant of the Heinz code. More code, more allocation, less looping. Len:=STRLEN(String); NewString:='*'; IF Len>0 THEN REPEAT i:=i+1; downcount:=Len+1-i; IF String[i] IN ['|','&'] THEN NewString := NewString + '*' + FORMAT(String[i]) + '*' ELSE NewString := NewString + FORMAT(String[i]); IF downcount>i THEN BEGIN IF String[downcount] IN ['|','&'] THEN NewString1 := '*' + FORMAT(String[downcount]) + '*' +NewString1 ELSE NewString1 := FORMAT(String[downcount])+NewString1; END; UNTIL (i+1>=downcount); String:=NewString+NewString1+'*';

Ok, thank you all!!! Very nice to see all those solutions. Allan Ohlfsen yours is very nice! But I’m using the following: FOR i := 1 TO STRLEN(String2) DO BEGIN CASE String2[i] OF '|' : String := String + '%1'; '&' : String := String + '%2'; ELSE String := String + FORMAT(String2[i]); END; END; I’m using the %1 and %2 so I can easily fill those (later on) with ‘|’, ‘|’, '|’, ect…

quote:


Originally posted by David Singleton
The precedence order of the C/AL operators: 1 .(fields in records), (indexing), () (parentheses), :: (scope) 2 NOT, - (unary), + (unary) 3 *, /, DIV, MOD, AND, XOR 4 +, -, OR 5 >, <, >=, <=, =, <>, IN 6 … (range)


David, if the range operator has lowest precedence, then the following filter expression should produce an error or at least behave somewhat strange: 1…5|10…15 But it works fine - you get all values from 1 to 5 as well as all from 10 to 15. What’s going on here?

Yes Heinz you’re right, so does it mean the manuals are wrong?

quote:


Originally posted by David Singleton
Yes Heinz you’re right, so does it mean the manuals are wrong?


What manual did you get this information from? We have only a few manuals, of which I have read only one or two [:D]

Just to share information, and show you all I’m not affraid to make an ass of myselfs! In my sample I’m using %1 and %2 which I’ll fill later on with the ‘|’ and '&’ sting. This does not work of you would like to format this sting ‘mon|17’. This will become ‘mon%117’. %1 is not in this string anymore. If you try to substrno %1 it will not work. Just wanted to sare this with you all. So I’m using almost an exact copy of Heinz code now.