A logic bug in my C# code, what should I do?

Question

I have a logical error. I provided the following as input:

• the salary is 30000
• the child n° is 9

So the the net salary will be:

• the family bonus + salary - tax

   (750)       + (30000) - (3000)
  

• but my program count them as

    (1500)    + (30000) + (6000)
  

My program doubled (accumulated) the family bonus and the tax. Can anyone explain why?

class Program
{
    static void Main(string[] args)
    {
        Employee e = new Employee();
        e.ReadEmployee();
        e.PrintEmployee();
    }
}

class Employee
{
    private string n;
    private int byear;
    private double sal;
    private bool gen;
    private bool mar;
    private int child;
    public static double tax = 0;
    public static double familybonus = 0;
    public string Ename
    {
        get { return this.n; }
        set
        {
            this.n = value;

        }
    }
    public int Birthyear
    {
        get { return this.byear; }
        set
        {
            if (value >= 1970 && value <= 1990) this.byear = value;
            else this.byear = 0;
        }
    }
    public double Salary
    {
        get { return this.sal; }
        set
        {
            if (value >= 5000 && value <= 50000) this.sal = value;
            else this.sal = 0;
        }
    }

    public bool Gender
    {
        get { return this.gen; }
        set { this.gen = value; }
    }
    public bool Married
    {
        get { return this.mar; }
        set { this.mar = value; }
    }
    public int NChildren
    {
        get { return this.child; }
        set
        {
            if (value >= 0 && value <= 12) this.child = value;
            else this.child = 0;
        }
    }

    public double getAge()
    {
        return 2008 - this.Birthyear;
    }
    public double getNet()
    {
        double net = getFamilyBonus() + this.Salary - getTax();
        return net;
    }

    public double getFamilyBonus()
    {

        if (this.Married == true)
            familybonus += 300;
        if (this.NChildren == 1) familybonus += 200;
        else if (this.NChildren == 2) familybonus += 350;
        else if (this.NChildren >= 3) familybonus += 450;
        return familybonus;
    }

    public double getTax()
    {
        if (Salary < 10000)
            tax = 0;
        if (Salary <= 10000 && Salary >= 20000)
            tax += Salary * 0.05;
        else tax += Salary * 0.1;
        return tax;

    }

    public void ReadEmployee()
    {
        Console.Write("Enter Employee Name: ");
        Ename = Console.ReadLine();
        Console.Write("Enter Employee birth date: ");
        Birthyear = int.Parse(Console.ReadLine());

        while (Birthyear < 1970 || Birthyear > 1990)
        {
            Console.WriteLine("Invalid Birthyear!");
            Console.Write("Enter Employee Birth date: ");

            Birthyear = int.Parse(Console.ReadLine());
        }
        string g = null;
        while (g != "M" && g != "m" && g != "F" && g != "f")
        {
            Console.Write("Enter Employee Gender (M/F)");
            g = Convert.ToString(Console.ReadLine());
        }

        if (g == "M" || g == "m")
            Gender = true;
        else
            Gender = false;
        Console.Write("Enter Employee Salary: ");
        Salary = Double.Parse(Console.ReadLine());
        while (Salary < 5000 || Salary > 50000)
        {
            Console.WriteLine("Invalid Salary!");
            Console.Write("Enter Employee Salary: ");
            Salary = int.Parse(Console.ReadLine());
        }
        string m = null;
        while (m != "true" && m != "True" && m != "false" && m != "False")
        {
            Console.Write("Married (true/false)");
            m = Console.ReadLine();

        }
        if (m == "true")
            this.Married = true;
        else
            this.Married = false;

        Console.Write("Enter Employee Children count: ");
        NChildren = int.Parse(Console.ReadLine());

        while (NChildren < 0 || NChildren > 12)
        {
            Console.WriteLine("Invalid NChildren!");
            Console.Write("Enter Employee Children count: ");

            NChildren = int.Parse(Console.ReadLine());
        }

    }
    public void PrintEmployee()
    {
        Console.Write("Hello ");
        {
            if (Gender == true)
                Console.Write("Mr. ");
            else
                Console.Write("Mrs. ");
            Console.WriteLine(Ename);
        }
        Console.WriteLine("You are {0} years old", getAge());
        Console.WriteLine("Salary= {0}", Salary);
        Console.WriteLine("Tax= {0}", getTax());
        Console.WriteLine("Family bonus= {0}", getFamilyBonus());
        Console.WriteLine("Net= {0}", getNet());
    }
}

Answer 1

I took the existing code, and hard-wired the inputs (rather than using Console.ReadLine()), I get:

You are 28 years old Salary= 30000 Tax= 3000 Family bonus= 750 Net= 25500

The main problem seems to be not initializing values - i.e. treating fields as variables:

public double getTax()
{
    if (Salary < 10000)
        tax = 0;
    if (Salary <= 10000 && Salary >= 20000)
        tax += Salary * 0.05;
    else tax += Salary * 0.1;
    return tax;
}

OK - and what does tax start at if Salary >= 10000, etc. Likewise familyBouns in getFamilyBonus. By the way, how can Salary be both <= 10000 and >= 20000?

To illustrate, I've changed the output to:

    Console.WriteLine("Tax= {0}", getTax());
    Console.WriteLine("Tax= {0}", getTax());
    Console.WriteLine("Tax= {0}", getTax());

Which shows:

Tax= 3000 Tax= 6000 Tax= 9000

My advice would be: don't store calculated values unless you know the math is so complex that it is worth it. Just calculate them as needed (no field at all).

Answer 2

Another problem seems to lie in the fact that you don't initialize familybonus when you say familybonus += 300. So everytime you call GetFamilybonus it's added to the previous result. You call it twice in the PrintEmployee function, once directly and once indirectly by calling getNet;