Cracktro :)

The Partridge Family were neither partridges nor a family. Discuss.
Byteraver
Posts: 60
Joined: May 19th, 2016, 6:48 am
Location: Belgium, Europe

Re: Cracktro :)

Post by Byteraver » October 24th, 2019, 7:16 pm

Hey albinopapa, thank you SO, SO much for going through my code and providing me with such valuable feedback. I will take my time to study your elaborate response in detail. I always felt it would be completely unreasonable to ask a more experienced programmer to go through my code and comment on my style or the lack thereof. Yet you did so without asking and for that I am most grateful. I've read it diagonally so far, and saw a lot of sound advice indeed.

As most people (?) I'm not really afraid of the logic behind the programming, but have a severe lack of patience when it comes to learning how to use the tools (C++, the debugger, image editing software maybe, etc) that are at our disposal. (Github is a particular pet peeve of mine, I hate it with a passion). This then bites me in the ass later on when I have to go through my own code again and understand none of it :D

I look at Chili's code examples and say to myself, now THAT is poetry. I don't understand it, but it looks beautiful :lol:
Now why can I not do that? Well, I'd say one of the issues with C++ is that it is compatible with C (more or less), and that I originally learned programming before Object orienting programming was a thing (talking Turbo Pascal 4.0 and Turbo Assembler here). I'm familiar with pointers and not afraid to use them, but I probably should ;) Because once your program grows too large one of them is going to fail and well... good luck debugging that.
The thing is, there are SO many ways to do the same thing in C++, and it is very hard for an amateur (when googling / using stackoverflow) to understand what is right or not, what is old fashioned or not, best practices etc. I believe this to be a major disadvantage of C++. Endless possibilities are not always advantageous. Still, I love its power, so C++ it is and C++ it will stay.

Advanced concepts such as multi threaded programming, Windows Programming, Inheritance, meta programming, templates, the STL, safe pointers etc I know very little about if not nothing. I'm only starting to dare to use std::vector and std::string rather than an ordinary array. You see, I apprehend stuff I don't know the internals of, and am afraid it might slow down my program enormously, because I use the tools (STL) in the wrong way, unknowingly. This happened with the code my girlfriend wrote for her PhD thesis; I made it 1000% faster by removing a vector resize from the most inner loop. Which mattered a lot, because the program then still needed two weeks (!) to finish. Better have a stable energy supplier in such case, I tell ya ;) because in case of a power outage, you loose up to two weeks worth of calculations.

Since Chili told me to use XAudio2 instead of the WAVE mapper I'm playing with that right now. So far I have succeeded in making it play a single wave file, after discovering that I need to put "CoInitializeEx( NULL,COINIT_MULTITHREADED );" in the beginning of the code. I barely understand what it does, but hey I have sound!
Now, onto https://docs.microsoft.com/en-us/window ... -from-disk :)

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » October 24th, 2019, 11:55 pm

I've been blocked with that kind of fear as well. It's nothing to be ashamed of for sure, at least I'm not. I've encountered others whom feel the same way and choose to implement their own versions of things just to get the feel for what the STL implementations do. Plus it give a good comparison for benchmarking.

I'm not terribly familiar with the STL myself actually and have only recently started preferring the algorithms over raw loops. The only thing that helped me understand how these work was looking through the library code myself. This is kind of difficult if you have a setting enabled that only debugs your project code and skips over external library code.

Yes, options are nice, but too many options becomes overwhelming. C++ is not the only place that happens. I dislike a lot of 3D modeling software suites because there are so many options that the interface is hard to navigate and the font becomes either too small for me or they remove the text in favor of icons which is less than helpful until you learn the entire interface. However, when Windows95 was a big thing, I loved how you could get to certain settings using three or more different methods. Windows8 and higher seems to be taking away choices in favor of simpler navigation which is also nice on it's own.

As far as using the STL containers and algorithms incorrectly, there is always testing. As a matter of fact, the code I posted on IniFile::readFile I think it was even has a few bugs in it as I found out after trying to run the program again.

