implemented low level find function for string matching

[aman-godara]

closes #421

Tasks:

• Implemented find function
• Added unit tests in test_strings_functions.f90
• Document the newly created function
• Function find: Pure or Elemental? -> Elemental

[awvwgk]

Thanks, this looks great so far.

Have you checked how robust your implementation works for an empty string input to the string and/or pattern?

[milancurcic]

Thanks, great progress so far. A few tests segfault, but I assume you're working on that.

What do you think about making find an elemental function instead of just pure?

[aman-godara]

I checked for empty string/pattern input: code was working fine. But I couldn't find the segfault @milancurcic mentioned. Can you please send me that particular test case which gave this error?

[milancurcic]

But I couldn't find the segfault @milancurcic mentioned.

This may be specific to my setup. I'll look at it later today and report back.

[awvwgk]

Thanks, the tests look complete now. Let us know once this PR is ready for review.

[aman-godara]

What do you think about making find an elemental function instead of just pure?

Currently this function is pure, I will gather some information and write back to you.

Sounds good, could you add those cases to the testsuite as well?

Done

Can you try to rebase your branch?

Thanks for this, done. That's exactly what I was looking for.

I didn't get any errors from cmake testing on my local machine but here on GitHub CI / Build (ubuntu-latest, 10) (pull_request) says there is some problem with optval.

[awvwgk]

I didn't get any errors from cmake testing on my local machine but here on GitHub CI / Build (ubuntu-latest, 10) (pull_request) says there is some problem with optval.

There is something broken with the manual makefile. Not sure what in particular is broken there, will have a look later.

[aman-godara]

What do you think about making find an elemental function instead of just pure?

I gave some thought to it, it can be made elemental.

Argument string and pattern cannot be passed in an array unless they both are of type string_type because, if otherwise, they might have varying lengths causing a problem to pass in the same array.

[awvwgk]

What do you think about making find an elemental function instead of just pure?

I gave some thought to it, it can be made elemental.

Argument string and pattern cannot be passed in an array unless they both are of type string_type because, if otherwise, they might have varying lengths causing a problem to pass in the same array. Thus, I will have to remove find_char_char function.

elemental sounds fine to me. I would vote for making all of them elemental. Searching a whole array of fixed length characters still seems seems like a valid application for find, even if slightly inconvenient. We don't run into a situation of returning strings of different length here, therefore I think elemental will be fine.

[awvwgk]

Thanks a lot, this looks good now.

[jvdp1]

LGTM. Thank you @aman-godara . IMO It can be merged, pending minor changes.

[aman-godara]

I added brackets ( [ and ]) to specify that arguments are optional for slice function (#414) in this PR.

[everythingfunctional]

Often there is a back=.false. argument to functions like this (i.e. the intrinsic function index, which is very much like this one). I'm about to leave on vacation so I won't have time for a much more in depth review. At a quick glance it otherwise looks ok.

@aman-godara (and anybody else), I recommend reading the book The Art of Unit Testing by Roy Osherove to improve your unit test skills, if you can find the time.

[aman-godara]

Often there is a back=.false. argument to functions like this (i.e. the intrinsic function index, which is very much like this one).

A possible way IMO of doing this is to make a new function find_low_level and cut & paste all the code written in find_char_char to this new function. I will keep this new function private and out of the interface find. Then I can add back argument to all the 4 functions that are in find interface. So, these 4 functions will call the find_low_level in their implementation. find_low_level will not take any back argument, rather these 4 high level functions will take care of the back argument and modify the input given by user accordingly before calling find_low_level.

[everythingfunctional]

Often there is a back=.false. argument to functions like this (i.e. the intrinsic function index, which is very much like this one).

A possible way IMO of doing this is to make a new function find_low_level and cut & paste all the code written in find_char_char to this new function. I will keep this new function private and out of the interface find. Then I can add back argument to all the 4 functions that are in find interface. So, these 4 functions will call the find_low_level in their implementation. find_low_level will not take any back argument, rather these 4 high level functions will take care of the back argument and modify the input given by user accordingly before calling find_low_level.

Sounds reasonable to me.

[jvdp1]

Often there is a back=.false. argument to functions like this (i.e. the intrinsic function index, which is very much like this one).

A possible way IMO of doing this is to make a new function find_low_level and cut & paste all the code written in find_char_char to this new function. I will keep this new function private and out of the interface find. Then I can add back argument to all the 4 functions that are in find interface. So, these 4 functions will call the find_low_level in their implementation. find_low_level will not take any back argument, rather these 4 high level functions will take care of the back argument and modify the input given by user accordingly before calling find_low_level.

@aman-godara @awvwgk Do you still plan to investigate/implement this option, suggested by @everythingfunctional ?

[awvwgk]

Let's open a new issue on this and continue the discussion there.