Is there a simpler way?

[Volatile Pulse]

Here is my code:

#include <iostream>
#include <string>
#include <list>

struct Results {
   /* Declare/Define class members */
   std::string name;
   float totalWeight = 0;
   int numOfFish = 0, numOfDead = 0, numOfAlive = 0;
   std::list<float> 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<Results> 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.

[o1vk4iN6]

I don't think this belongs in a comparison:

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.

// 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;

[kbw]

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:

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.

[Volatile Pulse]

Here is the updated comparison definition:

   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<float> leftSide(biggestFish); std::list<float> 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?

[naraku9333]

operator[] makes no sense for a list since there is no random access.

[vlad from moscow]

@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.

[kbw]

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)

[o1vk4iN6]

std::list<float> leftSide(biggestFish); std::list<float> rightSide(r.biggestFish);
leftSide.sort(); rightSide.sort();
leftSide.reverse(); rightSide.reverse();

why not:

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.

[Volatile Pulse]

@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.

[naraku9333]

You can use std::sort on a vector.
Edit: Why do you think you only have two point precision?

[Volatile Pulse]

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.

[o1vk4iN6]

Yes but creating 2 new lists, sorting than reversing them just to find the largest fish is overkill.