From my own testing on my own machine, using iterators vs pointer arithmetic vs index based loops there is little to no difference in performance, so using the right algorithm shouldn't have a performance impact vs writing your own raw loop and using pointer arithmetic or indices.

My first languages was Basic then Microsoft QuickBasic. The programs I tried in Basic were all from books and never worked. The QuickBasic period was mostly modifying the provided Nibbles game to add two more players using same keyboard. It wasn't until 2012 that I got into C++ with Chili and at the time one of the moderators here named Luis helped me on occasion. Everything I mentioned on the suggestion post has been listening to CppCon videos and reading through cppreference.com quite often for best practices and usage. I only use StackOverflow now...well I haven't been there in a while actually. Too many snobs or elitists and it's disgusting.

C++ is about abstractions to help your code express your intentions. The low level stuff still has it's place, but there are usually better ways of getting the results you want with sufficient speed and less code.

I too have looked at chili's code and thought how nice it looked and also how unbelievably lost I was at what exactly it did. This is the difference between a high school spanish class and being a native speaker. It's awe inspiring and disheartening at the same time.

Something I thought was neat at one time ( still kind of do ) was watching a youtube video of someone writing a program from the top down. He wrote out how the program should behave using functions, then just filled in the function definitions. So essentially, he wrote the program as if all the functions already existed. No extra lines of code, only function calls that didn't exist at the time. Then just went back in filled them in. Granted this was only a short program, but dang it seems like it would be easier. I have yet to try this myself and just might give it a go soon.

Don't get me wrong, I'm still trying to decipher a lot of what you have going on in the code and it's mostly because it's implementation details. When you bog it down with individual operations instead of named sections it makes code difficult to understand.

Here's a good example:

Code: Select all

