MarcusAseth

to_lower function entering an infinite loop

Recommended Posts

MarcusAseth    68

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
Kylotan    10002

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
MarcusAseth    68
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
MarcusAseth    68
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
Alberth    9525

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
MarcusAseth    68

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
alvaro    21263
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
MarcusAseth    68

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
MarcusAseth    68
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
alvaro    21263

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
alvaro    21263
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
MarcusAseth    68
2 minutes ago, alvaro said:

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

My bad, I just realized I had scrolled past the latest example and compiled the first (old) example x_x

Edited by MarcusAseth

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
MarcusAseth    68

@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
MarcusAseth    68
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
fastcall22    10845

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
MarcusAseth    68

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
alvaro    21263

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


  • Similar Content

    • By DKdowneR
      Hi! I made a tile map reading from a file. Almost everything works good, but when a player go out of map, program runs into an error and says that "vector subscript out of range". My question is how to make check for it
      Drawing map : 
      void GameplayScreen::DrawMap(SDL_Renderer *renderer) { for (int y = map.size() - 1; y >= 0; --y) { for (int x = getStartBlockX(), xEnd = getEndBlockX(); x < xEnd && x < map[y].size(); ++x) { if (map[y][x] != "0,0") { int tempX = atoi(map[y][x].substr(0, map[y][x].find(',')).c_str()); int tempY = atoi(map[y][x].substr(map[y][x].find(',') + 1).c_str()); srcRect.x = tempX * 32; srcRect.y = tempY * 32; srcRect.w = 32; srcRect.h = 32; destRect.x = x * 32 + posX; destRect.y = (y * 32 + posY); destRect.w = 32; destRect.h = 32; vBlock[Earth]->Draw(renderer, srcRect, destRect); } } } } getStartBlockX returns first map block, getEndBlockX returns last, so it's like render on screen only a piece of map, not all blocks.
      tempX returns x coordinate of tile image,  tempY y coordinate. So, for example, if map is like :

      0,0 0,0 0,0 0,0 0,0
      0,0 0,0 0,0 0,0 0,0
      1,0 2,0 0,3 1,0 1,0
      0,0 is first block image, 1,0 is one next to the first, 0,3 is 2 under the first block etc.
         
    • By Shaarigan
      So here again with some more opinion/discussion related topic about what features should/are normally be related into a game engine's core system from the implementation side of view. With a huge time I invested into writing and even more into research for my very own game engine project, there were as many differences as there are to coding guidelines. Especially open source engines (Urho3D, Lumberyard) and those that offer there source code (Unreal, CryEngine, Phyre) change during the centuries depending on current technical, coding and hardware standards. They all consist of third party code and libraries that are set apart from the core system bur rely on it.
      Now I'm on a point where I want restructure/cleanup my project(s), removing obsolete and integrate prototyping code as same as change the growing code base to have a better clean managed file structure. The question that has had to emerge for long or short is what parts of the code base should be a core feature in order to keep the core system clean and flexible. Again reading many cross references it pointed some similarity out but also huge differences (like Unreals hashing/encryption code beeing in core) so I made a list of important stuff any game needs (I left out platform specifics and audio/renderer here because they are platform dependent and should stay in an own module/system)
      Math (Vector, Matrix, Quaternion ...) Threading (Threading, Locks) Container (apart from the STL, Queue, Buffers, certain types of Array ...) String (management, encoding, utils) Input (seen a game without input?) IO (reading/writing files and manage filesystem structure) Type Handling And a list that is optional but may/may not be inside the core code
      Logging (because it is a development tool) Profiler (see logging) Serialization Plugins Events In my opinion, logging and profiler code is a heavyly used feature in development but is stripped out mostly when deploying a release version, not all games rely on events rather than using other interaction (like polling, fixed function loops) and also the same for plugins and serializing/deserializing data; so what do you think should also be a must have in core system or should go from the list into core system as well as a must have? What else should go outside as an own module/system (into an own subfolder inside the engine project's code base)?
      Courious to read your opinions
    • By Poprocks
      Hi everyone!
      I've managed to build zlib, tinyxml2 and tmxparser with cmake-gui & visual studio 2017, but when I try building the example program for tmxparser (https://github.com/sainteos/tmxparser/tree/master/test) I'm getting numerous unresolved externals for errors, and in the warnings they all state the library machine type 'x64' conflicts with target machine type 'x86'. (some of the unresolved symbols include __imp__UnhandledExceptionFilter@4, __imp__getCurrentProcess@0, __imp__HeapAlloc@12 and the file referenced in the error log is MSVCRTD.lib along with varying object files)
      Thing is, when I go back to the cmake configuration listings for all 3 of those libraries, i can't find anything to suggest i've explicitly configured anything as a 64bit build, and when I open the solutions cmake generated for each of the libraries, all of them (including the test project) have x86 as the current target.. what gives?
      Thanks in advance!
    • By mark_braga
      I am working on a tool to auto-generate some C/C++ code by parsing a file. Is there a tool that provides an interface to write C/C++ code like this
      Struct* pStruct = generator->beginStruct("Material"); { pStruct->addVariable("Buffer*", "pBuffer"); pStruct->addVariable("Texture*", "pTexture"); } generator->endStruct(pStruct); /* Output file struct Material { Buffer* pBuffer; Texture* pTexture; }; */ Now I know its quite trivial to write a tool like this but I am curious to know whether a tool already exists. Seems just like a tool generating json/xml but for C++.
      Note: I won't be doing any fancy stuff like templates or inheritance. Just pure C structs.
      Thank you
    • By noodleBowl
      Its been a while since I have done C++ and I was wondering if someone could help me understand local variables and what happens they go out of scope
      So I have this method for creating a vertex shader and it returns a custom class called VertexShader. But before it returns the VertexShader it places it into a std::unsorted_map called vertexShaders
      //Declared at the ShaderModule class level. Map to store VertexShaders in std::unordered_map<LPCSTR, VertexShader> vertexShaders; //Method to create a vertex shader. Returns a VertexShader VertexShader ShaderModule::createVertexShader(LPCWSTR fileName, LPCSTR id, LPCSTR entryPoint, LPCSTR targetProfile) { ID3DBlob *shaderCode = compileShaderFromFile(fileName, entryPoint, targetProfile); D3D11_INPUT_ELEMENT_DESC inputElementDescription[] = { { "POSITION", 0, DXGI_FORMAT_R32G32B32_FLOAT, 0, 0, D3D11_INPUT_PER_VERTEX_DATA, 0 }, { "COLOR", 0, DXGI_FORMAT_R32G32B32_FLOAT, 0, D3D11_APPEND_ALIGNED_ELEMENT, D3D11_INPUT_PER_VERTEX_DATA, 0 }, }; VertexShader vertexShader(id); //Create a new VertexShader device->CreateVertexShader(shaderCode->GetBufferPointer(), shaderCode->GetBufferSize(), NULL, &vertexShader.shader); device->CreateInputLayout(inputElementDescription, 2, shaderCode->GetBufferPointer(), shaderCode->GetBufferSize(), &vertexShader.inputLayout); shaderCode->Release(); vertexShaders[id] = vertexShader; //Place into the VertexShader map. Places copy? return vertexShader; } Now at the end of the method, right as the return is done, I see that the destructor for my variable vertexShader gets called. And this might sound ridiculous, but I really dont understand why?
      Shouldn't it live on in memory since it was placed into the vertexShaders map and only when the vertexShaders map is destroyed then the deconstructor for my VertexShader is called?  By doing vertexShaders[id] = vertexShader am I really just making a copy and then eventually another deconstructor for the VertexShader placed into the map will be called?
       
      Also when using the method the destructor is called twice? VertexShader being returned from the method now also getting destroyed?
      //Declaration of the vertex shader. Done at the class level VertexShader vShader; //Method that uses the createVertexShader void GraphicsSystem::initialize(HWND windowHandle) { /* Other init code*/ vShader = shaderModule.createVertexShader(L"default.shader", "VertexShader", "VShader", "vs_4_0"); }  
  • Popular Now