a static class method not returning the right value-CodePudding

The idea of the program is to output the department which has the biggest salary combined by every person working in that department. so I have my program.cs:

string print = string.Empty;
            int n = int.Parse(Console.ReadLine());
            for(int a = 0; a < n; a  )
            {
                string input = Console.ReadLine();
                List<string> inputs = input.Split(" ").ToList();
                if(inputs[4].Contains("@"))
                {
                    Employee info = new Employee(inputs[0], double.Parse(inputs[1]), inputs[2], inputs[3], inputs[4], int.Parse(inputs[5]));
                    print = info.ToString();
                }
                else
                {
                    Employee info = new Employee(inputs[0], double.Parse(inputs[1]), inputs[2], inputs[3], "n/a", int.Parse(inputs[4]));
                    print = info.ToString();
                }
                Employee.Calculation(inputs[3], double.Parse(inputs[1]));
            }
            Console.WriteLine(print);

and part of my Employee.cs, which is the inportant one:

public static void Calculation(string department, double salary)
        {

            Dictionary<string, double> data = new Dictionary<string, double>();
            if (data.ContainsKey(department))
            {
                data[department]  = salary;
            }
            else
            {
                data.Add(department, salary);
            }
            foreach (KeyValuePair<string, double> info in data)
            {
                if (info.Value > biggestSalary)
                {
                    biggestSalary = info.Value;
                    toReturn = info.Key;
                }
            }

        }
        public override string ToString()
        {
            string line1 = "Highest average salary: "   toReturn;
            return line1;


        }

with this input:

4
Pesho 120000 Dev Daskalo [email protected] 28
Toncho 333333.33 Manager Marketing 33
Ivan 15000 ProjectLeader Development [email protected] 40
Gosho 130033333 Freeloader Nowhere 18

the last line is ignored for some reason when I debugged it and it returns the 2nd biggest salary - in department "Marketing". with this input:

6
Stanimir 496.37 Temp Coding [email protected] 50
Yovcho 610.13 Manager Sales 33
Toshko 609.99 Manager Sales [email protected] 44
Venci 0.02 Director BeerDrinking [email protected] 23
Andrei 700.00 Director Coding 45
Popeye 13.3333 Sailor SpinachGroup [email protected] 67

i get "Coding" instead of "Sales". When you combine the 2 people working "Coding" you get 700 496 = 1196. When you combine the 2 people working in "Sales" you get 609 610 = 1219 and then the output should be "Highest average salary: Sales", but instead the output is "Highest average salary: Coding";

CodePudding user response：

You are creating a new dictionary every time the Calculation method is called.

Dictionary<string, double> data = new Dictionary<string, double>();


// The first block is never called as the Dictionary never contains anything at this point. The else block always runs.    
if (data.ContainsKey(department))
{
    data[department]  = salary;
}
else
{
    data.Add(department, salary);
}

There is therefore only ever one value in the dictionary for the one employee that that is meant to be added.

Since an employee with the Coding department has the highest individual value, that is the department that is returned.

Without commenting on the other aspects of the code the way to avoid this issue is to create the Dictionary first outside of the Calculation method.

CodePudding user response：

Assuming that you would add Employee to the List<Employee>, finding this value using LINQ would look like

public class Employee
{
    public string Name { get; set; }
    public string Dept { get; set; }
    public decimal Salary { get; set; }

}

// preload
var empList = new List<Employee>(); 
empList.Add(new Employee(){Name = "A", Salary = 10, Dept = "Sales" });
empList.Add(new Employee(){Name = "B", Salary = 10, Dept = "Coding" });
empList.Add(new Employee(){Name = "C", Salary = 30, Dept = "Sales" });
empList.Add(new Employee(){Name = "D", Salary = 20, Dept = "Coding" });
                
// execute
var topItem =  empList.GroupBy(_ => _.Dept)
    .Select(g => new { D = g.First().Dept, TS = g.Sum(s => s.Salary)})
    .OrderByDescending(item => item.TS)
    .First();

Console.WriteLine($"The department '{topItem.D}' has biggest salary: '{topItem.TS}'");

The department 'Sales' has biggest salary: '40'

Returning to the point that your code structure has a role in your problem. Even if you calculate this in the loop, you still want to accumulate your employees on the class/application scope variable, so you have continuous access to it.

As applicable to your case, your Employee info = new Employee seem not added to any list. And Employee.Calculation does not use any program-lever variable, which would keep the state.

If I wanted to keep this structure, the one you have, I could declare your class like

public class Employee
{

    private static List<Employee> _empList = new List<Employee>(); 

    // your constructor
    public Employee (........)
    {
        // Assign your properties here
        _empList.Add(this);
    }


    // And your `Employee.Calculation` would lose any parameters and look like this 
    public static void Calculation()
    {
        var topItem =  _empList.GroupBy(_ => _.Dept).......
        
        Console.WriteLine($"The department '{topItem.D}' has biggest salary: '{topItem.TS}'");
    }
}

^^^ that is if I really wanted to fix the issue but keep the structure you have already