void    doCubicBezier( Graphics& gfx )
{
    struct Point {
        int x;
        int y;
    } p0,p1,p2,p3;

    // make sure we don't draw off the screen
    const int border = 40;
    p0.x = border + rand() % (Graphics::ScreenWidth - (border * 2));
    p1.x = border + rand() % (Graphics::ScreenWidth - (border * 2));
    p2.x = border + rand() % (Graphics::ScreenWidth - (border * 2));
    p3.x = border + rand() % (Graphics::ScreenWidth - (border * 2));
    p0.y = border + rand() % (Graphics::ScreenHeight - (border * 2));
    p1.y = border + rand() % (Graphics::ScreenHeight - (border * 2));
    p2.y = border + rand() % (Graphics::ScreenHeight - (border * 2));
    p3.y = border + rand() % (Graphics::ScreenHeight - (border * 2));

    // draw four reference points. First the endpoints in red:
    gfx.drawLine( p0.x - 4,p0.y - 4,p0.x + 4,p0.y + 4,Colors::Red );
    gfx.drawLine( p0.x + 4,p0.y - 4,p0.x - 4,p0.y + 4,Colors::Red );
    gfx.drawLine( p3.x - 4,p3.y - 4,p3.x + 4,p3.y + 4,Colors::Red );
    gfx.drawLine( p3.x + 4,p3.y - 4,p3.x - 4,p3.y + 4,Colors::Red );
    // and now the middle points:
    gfx.drawLine( p1.x - 4,p1.y - 4,p1.x + 4,p1.y + 4,Colors::LightBlue );
    gfx.drawLine( p1.x + 4,p1.y - 4,p1.x - 4,p1.y + 4,Colors::LightBlue );
    gfx.drawLine( p2.x - 4,p2.y - 4,p2.x + 4,p2.y + 4,Colors::LightBlue );
    gfx.drawLine( p2.x + 4,p2.y - 4,p2.x - 4,p2.y + 4,Colors::LightBlue );
    // name them:
    gfx.printXY( p0.x - 16,p0.y - 16,"P0" );
    gfx.printXY( p1.x - 16,p1.y - 16,"P1" );
    gfx.printXY( p2.x - 16,p2.y - 16,"P2" );
    gfx.printXY( p3.x - 16,p3.y - 16,"P3" );

    const int nrPoints = 50;
    float m = 0;
    float mprev = 0;
    float xprev = (float)p0.x;
    float yprev = (float)p0.y;

    float x1prev = (float)p0.x;
    float x2prev = (float)p0.x;
    float y1prev = (float)p0.y;
    float y2prev = (float)p0.y;

    for ( int t = 0; t <= nrPoints; t++ ) {
        // calculate t == t2:
        float t2 = (float)t / (float)nrPoints;
        // calculate (1 - t) == t1:
        float t1 = 1.0f - t2;
        // calculate coordinates of t (quadratic):
        //int x = (int)(t1 * t1 * p0.x + t1 * 2 * t2 * p1.x + t2 * t2 * p2.x);
        //int y = (int)(t1 * t1 * p0.y + t1 * 2 * t2 * p1.y + t2 * t2 * p2.y);
        // calculate coordinates of t (cubic):
        float x = (t1 * t1 * t1 * p0.x + t1 * t1 * 3 * t2 * p1.x + t1 * 3 * t2 * t2 * p2.x + t2 * t2 * t2 * p3.x);
        float y = (t1 * t1 * t1 * p0.y + t1 * t1 * 3 * t2 * p1.y + t1 * 3 * t2 * t2 * p2.y + t2 * t2 * t2 * p3.y);
        //gfx.drawLine( (int)xprev,(int)yprev,(int)x,(int)y,Colors::Green );

        // draw perpendicular line:  
        float thickness = 20; // in pixels, should be less than border

        // get crossing point of two lines:
        float xc = xprev + (x - xprev) / 2;
        float yc = yprev + (y - yprev) / 2;

        float yD = y - yprev;
        float xD = x - xprev;

        float alfa;

        mprev = m;
        m = (-xD / yD) ;

        alfa = atan( m );

        /*
        if ( alfa > (3.14159265359 / 2.0) )
            alfa = alfa - (3.14159265359 / 2.0);
        */

        float xd = (thickness / 2) * cos( alfa );
        float yd = (thickness / 2) * sin( alfa );

        if ( m * mprev < 0 ) {
            float tmp = y1prev;
            y1prev = y2prev;
            y2prev = tmp;
            yd = -yd;
            /*
            xd = -xd;
            tmp = x1prev;
            x1prev = x2prev;
            x2prev = tmp;
            */
        }

        float x1 = xc - xd;
        float y1 = yc - yd;
        float x2 = xc + xd;
        float y2 = yc + yd;


        gfx.drawLine( (int)x1prev,(int)y1prev,(int)x1,(int)y1,Colors::Magenta );
        gfx.drawLine( (int)x2prev,(int)y2prev,(int)x2,(int)y2,Colors::Magenta );
        x1prev = x1;
        x2prev = x2;
        y1prev = y1;
        y2prev = y2;

        /*
        // perpendicular line:
        float x1 = xc - (thickness / 2) * cos( alfa );
        float y1 = yc - (thickness / 2) * sin( alfa );
        float x2 = xc + (thickness / 2) * cos( alfa );
        float y2 = yc + (thickness / 2) * sin( alfa );
        gfx.drawLine( (int)x1,(int)y1,(int)x2,(int)y2,Colors::Magenta );
        */

        gfx.drawLine( (int)xprev,(int)yprev,(int)x,(int)y,Colors::Green );

        xprev = x;
        yprev = y;
    }
}
The function is called "doCubicBezier", but what the hell is going on? LOL. Why is there so much going on in drawing a bezier curve? I'm going to have to isolate this function and step through it about 20 times or more just to understand what everything does. Luckily you are the type of programmer that has comments, this helps some until you have code commented out and I'm not sure if it is suppose to be there for future use or is no longer needed.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » October 25th, 2019, 2:23 am

Is this what it is suppose to look like?
Image
It has some graphics issues to me near P0 and between P2 and P1, but I could be wrong. Aside from thickness, what was the goal?
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

