Page 2 of 2

Re: Daily Learner

Posted: March 14th, 2018, 1:16 am
by chili
don't use swap
just put the dice in containers m8! then you can sort

also, it seems that you just select a total number of attacking armies at the beginning and then the battle runs until out of armies, but you should be able to chose 1-3 every round of attacking (and defender chooses 1-2)

Re: Daily Learner

Posted: March 14th, 2018, 2:29 am
by chili
https://onlinegdb.com/B1fCdbUFz

Here is a procedural implementation that makes use of the std lib. As you can see, the combat calculation code is concise and clear when you make proper use of containers and algorithms.

Re: Daily Learner

Posted: March 14th, 2018, 4:43 pm
by Zedtho
Thanks a lot for helping! Yeah, looking at that code, mine really looks like a dump compared to what it could be like. I'll be sure to fix it this Friday when I get time :)

Re: Daily Learner

Posted: March 14th, 2018, 5:29 pm
by albinopapa
Hey Zedtho, I don't think Chili was trying to discourage you or your code, but used it as a teaching moment about the STL and clean coding practices. This is somewhat important if you plan on joining a company or code in a group or team. You want to make sure your code is easily readable by anyone.

For instance, one of the hardest things for me to reason about your code here and in your game project, is your if/else blocks.

Code: Select all

// You do something like
if( value == 0 )
{
     return 3;
}
else
     if( condition == 1 )
     {
          return 9;
     }
     else
          if( condition == 2 )
          {
               return 0;
          }
While it is true that the next command after an if or else statement is executed without the need for the curly braces, to me it only makes sense to use that feature if you truly only have one more command.

Code: Select all

if( value == 0 )
     return 3;
else if( value == 1 )
     return 9;
else if( value == 2 )
     return 0;
else 
     throw std::runtime_error("Invalid value.")
The reason I bring this up is because it is hard to reason from your code if the dangling if block after the else is suppose to be just an 'else' or an 'else if'. After looking back at the code you posted, it looks more like:
if( ... )
{
}else
if( ... )
{
}else
if( ... )
The dangling 'if' that I refer to is because of the way I have my VS setup to format code and automatically indent, so it appears as I described when I copy paste code into a project.

This still makes me wonder why not just use else if. This is even harder to reason about without the visual cues of nesting with indentation.

Re: Daily Learner

Posted: March 15th, 2018, 3:12 pm
by Zedtho
Yeah I see what you mean, I'll be sure to do that in future projects! Thanks for the feedback :)

Re: Daily Learner

Posted: March 15th, 2018, 4:07 pm
by chili
Yup, not trying to shit on anyone. Just wanna get more ppl into modern C++ is all :)