• 516052 (unregistered)

    So many pointlessly complicated ways to reinvent the wheel. Like, ok, I get it. Maybe your boss really is that stupid and makes you work with pre 98 C++ standards so you have to do everything by hand. But if that is the case why not just do the simple things? Why bother with itterators and stuff and make your life miserable?

    find(haystack, needle)
{
	if(needle.length > haystack.length) return false;
	
	for(int i=0; i<haystack.length - needle.length; i++)
	{
		for(int j=0; j<needle.length; j++)
		{
			if(needle[j] != haystack[i+j]) goto notFound;
		}
		
		return true;
		
		notFound:
			continue;
	}
}
  • Vilx- (unregistered)

    Wait... this isn't even a "startsWith". It's just "contains" with extra steps. Like, it will find "aaa" inside "bbb aaa bbb". And the rewind will be off too.

  • Vilx- (unregistered)

    No, wait, you're right. It's because it only checks up to "str.length" first characters, where "str" is the needle we're looking for.

  • Greg (unregistered) in reply to 516052

    find( std::string const & haystack, std::string const & needle) { return strpos( needle.c_str(), haystack.c_str() ) != -1; }

  • (nodebb) in reply to Greg

    Unfortunately, in the completely general case, this doesn't work because a std::string can contain embedded NUL characters, which std::string::find() will cope with just fine, but the C functions will most definitely not cope with at all.

  • (nodebb) in reply to 516052

    Maybe your boss really is that stupid and makes you work with pre 98 C++ standards

    If your boss is this stupid, you should urgently look for either (a) a job with a diffferent boss in the same company, or (b) a job in a different company, or (c) a hitman to rid you of your boss.

    Also, you can eliminate the goto by replacing it with a break and make the return conditional on whether you looked at the entire length of needle.

  • Foo AKA Fooo (unregistered)

    Also, rewinding the iterator is unnecessary if they didn't pass it by reference in the first place, which there's no reason to. The whole thing boils down to:

      return string(it, it + str.length()) == str;

    (With somewhat recent versions, you can use string_view instead of string to avoid copying.)

    At least, this could be a teachable moment in code review (if the organization permits that, which I doubt here), or rather a lot of teachable points:

    • Replace the while-loop with a usual for-loop.
    • Merge the two instances of iterator rewinding.
    • Realize they're completely unnecessary when you pass "it" by value.
    • At this point, the code is somewhat readable, so realize you don't need the "find" in the loop and move it out.
    • Since now tmp and str definitely have the same length, replace "find" with "==".
    • Since there's only one test now, "result" is redundant; return the result of the comparison directly.
    • Slightly advanced, you can construct a string from two iterators directly without a manual loop.
    • Explain that we don't pay by LOC so you won't lose 90% of your salary, OTOH I'll get a bonus for removing all this code. (Well one can dream ...)
  • 516052 (unregistered) in reply to Steve_The_Cynic

    On the first point I agree with you. Well, I agree with you now. I remember the days when you didn't have anything but ye old C to work with. xP But that's a story for another day.

    On the second though I actually prefer the goto. It's cleaner and more old school. The later of which is what I was going for with this code.

    This said, it is a pet peeve of mine when I see people use advanced and complex language constructs to do simple things. If you look at code written these days it some times feel like people are afraid of the basics. And the code in this article is a prime example of that sort of behavior.

    You don't need 3 layers of interfaces, LINQ, smart pointers and generics to write hello world. And yet people keep doing that with such a consistency that I am unsure whether they are showing off or just so ignorant of the basics.

  • (nodebb) in reply to 516052

    What I had in mind for getting rid of the goto was more like this:

    find(haystack, needle)
{
	if(needle.length > haystack.length) return false;
	
	for(int i=0; i<haystack.length - needle.length; i++)
	{
		for(int j=0; j<needle.length; j++)
		{
			if(needle[j] != haystack[i+j])
                            break;
		}
		
                if (j == needle.length)
		    return true;
	}

	return false; // you left this line out!
}

    It removes a "continue before closing brace of loop", about which some static analysis tools will complain.

  • (nodebb)

    If you're stuck with pre-98 C++, you use the strstr() function in the C library to do the same thing. I think most of the C++ string functions were implemented using the C library functions.

  • 516052 (unregistered)

    All this talk of C and basic coding really made me feel nostalgic for the good old days. Thanks you guys.

  • (nodebb) in reply to Worf

    In fact, you can't or at least shouldn't, because C++ string objects can contain embedded NUL characters. Well, that's true for std::string but might not be for pre-STL string objects, which makes the concept of passing some equivalent of somevar.c_str() to a C library function highly questionable.

  • 516052 (unregistered)

    Dad joke time - C++ Edition:
    How do you know your programming language is all grown up? It got an STD.

Leave a comment on “Please Find, Rewind”

Log In or post as a guest

Replying to comment #697313:

Log Out

« Return to Article