Byteraver
Posts: 60
Joined: May 19th, 2016, 6:48 am
Location: Belgium, Europe

Re: Cracktro :)

Post by Byteraver » October 25th, 2019, 5:03 pm

Hi albinopapa :)

Now that you mention it, my first programming experiments were also in qBasic (the free version of QuickBasic, without the "compiler"). My experiments were so bad I didn't consider it programming ;)

CppCon videos are great, although I wouldn't pretend I understand them, my level of C++ is generally too low for that. The cppreference.com site I don't like. I don't like the way the code is presented visually, and the explanations are too theoretical / academic for me usually.

While I like Stackoverflow, I find that when it comes to c++, I often have the impression I'm reading up on a philosophical discussion. That is simply the result of the fact that c++ is so powerful, its almost a way to translate human thought / concepts into code, rather than just being a programming language.

Some criticize c++ for being a low level language that tries to be a higher level language, and there is some truth in that I guess. I remember a quote from a demo programmer who went like "c is just assembler with more macros" ;) That was 25 years ago though.

The topdown approach is very interesting, when I published my Arkanoid game on this forum I wrote that programming starts with paper & pencil. I guess that approach goes in the same realm. If you can say such a thing ;)

When deciphering my code (I'm so glad you're still at it), you might want to keep in mind that I find it easier to quickly modify an existing program than starting a new project, copy the chili framework there, etc. This means that the repo https://github.com/TheRealByteraver/RTSProject contains testcode that is not actually related to the program, and that will be deleted later. The Bezier curve thing is an example of that. I want to be able to read SVG (vector graphics) files, and for that I need to be able to draw Bezier curves, so I quickly wrote a test program to see if I could do that. The screenshot depicts what the code does; I was investigating how to draw a thick curve (thicker than one pixel) as this is a requirement to be able to draw bezier curves. As you can see, it tries to draw two curves that follow the central one but fails to do so ;). I will probably solve this later by connecting the outer and inner lines with triangles and filling them up (like a rasterizer).

