MarcusAseth

to_lower function entering an infinite loop

Recommended Posts

I am super confused by this...

I was in the middle of an experiment, maybe is just that I am tired and I'm missing the obvious here (so facepalm is ready), but things are not working as expected, Image below (I'll also paste the code in case you want to try it out yourself)

#include <iostream>
#include <ostream>
using namespace std;

void to_lower(char* s)
{
	char* text = s;
	while (s != '\0')
	{
		char letter = *s;
		if (letter > 'A' && letter < 'Z')
		{
			*s = 'a' + (letter - 'A');
		}
		s++;
	}
	cout << text << endl;
}

int main()
{
	char* text = new char[12] {"Hello World"};
	to_lower(text);
	delete[] text;
	return 0;
}

As you see in the debug image, the content of s[0] is 'l', but "char letter = *s;" is yelding a 'e'...what is this madness?! O_O

 

Untitled-1.png

 

EDIT: forget what I asked above, I just understood that the yellow arrow means that the line isn't executed yet, I just had to step forward once :|

So, since that question turned useless I renamed this topic and I have a new question: my function enters an infinite loop.

Apparently the test "while (s != '\0')" is not good because when s points to '\0', the content of s is "" according to the debugger... so how do I test for '\0'? :P

 

Edited by MarcusAseth

Share this post


Link to post
Share on other sites

You're comparing a pointer, i.e. a memory address, against what you're expecting to see when dereferencing the pointer, i.e. the number 0 or '\0'. You need to dereference the pointer.

Also, your if statement is wrong.

And, you're wasting your time duplicating a standard language function.

Share this post


Link to post
Share on other sites
10 minutes ago, Kylotan said:

You're comparing a pointer, i.e. a memory address, against what you're expecting to see when dereferencing the pointer, i.e. the number 0 or '\0'. You need to dereference the pointer.

Also, your if statement is wrong.

And, you're wasting your time duplicating a standard language function.

ops, missed that dereference in the while, I knew I was tired x_x

Fixed my >= and <= , thanks! :D

I disagree on the last point though, I don't think this time is wasted, we are probably assigning value to different things :P

Edited by MarcusAseth

Share this post


Link to post
Share on other sites
9 minutes ago, Alpha_ProgDes said:

Though you wouldn't do this in production code, I agree you should know/understand how to handle pointers.

Exactly my goal here :P

What I'm currently writing is probably ugly and a bit buggy, but I'm getting useful practice, today first time I use const_cast and write my template, so...IMPROVEMENT (for better or worse...):D

char* findx(const char* s, const char* x)
{
	if(s == nullptr || x == nullptr)
	{ return nullptr; }

	char* Tmp = const_cast<char*>(x);
	int FindSize = 0;
	while (*Tmp++ != '\0') { FindSize++; };

	int ID = 0;
	int firstOccurrence = 0;
	bool isMatch = false;
	
	while (s[ID] != '\0')
	{
		if (s[ID] == x[0])
		{
			firstOccurrence = ID;
			for (size_t i = 0; i < FindSize; i++)
			{
				if (s[ID + i] != x[i]) { break; }
				if (i == FindSize - 1) { isMatch = true; }
			}
			if (isMatch) { break; }
		}
		ID++;
	}
	return (isMatch ? const_cast<char*>(s) + firstOccurrence : nullptr);
}

 

Edited by MarcusAseth

Share this post


Link to post
Share on other sites

Using const cast looks horribly wrong, "find" shouldn't change the string!

You have "char *tmp" and return a "char *",  change that to "const char *" too.

 

At a higher level, you are accepting const char * values as parameters, users don't expect that you break your promise, and sneakily return a modifiable point in their const string as return value.

Edited by Alberth

Share this post


Link to post
Share on other sites

The Exercise in the book I'm following gave as only instructions:

Quote

Write a function, char* findx(const char* s, const char* x), that finds the first occurrence of the C-style string x in s.

I don't know how to return a char* out of a const char*, so I tried removing the const, just to fullfill the assignment :P

Edited by MarcusAseth

Share this post


Link to post
Share on other sites

Here you go friend

//take two char* (strings) and return char* to first occurence of second string
char* findx(char* str, char* strx ) {
	
	if (!str || !strx) return nullptr;

	char* found = nullptr;

	for (int s = 0, x = 0; str[s] != '\0' && strx[x] != '\0'; s++) {

		if (str[s] == strx[x]) {
			if( !found ) found = &str[s];
			x++;
		} else {
			found = nullptr;
			x = 0;
		}
	}

	return found;
}

 

Share this post


Link to post
Share on other sites
11 hours ago, MarcusAseth said:

