CODEWITHSUNDEEP
c#decimalpercentage
Pelle
Pelle

Reputation: 741

Simplify percentage calculation c#

I have this code, which I believe is self-explanatory (although ugly):

    public decimal Stat
    {
        get
        {
            if (Incorrect == 0)
                return 100;
            decimal x = (decimal)(Correct / Incorrect) / (decimal)(Correct + Incorrect) * 100;
            return x;
        }
    }

Is there a way to make this code prettier?

Upvotes: 1

Views: 517

Answers (4)

Dmitrii Bychenko
Dmitrii Bychenko

Reputation: 186843

You have to use decimal, not int arithmetics. The easiest way is to start formula with decimal 100m:

public decimal Stat
{
    get
    {
        return (Correct + Incorrect == 0)
          ? 100m
          : 100m * Correct / (Correct + Incorrect);
    }
}

Upvotes: 4

Mockingbird
Mockingbird

Reputation: 1031

  1. You can cast only one side of the division, or just multiply it by 100.0 at the beginning (then casing to decimal will not be needed).

  2. You don't have to take care of the edge case of Incorrect == 0, because if it holds, then 100 will be returned (100.0 * X / (X+0) equals 100.0)

    public decimal Stat
{
    get
    {
        return 100m * Correct / (Correct + Incorrect);
    }
}

Upvotes: 2

Mike Zboray
Mike Zboray

Reputation: 40838

Two points,

  1. A percentage correct would be Correct * 100m / (Correct + Incorrect). You've divided by Incorrect again after this. I don't know why that is but it seems wrong.

  2. The result of integer division is another integer. If Correct is 1 and Incorrect is 4 then the result of Correct / Incorrect is 0. Always convert to the floating point types before doing division.

I would rewrite this code like so,

public int Total => Correct + Incorrect;

// renamed "Stat"
public decimal PercentageCorrect => (Correct * 100m) / Total;

Total seems like a useful quantity. Let's just make it a property. Renaming "Stat" makes it obvious what it is. Just reading your code, I had to ask what "Stat" was because it was not obvious what you were trying to do.

Upvotes: 1

Idle_Mind
Idle_Mind

Reputation: 39152

That should probably be:

public decimal Stat
{
    get
    {
        decimal Total = (decimal)(Correct + Incorrect);
        return (decimal)Correct / Total * 100.0M;
    }
}

Upvotes: 0

Related Questions