Another example of testcode is the sincInterpolationDemo( Graphics& gfx ) function, that generates a waveform which is supposed to simulate an audio wave, that then gets sampled, so I can reconstruct the original waveform based on the sample points, using different interpolation techniques. I wanted to check how much sinc interpolation (sin(x)/x) is better than cubic interpolation. This was done for my mod replay code (https://github.com/TheRealByteraver/XMPlayer).

Again, when I wrote the cracktro, I added a header file with the intro code and locked the state machine in game.cpp in the intro state.
Same goes for the floodfilltest, a lame floodfill I wrote, again because I would need it for the SVG project.

If I am allowed to make a suggestion, I would be interested to know what you think of https://github.com/TheRealByteraver/XMLParser (it is not a lot of code), as this is something I made recently, so it would probably better reflect my current c++ level. It reads in an XML file (again, necessary for reading .SVG files) into memory. The code is not finished in the sense that the class does not have an interface yet, but the reading part is mostly finished. No pressure :) The XMPlayer repo is also rather OK-ish, except for the .IT loader and the MOD_to_WAV.cpp file which needs cleaning up.

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » October 26th, 2019, 5:26 am

I've been looking through the XMLParser code, downloaded an SVG file since one wasn't provided or couldn't find it and my first impressions are it looks a lot cleaner even if I don't understand things because XML is a freaking mystery to me.

Something I wanted to say, mostly unrelated to your code, but came about because of your code. I found a place to use do/while that actually makes sense to me.

Here's a loop in XMLFile::tagIsAComment function:

Code: Select all

	for ( ;;) { 
		size_t l = endMarker.length();
		if ( l == 0 )
			break;
		l--;
		if ( endMarker[l] != ' ' )
			break;
		endMarker.resize( l );
	}
Here's HOPEFULLY the same functioning loop as a do/while loop

Code: Select all

	auto i = endMarker.length();
	do { --i; } while( i < endMarker.length() && endMarker[ i ] == ' ' );
	auto const offset = i < endMarker.length() ? i : std::size_t( 0 );
	endMarker.erase( endMarker.end() - offset, endMarker.end() );
First decrement i.
If endMarker length is 0, then i will wrap around to UINT_MAX and ( i < endMarker.length() ) will be false and the loop will exit before accessing endMarker[ UINT_MAX ] ( hopefully read in that order ).
If endMarker[ i ] == ' ' loop continues else it exits
Must do one more check on 'i' as the loop might have exited because of wrap around
The range version of std::string::erase() as well as other containers like std::vector will exit early if the first iterator is equal to the second iterator, so no need to "if check".

Anyway, I tried other variations of loops using different arrangements of for() or while() and this is by far the cleanest. I couldn't think of an STL algorithm that would help find the end of printable ascii characters excluding white space. All the algorithms take a known beginning and end iterator. Even the std::string::find* functions don't seem to help unless you want to loop through the entire string. It would be nice to pass reverse iterators to the std::string::find_first_not_of:

Code: Select all

// Would be ideal to have something like this
auto it = endMarker.find_first_not_of( endMarker.rbegin(), endMarker.rend(), ' ' );
endMarker.erase( offset, endMarker.end() );
That kind of brings me to a suggestion for a project like this.
Consider creating some "quality of life" algorithms if for no other reason than readability and perhaps maintainability. Each algorithm should do one thing and one thing only, usually in a loop.

I still see a lot of in/out parameters, but at the same time you are returning multiple parameters in some cases. One of the issues with in/out parameters is you have to create the object before passing it in. Since const is a helpful construct, it would be nicer to return objects through the return parameter allowing you to make things const.
You also do this because you have bool or int status values. If you are going to exit the program anyway, throwing exceptions instead is a lot easier to manage.

For example:

Code: Select all

	// we didn't find a tag end marker: exit with error
	if( pos2 == std::string::npos )
	{
		throw std::runtime_error( "Didn't find a tag end marker." );
	}
In main, use try/catch.

Of course, if you don't need to exit the program when the file is invalid or can't be found, then you may not want to use exceptions. In those cases, you could use a C++17 library feature called std::optional. This way you still use the return parameter. With std::optional you either return the object or return an empty std::optional ( which holds an std::nullopt object ).
Use it like a pointer:

Code: Select all

if( !optValue ) 
{
	MessageBox( nullptr, L"Not a valid XML file.", L"XML Parsing Error", MB_OK );
	fileIsParsed = false;
	return;
}
This would be good for when you have a GUI implemented and user can open files using the OpenFile dialog box. You wouldn't want to exit the image program just because of an invalid file AND you don't want XML parsing logic in the main program.

So that brings back the "single responsibility" idiom. Consider separating functions into smaller sections. Your extractFromStringStream for example could probably be broken up into functions as each commented section hints at.
  1. start reading where we left of
  2. start looking for the start marker (e.g. '<')
  3. We copy the last characters preceding the marker to the preceding string.
  4. we continue with everything after the start marker, including the marker itself:
  5. start looking for the end marker (e.g. '>')
  6. check if the end marker (e.g. '>') is at the end of the string:
Not every commented section needs to be it's own function, but splitting them up not only helps readability, but testing sections or debugging sections is easier than having to test or debug the entire function at once.

Move semantics using std::move or just returning a value from a function using the return parameter can helpful in some cases.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

Byteraver
Posts: 60
Joined: May 19th, 2016, 6:48 am
Location: Belgium, Europe

Re: Cracktro :)

Post by Byteraver » October 26th, 2019, 10:21 am

Hi again albinopapa!
(I will here answer your post on the XML parser and answer your previous posts later)
About the loops: I know about the different loops in C++, how they work etc. It is just that I read somewhere that sticking to one style of loop makes the code clearer / simpler (for the reader). It kind of made sense to me. Some senior programmers tend to dislike the "do {} while ();" loop construct as it is supposedly error-prone, especially when the code preceding it gets modified. I don't see the problem, but I do like to keep things simple. It would be interesting to get your thoughts on that?