@h8CplusplusGuru much more clean/compact than mine, regardless if I had done it without the const char required by the exercise, thanks for showing me

 

 

Except that code doesn't find "abac" in the string "ababac". Correctness matters.

 

Share this post


Link to post
Share on other sites

lol xD 

Edit: since my lol above was downvoted, I realize it is kind of ambigous and possibly gives the wrong impression of what I laughed about, to put it in context: : it was because the other day I was saying to @h8CplusplusGuru that in my opinion the benefit of the forum was that anyone can correct each other and be corrected so that who's learning has more chance to get the correct informations. My lol was on the lines of "I knew it!"/"I said it"- don't hate pls :D

 

 

Edited by MarcusAseth

Share this post


Link to post
Share on other sites
On 9/14/2017 at 10:45 AM, alvaro said:

 

Except that code doesn't find "abac" in the string "ababac". Correctness matters.

 

I thought it was odd that there was only one loop!  Regardless, it was my intention to show that he's mostly poking around in the dark.

On 9/14/2017 at 10:59 AM, MarcusAseth said:

lol xD 

Edit: since my lol above was downvoted, I realize it is kind of ambigous and possibly gives the wrong impression of what I laughed about, to put it in context: : it was because the other day I was saying to @h8CplusplusGuru that in my opinion the benefit of the forum was that anyone can correct each other and be corrected so that who's learning has more chance to get the correct informations. My lol was on the lines of "I knew it!"/"I said it"- don't hate pls :D

I think the first line of what I had said was that forums such a gamedev are always around.

//take two char* (strings) and return char* to first occurence of second string
char* findx(char* str, char* strx ) {
	
	if (!str || !strx) return nullptr;


	for (int s = 0; str[s] != '\0'; s++) {

		if (str[s] != strx[0]) continue;

		char* found = &str[s];

		int x = 0;
		for ( x=1; strx[x] != '\0' && str[s + x] != '\0'; x++) {
			if (str[s + x] != strx[x]) 
				break;
		}
		if (strx[x] == '\0') return found;
	}

	return nullptr; 
}

Anyways, If you ever need a flashlight =)

Share this post


Link to post
Share on other sites
1 hour ago, h8CplusplusGuru said:

I thought it was odd that there was only one loop!  Regardless, it was my intention to show that he's mostly poking around in the dark.

Well it wasn't odd, you could do it with a single loop, I just did it out of curiosity to discover if I could do it. Seems I can :D
Let me know if you find any logic bug though, just to be sure this is viable.

char* findx(char* s, char* x)
{
	if (s == nullptr || x == nullptr) { return nullptr; }

	int Incremented = 0;
	for (; *s != '\0'; s++)
	{
       		if (*x == '\0') { break; }
		if (*s == *x)
		{
			x++;
			Incremented++;
			continue;
		}
		else
		{
			x -= Incremented;
			s -= Incremented;
			Incremented = 0;
		}
	}
	return (*x == '\0') ? s -= Incremented : nullptr;
}

 

Edited by MarcusAseth

Share this post


Link to post
Share on other sites

As a matter of style, I wouldn't check if the inputs are nullptr. Just like with standard-library functions, it's your responsibility not to pass null pointers around.

However, searching for the empty string should return the first location where that string can be found as a substring, which should be the first character.

Although the new code by h8CplusplusGuru is better, I think it doesn't do the right thing if you look for the empty string.

Share this post


Link to post
Share on other sites
1 hour ago, MarcusAseth said:

I actually tested @h8CplusplusGuru  code with your previous parameters findx("ababac", "abac") and it return nulltpr, so something else is wrong beside passing an empty string :/

Hmmm... I'm not sure how you tried, but it seems to work for me.

Share this post


Link to post
Share on other sites

I'm fully aware it can be done with a single loop, lol. My thought was that It didn't check previous values in the array, most easily solved with a second loop rather than contriving something with a single loop.

char* findx(char* str, char* strx)
{
	if (str == nullptr || strx == nullptr) { return nullptr; }

	char* found = nullptr;
	int s = 0, x = 0;

	while (str[s] != '\0') {

		if (!found) found = &str[s];
		if (strx[x] == '\0') return found;

		if (str[s] != strx[x]) {
			found = nullptr;
			s = s-x +1;
			x = 0;
		} else {
			s++;
			x++;
		}
	}

	return nullptr;
}

 

 

 

Edited by h8CplusplusGuru

Share this post


Link to post
Share on other sites

@h8CplusplusGuru  personally I like this version better for readability, I wonder if there is a way to time it (since it run probably too fast) to see if is better performant than the double loop version.

Only thing is it should return found instead of nullptr at the end :P

 

Edited by MarcusAseth

