### Is there a simpler way?

Here is my code:
 ``12345678910111213141516171819202122232425262728293031323334353637383940414243444546474849505152535455565758596061626364656667686970717273747576`` ``````#include #include #include struct Results { /* Declare/Define class members */ std::string name; float totalWeight = 0; int numOfFish = 0, numOfDead = 0, numOfAlive = 0; std::list biggestFish; unsigned int points = 0; unsigned int position; /* Define Comparison Operators */ // Result1 > Result2 friend bool operator > (Results leftSide, Results rightSide) { // Compare the total weights if (leftSide.totalWeight > rightSide.totalWeight) return true; else if (leftSide.totalWeight == rightSide.totalWeight) // Compare the total number of fish if (leftSide.numOfFish > rightSide.numOfFish) return true; else if (leftSide.numOfFish == rightSide.numOfFish) // Compare the number of living fish if (leftSide.numOfAlive > rightSide.numOfAlive) return true; else if (leftSide.numOfAlive == rightSide.numOfAlive) { // Sort list's and flip them leftSide.biggestFish.sort(); rightSide.biggestFish.sort(); leftSide.biggestFish.reverse(); rightSide.biggestFish.reverse(); // Compare the largest fish if (leftSide.biggestFish > rightSide.biggestFish) return true; } return false; } // Result1 < Result2 friend bool operator < (Results leftSide, Results rightSide) { return (rightSide > leftSide); } }; int main() { std::list EventResults; // Create new member Results myResults; myResults.name = "David"; myResults.totalWeight = 20.07; myResults.numOfFish = 5; myResults.biggestFish.push_back(6.3); myResults.numOfDead = 0; myResults.numOfAlive = myResults.numOfFish - myResults.numOfDead; EventResults.push_back(myResults); // Create new member Results myResults1; myResults1.name = "Steve"; myResults1.totalWeight = 20.07; myResults1.numOfFish = 5; myResults1.biggestFish.push_back(6.1); myResults1.biggestFish.push_back(6.2); myResults1.numOfDead = 0; myResults1.numOfAlive = 5; EventResults.push_back(myResults1); // Sort and flip EventResults EventResults.sort(); EventResults.reverse(); int i = 1; for (auto result : EventResults) std::cout << (result.position = (i ++)) << ") Name: " << result.name << "\n"; return 0; }``````

While typing it up, everything makes sense, and it works, so far, anyways. But I was wondering if there is a better way to do the sort algorithm. I am aware that if it makes it through all of the if statements and everything is equal, it'll return false, but as of now, I was informed not to worry about a tie. However, I was also wondering if there was a more simple way of sorting this.
Last edited on
closed account (o1vk4iN6)
I don't think this belongs in a comparison:

 ``1234`` `````` leftSide.biggestFish.sort(); rightSide.biggestFish.sort(); leftSide.biggestFish.reverse(); rightSide.biggestFish.reverse(); ``````

It is modifying the data which one wouldn't think would happen. You can simply do a search and find the largest value, this way not modifying any data, and you can define the comparison as "const".

Also instead of doing a ">" than "==" why not invert your operation so just do "<=". One less comparison for each.

 ``12345678910111213141516171819202122`` `````` // before if (leftSide.totalWeight > rightSide.totalWeight) { return true; } else if (leftSide.totalWeight == rightSide.totalWeight) } // check num of fish etc... } //after if (leftSide.totalWeight <= rightSide.totalWeight) { // check num of fish, etc... } return true; ``````

It's conventional to use just operator<.

The reason is, if you make an object that goes into a sorted containter, it reduces the number operations that you need to implement.

Back to your code. Your class should not implement operator>, it should implement just operator<.