When you say I should create some "quality-of-life" functions, I suppose you mean I should replace code such as:

Code: Select all

	for ( ;;) { 
		size_t l = endMarker.length();
		if ( l == 0 )
			break;
		l--;
		if ( endMarker[l] != ' ' )
			break;
		endMarker.resize( l );
	}
with something like:

Code: Select all

endMarker = trimTrailingWhiteSpace( endMarker );
? I totally agree ;)

About exceptions: I don't like them, because I don't know how they work on a low level (like what is really happening? Are they interrupts? Events? Jumps? I believe it was the latter). From what I gathered so far, the compiler just jumps to where ever you catch the exception, and prepares for these jumps at compile time, making sure the stack and heap are cleaned up while doing so. Yikes. It scares me, because I suppose there are rules to follow when you use exceptions, and I don't know these rules. It surely looks elegant, but I have to understand it before I use it, otherwise it will be impossible to debug. It is a high level feature, isn't it? I should probably just study them properly. It definitely makes for code that is easier to read.

Byteraver
Posts: 60
Joined: May 19th, 2016, 6:48 am
Location: Belgium, Europe

Re: Cracktro :)

Post by Byteraver » October 26th, 2019, 12:20 pm

I had a second look at the way you rewrote the for loop and I think I still like my version more - no offense. It needs more lines of code, but I feel it is easy to understand. It is slow though, since the "endMarker.resize( l )" is inside the loop, and I suppose this is costly. In practice however it is rather rare that more than one space needs to be trimmed in this particular application. For a more general function this would probably work faster:

Code: Select all

std::string trimTrailingSpace( std::string str )
{
	int l = str.length();
	if ( l == 0 )
	    return str;
	l--;  
	int i = 0;
	for ( ; l >= 0 ; l-- ) { 
		if ( str[l] != ' ' )
			break;
		i++;
	}
	str.resize( l - i + 1 );
	return str;
}
But unless the function is called thousands of time it'll make no difference of course ;)

Byteraver
Posts: 60
Joined: May 19th, 2016, 6:48 am
Location: Belgium, Europe

Re: Cracktro :)

Post by Byteraver » October 26th, 2019, 12:24 pm

Hi albinopapa!

(answer to your initial code-review answer in this post, part I: )
First of all, your version of the ini reader code looks way more elegant ;) Thank you for that. I especially like that it is shorter.

I started to look at your comments, and I have a lot of questions. If you feel inclined to answer, feel free to just give me a link to examples / discussions elsewhere, I don't mind reading. I tried to google for answers where I could, but could not find all the answers, hopefully you can fill in the void :)

I am a bit dumbfounded by:

Code: Select all

auto strBuf = std::string();
Why is this better than:

Code: Select all

std::string strBuf;
And more importantly, what does it do (like what goes really on here, in detail)? You call the constructor of an std::string without parameters and assign the created object to strBuf, right? What is the difference with declaring a variable/object of the std::string type?

About using both C and C++ functions: guilty as charged. One of the difficulties with C++ is that when you google example code, you may find C code that does the job fine, without realising it is C rather than C++ code. Most of the time I can't tell the difference. This is not aided by the fact that C++ programmers prefer to use C code inside C++ projects for certain things like file I/O because the C++ libraries are not mature enough yet (slow). At least, that is how it used to be until a few years back I believe. Thanks for pointing it out.

About your remarks:
"C++17 guarantees in most cases copy and move elision for return values. This means you can stop using in/out parameters."
-> Is using in/out considered bad style? If so, why? I see it is also used in your version of the code in the line:

Code: Select all

std::getline( rawData, strBuf );
Is that so because the std::getline() function only works in this way?
The thing is, the line:

Code: Select all

strBuf = deleteWhiteSpace( strBuf );
looks like it would produce way slower code than the line

Code: Select all

deleteWhiteSpace( strBuf );
for the casual viewer (me :D) that does not realize the compiler will optimize all of it away.