Share this post


Link to post
Share on other sites
39 minutes ago, h8CplusplusGuru said:

Why should it return found? If it gets to that line it's always a nullptr.

Try it with the findx("ababac", "abac"), it gets there and return a nullptr instead of the substring starting at found. That's why it should return found.

Edited by MarcusAseth

Share this post


Link to post
Share on other sites

This thread is a great example of why it is important to write tests.  We have at least five different implementations in this thread of `findx`, each with varying levels of “correct”.  Automated tests can provide an empirical measure of correctness.  And since `findx` is a pure function, we can easily write automated tests to help us.

First, a crude testing routine for any `findx` implementation:

const char* findx(const char*, const char*);

void test_findx() {
	struct test_t {
		const char* input;
		const char* subject;
		const char* expected;

		test_t(const char* input, const char* subject, const int pos) {
			this->input    = input;
			this->subject  = subject;
			this->expected = ( pos == -1 ) ? nullptr : (input + pos);
		}
	} tests[] = {
		test_t("abc",         "abc",  0),
		test_t("abcdef",      "abc",  0),
		test_t("ababac",      "abac", 2),
		test_t("ababac-----", "abac", 2),

		test_t("abc",         "xyz",  -1),
		test_t("abc",         "",     -1),
		test_t("",            "",     0),
		test_t("",            "a",    -1),

		// ...
	};

	int failed = 0;
	for ( const auto& test : tests ) {
		const char* result = findx(test.input, test.subject);

		if ( result != test.expected ) {
			failed ++;
			cerr << "Test #" << (&test - tests) << " failed" << endl;
		}
	}

	return failed;
}

And running the full example test here:  https://ideone.com/G2Suog#stdout

Test #0 failed: "abc" in "abc", expected "abc", got nullptr
Test #2 failed: "abac" in "ababac", expected "abac", got nullptr
Test #5 failed: "" in "abc", expected nullptr, got "abc"
Test #6 failed: "" in "", expected "", got nullptr
Failed 4 of 8 tests.

... we can see that there are a few cases we’ve missed.  And while we continually fix the implementation, we can think of more corner cases to add to the tests.

Testing can go along way in helping you write correct code and can also help you keep your sanity in face of bizarre bugs.

Share this post


Link to post
Share on other sites

mine passes the tests, yay! :D

well, actually it "fails" this 

Quote

test_t("abc",         "",     -1)

Test #5 failed: "" in "abc", expected nullptr, got "abc"

but I don't consider it a fail, because my original intention was indeed to return the left-hand side argument in case someone did something as pointless as to try to find nothing. But I don't actually know how the usual function decide to handle that case, so I will assume they return nullptr as expected by the testcase.

Anyway I've already saw in the code above something interesting I wasn't fully aware I could write, so I go back to read carefully trough it, thanks for posting it @fastcall22 :)

 

Edited by MarcusAseth

Share this post


Link to post
Share on other sites

As a mathematician, the desired behavior of findx when the second argument is the empty string is obvious: The first instance of the empty string is found at the beginning of the string.

For instance, I could make a function that splits a string into substrings using a separator, like "split" in Perl. This is how that Perl function behaves:

	$ perl -e 'for $x (split ",", "a,b,c") { print "\"$x\"\n";}'
	"a"
	"b"
	"c"
	$ perl -e 'for $x (split "", "a,b,c") { print "\"$x\"\n";}'
	"a"
	","
	"b"
	","
	"c"