You can do this either by implementing operator< within the class as in:
 ``123`` ``````struct Results { bool operator<(const Results &) const;``````
or an external function that takes the two values:
 `` `` ``bool operator<(const Results &l, const Results &r);``
Please note the use of const & as parameters.

If your if clauses end with a return, you can avoid the else as it's implied.

You can't test floating point number for equality in that way. They're equal if the difference is less than the epsilon for the type.

As as mentioned above, comparison shouldn't change the state of the object being compared. So you shouldn't be sorting in your comparison. In the prototypes above, the objects are const, so we promise not to modify them.
Here is the updated comparison definition:
 ``12345678910111213141516171819`` `````` bool operator < (const Results &r) const { // Compare the total weights if (totalWeight > r.totalWeight) return false; // Compare the total number of fish if (numOfFish > r.numOfFish) return false; // Compare the number of living fish if (numOfAlive > r.numOfAlive) return false; // Sort list's and flip them std::list leftSide(biggestFish); std::list rightSide(r.biggestFish); leftSide.sort(); rightSide.sort(); leftSide.reverse(); rightSide.reverse(); // Compare the largest fish if (leftSide >= rightSide) return false; return true; }``````

Instead of changing the actual list, I changed a temp list. Also, in order to ensure my other conditions are met, instead of making it >=, I needed to do >. It does sort, and I'm kind of disappointed that there is no [] operator for lists. I would love something like a vector/list hybrid that has a sort feature and [] operator. I checked out some of the other containers and alas, there is nothing quite like I want.

kbw wrote:
You can't test floating point number for equality in that way. They're equal if the difference is less than the epsilon for the type.

What do you mean by this? Everything seems to work properly.

Edit: @kbw
When you said `bool operator<(const Results &) const;` did you miss the variable name for the reference? or was that intended? If so, how do you access it? this?
Last edited on
`operator[]` makes no sense for a list since there is no random access.
@Volatile Pulse
 When you said bool operator<(const Results &) const; did you miss the variable name for the reference? or was that intended? If so, how do you access it? this?

There is no need to specify a variable name of a parameter in a function declaration or definition if the parameter is not used. As in function declarations parameters are not used they may be missed.
 You can't test floating point number for equality in that way. They're equal if the difference is less than the epsilon for the type.
Integral types (char, int and so on) have a an exact representation for values, but floating point types don't. So can't test them for equality directly as the value has a certain accuracy. That accuracy is the epsilon.

In C, the values are FLT_EPSILON, DBL_EPSILON, LDBL_EPSILON. As you're using float, your comparison should look like:
 `` `` ``if (fabs(leftSide.totalWeight - rightSide.totalWeight)) < FLT_EPSILON)``
closed account (o1vk4iN6)
 ``12345`` `````` std::list leftSide(biggestFish); std::list rightSide(r.biggestFish); leftSide.sort(); rightSide.sort(); leftSide.reverse(); rightSide.reverse(); ``````

why not:

 ``1234567`` `````` float biggest = *std::max_element( biggestFish.begin(), biggestFish.end() ); for( float & f : r.biggestFish ) if( f > biggest ) // epl return false; ``````

This way you save on allocating memory by creating a copy of the list.
Last edited on
@vlad from moscow
Is the parameter not used though? It is in my function anyways.

@naraku9333
I know, but that's why I said I wanted a something that combined them.

@kbw
I don't believe the float comparison is going to cause an issue with only two point precision, or will it? I started reading about why it must be done, and it looks like the accuracy of comparison is only compromised by the system's process of handling floats anyways. Reading a computer science article, even simple adding and subtracting of floats is prone to errors. How or why would this make it more accurate? And is it needed with small precision?

@xerzi
I need to compare the list because it's possible two people have caught the same sized largest fish, while the second one would determine the difference. Just one more step to increased accuracy.
You can use std::sort on a vector.
Edit: Why do you think you only have two point precision?
Last edited on
That's all the scales for the event have, rather, not two point precision, but the values will only have up to 2 decimal places. And I'll have to look into the sort feature on the vector since I'm more comfortable with them than lists.
closed account (o1vk4iN6)
Yes but creating 2 new lists, sorting than reversing them just to find the largest fish is overkill.
Topic archived. No new replies allowed.