Board game in c++ using qt

Question

I have been working on a board game in c++ using qt. The game cosist of four palyer with four different colors. I am looking forward to improving my board looking.

This is all working fine, but I want to know if there is any other way to do this?

"Readable online" The code for this part.

 std::vector<std::pair<char,char> > directions{std::make_pair(1,-1),std::make_pair(1,1),std::make_pair(-1,1),std::make_pair(-1,-1) };
 for(size_t d =0; d < directions.size(); ++d){
 for(int i=0; i<5;++i){
 if(d % 2 == 0)
 x_pos += directions[d].first * offset;
 else
 y_pos += directions[d].second * offset;
 fieldPos.push_back(QPointF(x_pos,y_pos));
 }
 x_pos += directions[d].first * small_offset;
 y_pos += directions[d].second * small_offset;
 for(int i=0; i<5;++i){
 fieldPos.push_back(QPointF(x_pos,y_pos));
 if(d % 2 == 0)
 y_pos += directions[d].second * offset;
 else
 x_pos += directions[d].first * offset;
 }
 for(int i=0; i<2;++i){
 fieldPos.push_back(QPointF(x_pos,y_pos));
 if(d % 2 == 0)
 x_pos += directions[d].first * large_offset;
 else
 y_pos += directions[d].second * large_offset;
 }
 fieldPos.push_back(QPointF(x_pos,y_pos));
 }

I also wonder how to get my board to look like the (rounded )one on the right. That would be very big help for me :)

enter image description here

• Mast: Did you fix the problems mentioned here?
• Programmer: Yes this is different
• Programmer: @Carcigenicate No not at all
• user1118321: Could you describe what it is this code is supposed to do? Other than pushing some points into a vector, it's not obvious what the significance of the various loops is.
• Programmer: @user1118321 i wanted to get my code reviewed in general. this part of my code handles the small circles, so that they goe around the corss. I wanted to get them if its posiible to go around a big rounded ellipse as shown at the end of the post. You may check my full code from readable online

Answer 1

As mentioned in my comment, it's not entirely clear what the purpose of this code is, beyond adding points to a vector. Given that, I see several things that I think could make this more clear.

Use Types

C++ is a strongly typed language and that not only gives you type safety, it also improves comprehension. The ultimate goal of this code appears to be to add several QPointfs to an array. But it's written with 3 different types to accomplish that job - whatever type x_pos and y_pos are (presumably float or double?), chars in the directions vector, and QPointf in the fieldPos vector. This is inefficient as the compiler will end up inserting a bunch of code to convert between types, but more importantly, it obscures what's happening.

I would declare directions like this:

std::vector<QPointf> directions { 
 QPointf( 1.0, -1.0), 
 QPointf( 1.0, 1.0), 
 QPointf(-1.0, 1.0), 
 QPointf(-1.0, -1.0) 
};

I would change x_pos and y_pos into a single QPointf. It implements operator+=() so you can work in a similar manner as you are currently.

In general, when you find yourself using std::pair, it's a good sign that you need to use a better type like some sort of point, (mathematical) vector, or size struct. Code that uses std::pair is very difficult to read because the members, first and second are meaningless names for whatever task your code is performing.

Put the Right Things Into the Array

It might be more clear to just make a longer array with the directions you actually want instead of all the various loops. For example, instead of cramming everything into 4 QPointfs and looping over them and flipping back and forth between x and y positions, just make the array have what you want in it:

std::vector<QPointf> directions { 
 QPointf( 1.0, 0.0), 
 QPointf( 0.0, -1.0), 
 QPointf( 1.0, 0.0), 
 QPointf( 0.0, -1.0),
 // etc.
};

This will eliminate the need for multiple loops and the need to check even or odd iterations, which allows you to do the following.

Use Range-based Looping

Rather than writing a loop based on the size of the directions array and then having magic numbers like 5 in your inner loops, you can use the new C++11 and later syntax for range-based looping:

for (auto nextPoint : directions)
{
 pos += nextPoint * offset;
 fieldPos.push_back(pos);
}

But of course, you're not multiplying them all by offset. So your vector may need to contain objects of a new type in it that's something like:

typedef struct Direction {
 QPointf direction;
 float offset;
} Direction;

I'd use better names than Direction and direction, but since I don't know what they represent, I can't. If it's for a player, then PlayerDirection, or whatever's appropriate. So the loop would be more like:

for (auto nextDir : directions)
{
 pos += nextPoint.direction * nextPoint.offset;
 fieldPos.push_back(pos);
}

It means you'll end up having a long declaration for the array, but it will make the code a lot simpler. You could even put the values into a data file and read them at runtime if you think they'll change in the future.