Skip to main content
added 322 characters in body
Source Link
janos
  • 113.1k
  • 15
  • 154
  • 396

Or actually, it's wasteful to sort the vector twice, better sort once and save in a temp variable:

std::vector<T> temp = this->sorted();
return (temp[s / 2 - 1] + temp[s / 2]) / 2;

Now that I look at this cleaner version, it's clear that this won't work if the vector is empty, because if s == 0 this will end up referencing temp[-1]. This was not so easy to see in the original version, now it's obvious.

This was just the worst example I found, but in many many places you use far more parentheses than you need. I suggest to review the entire code and trim a little bit.

This was just the worst example I found, but in many many places you use far more parentheses than you need. I suggest to review the entire code and trim a little bit.

Or actually, it's wasteful to sort the vector twice, better sort once and save in a temp variable:

std::vector<T> temp = this->sorted();
return (temp[s / 2 - 1] + temp[s / 2]) / 2;

Now that I look at this cleaner version, it's clear that this won't work if the vector is empty, because if s == 0 this will end up referencing temp[-1]. This was not so easy to see in the original version, now it's obvious.

This was just the worst example I found, but in many many places you use far more parentheses than you need. I suggest to review the entire code and trim a little bit.

added 322 characters in body
Source Link
janos
  • 113.1k
  • 15
  • 154
  • 396

This was just the worst example I found, but in many many places you use far more parentheses than you need. I suggest to review the entire code and trim a little bit.

Also try to put spaces around operators like I did in this example.

This was just the worst example I found, but in many many places you use far more parentheses than you need. I suggest to review the entire code and trim a little bit.

Also try to put spaces around operators like I did in this example.

added 322 characters in body
Source Link
janos
  • 113.1k
  • 15
  • 154
  • 396

Excessive parentheses

The too many parantheses are seriously hurting readability here:

return ((this->sorted()[(s/2)-1]+this->sorted()[(s/2)])/2);

And the lack of spaces around operators don't help!

This should have been:

return (this->sorted()[s / 2 - 1] + this->sorted()[s / 2]) / 2;

Placement of curly braces

You're placing curly braces inconsistently. Sometimes you use put the opening brace on the same line as the statement, like this:

double sum(){
    // ...
    while (l<s) {
        // ...
    }
}

Other times you place opening brace on the next line, like this:

if (s%2==0)
{
    // ...
}
else
{
    // ...
}

I suggest to pick either of these styles and stick to it. (I prefer the first.)

Excessive parentheses

The too many parantheses are seriously hurting readability here:

return ((this->sorted()[(s/2)-1]+this->sorted()[(s/2)])/2);

And the lack of spaces around operators don't help!

This should have been:

return (this->sorted()[s / 2 - 1] + this->sorted()[s / 2]) / 2;

Placement of curly braces

You're placing curly braces inconsistently. Sometimes you use put the opening brace on the same line as the statement, like this:

double sum(){
    // ...
    while (l<s) {
        // ...
    }
}

Other times you place opening brace on the next line, like this:

if (s%2==0)
{
    // ...
}
else
{
    // ...
}

I suggest to pick either of these styles and stick to it. (I prefer the first.)

Source Link
janos
  • 113.1k
  • 15
  • 154
  • 396
Loading