[DGD] find_in_str

[Mikael Lind]

Hello Chris,

On 08/07/07, chris . <psych_mayo at hotmail.com> wrote:
> This is a function i made for my auto object.  It allows you to search a
> string for instances of a substring.  It returns an array containing the
> locations in the string of the start point for each substring.  I welcome
> suggestions and mention of any bugs.

First suggestion: Put utility functions in separate utility programs
(e.g. /lib/util/string) instead of adding them to the auto object.
Rationale: Cohesion.

> static int *find_in_str(string search, string str) {
>         int i;
>         string *exploded;
>         exploded = explode(search+str+search,search);
>         if(sizeof(exploded) > 1) {
>                 int charnum;
>                 int *output;
>                 charnum = 0;
>                 output = ({});
>                 for(i=0;i < sizeof(exploded)-1;i++) {
>                         charnum += strlen(exploded[i]);
>                         output += ({charnum});
>                         charnum += strlen(search);
>                         }
>                 if(sizeof(output) != 0)
>                         return output;
>                 }
>         }

There does not seem to be a return statement for all control paths. I
think that this means that the function will return nil in those
cases.

Second suggestion: Have a return statement for all control paths.
Rationale: Style.

Third suggestion: Personally, I probably would prefer returning an
empty array instead of nil when the string is not found. Rationale:
Consistency.

Fourth suggestion: You can avoid the array concatenations by
preallocating the result array with allocate(). Rationale:
Performance.

The name is slightly ambiguous. What would you call a function that
only returns the index of the first match?

Fifth suggestion: Rename the function to find_all_substrings() or similar.

I have not tested your function so I do not know if it works. I did
not see any obvious problem with it when reading it. The function
looks well suited for unit testing. You could write a test program
that calls it with some tricky input and verify that it does the right
thing.

Regards,
Mikael