"C++17 also has std::optional for cases where you can't always return a value, it's like returning a pointer, but with value semantics so there isn't a heap allocation."
-> Neat, I'll keep it in mind. I'm not sure I completely understands how it works. From what I've read so far, it includes a boolean telling you if the return value is valid or not, right? Saving you from checking if the return value is invalid simply by looking at its contents (e.g. like "-1" indicating a substring was not found in a given string, for a function that would otherwise return the location of the substring in the give string).

"Iterators are safer to use than pointers, but may be a little slower during Debug."
-> An iterator is like a fancy pointer right? Is it preferable to use a vector iterator instead of just indexing the vector with an int?

(more later, need to get a haircut now :( )

User avatar
chili
Site Admin
Posts: 3948
Joined: December 31st, 2011, 4:53 pm
Location: Japan
Contact:

Re: Cracktro :)

Post by chili » October 26th, 2019, 5:39 pm

This was quite the code review :D
Chili

albinopapa
Posts: 4373
Joined: February 28th, 2013, 3:23 am
Location: Oklahoma, United States

Re: Cracktro :)

Post by albinopapa » October 26th, 2019, 7:33 pm

Some senior programmers tend to dislike the "do {} while ();" loop construct as it is supposedly error-prone, especially when the code preceding it gets modified. I don't see the problem, but I do like to keep things simple. It would be interesting to get your thoughts on that?
I am not a senior programmer in anything, and I don't know any senior programmers, but I do have an opinion about the comment "it is error prone especially when the code preceding it gets modified". How is do/while any different than declaring your variables before a for or just a plain while loop? As far as I can tell, there's no difference. Aside from that, I might agree for the most part about being consistant and as I stated I tried different variations of for and while loops. The do/while version just looked so clean to me, but you may be right considering I still had to leave a comment about how it worked lol. The main things I don't like about your version is the use of 'l' as a variable name ( I know, petty ) as it looks too similar to 'i' or 'I' to someone with a visual problem like myself AND more importantly the use of the resize function instead of the erase function. The erase function is more discriptive and tells the reader exactly what you are doing...erasing a portion of the string. Using the resize function just says you are resizing the string. However in either case, you have to mathematically work through your version to determine what part of the string is being erased or what size the string is going to be after the resize. Think of this, which seems more natrual, "erase all white space after the last period" or "resize your array so there is no white space after the last period". Hopefully, the former seems more natural and expresses your intension.
When you say I should create some "quality-of-life" functions, I suppose you mean I should replace code such as:...with something like: endMarker = trimTrailingWhiteSpace( endMarker );
Yes, that is exactly what I'm saying. While you are correct that this is a relatively short project and doing such a thing would seem meaningless, I tend to prefer writing these quality-of-life functions because it makes reading the code more explicit and easier to understand. Code can change over time and comments either need to change to reflect the changes in code OR you can let the names of your functions reflect those changes. Having a function called trimTrailingWhiteSpace is very descriptive and tells the person reading the code ( sometimes you ) exactly what it's doing and if the implementation of the calling function changes at least the code inside trimTrailingWhiteSpace won't be affected and there is less of a need for comments.
About exceptions: I don't like them, because I don't know how they work on a low level (like what is really happening? Are they interrupts? Events? Jumps? It is a high level feature, isn't it?
The compiler will exit each previously called function and destructors of each object on the stack that was created during each of those function calls until it either reaches the catch block or exits main without a handler at which point the compiler will just terminate. I say the compiler, but obviously having an exception thrown outside of the debug version has no compiler and it would be more correct in saying that the program will reach a point where no catch block is reachable and just terminate in the latter case. They are a way of having your program clean up after itself IF YOU USE DESTRUCTORS AND NOT CLEANUP FUNCTIONS. There are apparently some gotcha's when using exceptions apparently, like if an exception is thrown before a previous exception is handled the compiler writers are allowed to just call terminate() without unwinding the stack or cleaning up anything. I guess in this case, they would be an advanced topic considering the potentially harmful caveats. Heap allocated memory is always released back to the OS when program ends no matter how the program ends, but I'm not sure about OS acquired handles to resources.
Why is: auto strBuf = std::string(); better than: std::string strBuf;
And more importantly, what does it do?
In C++11 the C++ standard introduced 'auto' as a way of allowing the compiler to deduce the type of declared variables and more. In this particular example, there is no difference between the two versions. In order for the compiler to know that strBuf is an std::string, and the auto version uses something called "copy initialization" which is somewhat of a lie as there is actually NO copy going on. The binary code generated should be exactly the same in either case as long as it is copy initialization and not copy assignment. So why is it better? It's not. I've just become use to this method because I like using auto. Let's say you have a function that returns some object.

Code: Select all

MyWidget& getWidget()
Now, I can do one of two things,
MyWidget& widget = getWidget();
or
auto& widget = getWidget();

Now, let's say MyWidget becomes MyMouseWidget. What has to change in the two versions?
First the function declaration changes no matter what:

Code: Select all

MyMouseWidget& getWidget()
In the first version, you also have to change the type of widget being assigned to:
MyMouseWidget& widget = getWidget();

In the second version, no change required
auto& widget = getWidget();

There are other useful reasons to using auto, the most common example is creating an iterator:
std::vector<MyWidget>::iterator iter = widgets.begin();
OR
auto iterator = widgets.begin();
I prefer the latter.
Is using in/out considered bad style? If so, why?
I did preemptively answer this one in the previous post. In/out parameters require you to declare an object before passing to a function to have it filled in. This means it is either first in an undefined state, default constructed or constructed/assigned dummy values, then passed to a function that could possibly not do anything with it because some condition isn't met or error occurs. If the function must create an object then returning the newly created object to a variable being initialized means the same as in the case of copy initialization, no copy is done you are just giving a name to the returned value. Also, as a person who likes using const, you get to declare said returned object as const which may have efficiency gains as well as protect you from accidentally modifying the returned value if that is not desired. Yes, std::getline does indeed have in/out variables and for this TYPE of function it's unavoidable. Std::getline takes in a stream object and an object to fill in using the stream. Since functions can be overloaded, but functions with different return values only are basically the same function, you need a way of differentiating between the different overloads.

void func();
int func();
float func();

The compiler sees these as all the same function so sometimes in/out params are the choice for some. Plus, std::getline returns a reference to the stream you pass in, so there is no other way around it. I believe the design decision was made so you could, if desired, chain calls to the stream member functions or related free functions. Don't even ask for an example of where chaining calls from std::getline would be usable because I don't have one LOL.
From what I've read so far, it includes a boolean telling you if the return value is valid or not, right?
More or less, yes std::optional does have machinery inside that determines if it has been created with a valid object or not and is implicitly convertable to bool using an operator bool() overload. The operator*() overload returns a reference to the object stored if there is one or throws an exception if it doesn't have a valid object. Previously, some would have just created a std::pair<bool, std::string> or something and tested the .first for true or false before accessing the .second member.
An iterator is like a fancy pointer right? Is it preferable to use a vector iterator instead of just indexing the vector with an int?
Yes, iterators are indeed considered fancy pointers and behave as such. The difference being they are an abstraction representing a pointer and as such you can write code that tests if an iterator is valid like testing if it's within a certain range without having to expose that part to the main logic of the program.
To answer the second part, I think I mentioned before that the difference between using an iterator or index based loop has little to know difference in performance. The iterator version does have a more expressive feel to it since you are working with an object through a pointer like interface instead of saying:

myints[ i ] = 42;

you can say:

*it = 42;.

However, before you think I'm suggesting using iterators over indices think back to the long list of suggestions a few posts back. Prefer algorithms over raw loops, prefer range based for loops over other loops.

Hopefully I didn't miss anything.
If you think paging some data from disk into RAM is slow, try paging it into a simian cerebrum over a pair of optical nerves. - gameprogrammingpatterns.com

Post Reply