So if you look for the empty string, you find it everywhere, and "split" separates every character.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now


  • Forum Statistics

    • Total Topics
      628288
    • Total Posts
      2981843
  • Similar Content

    • By reders
      Hi, everyone!
      I "finished" building my first game. Obviously Pong.
      It's written in C++ on Visual Studio with SFML.
      Pong.cpp
      What do you think? What should I consider doing to improve the code?
      Thank you very much.
       
      EDIT: added some screenshot and .zip file of the playable game
       
      Pong.zip


    • By noodleBowl
      I was wondering if anyone could explain the depth buffer and the depth stencil state comparison function to me as I'm a little confused
      So I have set up a depth stencil state where the DepthFunc is set to D3D11_COMPARISON_LESS, but what am I actually comparing here? What is actually written to the buffer, the pixel that should show up in the front?
      I have these 2 quad faces, a Red Face and a Blue Face. The Blue Face is further away from the Viewer with a Z index value of -100.0f. Where the Red Face is close to the Viewer with a Z index value of 0.0f.
      When DepthFunc is set to D3D11_COMPARISON_LESS the Red Face shows up in front of the Blue Face like it should based on the Z index values. BUT if I change the DepthFunc to D3D11_COMPARISON_LESS_EQUAL the Blue Face shows in front of the Red Face. Which does not make sense to me, I would think that when the function is set to D3D11_COMPARISON_LESS_EQUAL the Red Face would still show up in front of the Blue Face as the Z index for the Red Face is still closer to the viewer
      Am I thinking of this comparison function all wrong?
      Vertex data just in case
      //Vertex date that make up the 2 faces Vertex verts[] = { //Red face Vertex(Vector4(0.0f, 0.0f, 0.0f), Color(1.0f, 0.0f, 0.0f)), Vertex(Vector4(100.0f, 100.0f, 0.0f), Color(1.0f, 0.0f, 0.0f)), Vertex(Vector4(100.0f, 0.0f, 0.0f), Color(1.0f, 0.0f, 0.0f)), Vertex(Vector4(0.0f, 0.0f, 0.0f), Color(1.0f, 0.0f, 0.0f)), Vertex(Vector4(0.0f, 100.0f, 0.0f), Color(1.0f, 0.0f, 0.0f)), Vertex(Vector4(100.0f, 100.0f, 0.0f), Color(1.0f, 0.0f, 0.0f)), //Blue face Vertex(Vector4(0.0f, 0.0f, -100.0f), Color(0.0f, 0.0f, 1.0f)), Vertex(Vector4(100.0f, 100.0f, -100.0f), Color(0.0f, 0.0f, 1.0f)), Vertex(Vector4(100.0f, 0.0f, -100.0f), Color(0.0f, 0.0f, 1.0f)), Vertex(Vector4(0.0f, 0.0f, -100.0f), Color(0.0f, 0.0f, 1.0f)), Vertex(Vector4(0.0f, 100.0f, -100.0f), Color(0.0f, 0.0f, 1.0f)), Vertex(Vector4(100.0f, 100.0f, -100.0f), Color(0.0f, 0.0f, 1.0f)), };  
    • By Rannion
      Hi,
      I'm trying to fill a win64 Console with ASCII char.
      At the moment I have 2 solutions: one using std::cout for each line, let's say 30 lines at once using std::endl at the end of each one.
      The second solution is using FillConsoleOutputCharacter. This method seems a lot more robust and with less flickering. But I'm guessing, internally it's using a different table than the one used by std::cout. I'm trying to fill the console with the unsigned char 0xB0 which is a sort of grey square when I use std::cout but when using FillConsoleOutputCharacter it is outputted as the UTF8 char '°'.
      I tried using SetConsoleOutputCP before but could not find a proper way to force it to only use the non-extended ASCII code page...
      Has anyone a hint on this one?
      Cheers!
    • By Vortez
      Hi guys, i know this is stupid but i've been trying to convert this block of asm code in c++ for an hour or two and im stuck
      ////////////////////////////////////////////////////////////////////////////////////////////// /////// This routine write the value returned by GetProcAddress() at the address p /////////// ////////////////////////////////////////////////////////////////////////////////////////////// bool SetProcAddress(HINSTANCE dll, void *p, char *name) { UINT *res = (UINT*)ptr; void *f = GetProcAddress(dll, name); if(!f) return false; _asm { push ebx push edx mov ebx, f mov edx, p mov [ebx], edx // <--- put edx at the address pointed by ebx pop edx pop ebx } return res != 0; } ... // ie: SetProcAddress(hDll, &some_function, "function_name"); I tried:
      memcmp(p, f, sizeof(p)); and UINT *i1 = (*UINT)p; UINT *i2 = (*UINT)f; *f = *p; The first one dosent seem to give the right retult, and the second one won't compile.
      Any idea?
    • By adapelin
      I am a computer engineering student and i have the assignment below. İ only can write the 2D maze array and have no idea about creating car and time as well. Could anyone write and explain hot to do???
      Minimum Criteria: You are expected to design the game by using C ++ . Below are the minimal criteria: • You must create game board with 2 - Dimensional Matrix • Bonuses create with randomly in the game board • All bonuses have got the same value but different effect for car and score . These effects may be positive or negative . • You must use pointer for creating and using car . Some bonuses may be change car type. • When the game finish, you must show high - score. • For moving car , you need to create coordinate s randomly and you need to write proper control statements. • You must use functions for drawing game board and changing car type . If you need extra functions, you can use it. • If you cannot get out the maze when the time is up , the game is over and you need to show high score. In this project, you must do all minimum criteria. In the end, your program must be work without any errors. Bonus: • Save and load high score information to/from disk • Each bonus has got different random values. • You can create cheat codes for the game. • You can create alternative control for car . • Car can jump over the wall but may lose the score . When car exit the maze , game is over and you need to show high score.    
  • Popular Now