What's wrong with this code?

Recently went to a job interview and I have been told that the below solution is not good. Unfortunately they did not give me a feedback. Can anyone please review my code and see anything obviously wrong here?

Interview Question:

The file hockey.csv contains the results from the Hockey Premier League. The columns 'F' and 'A' contain the total number of goals scored for and against each team in that season (so Alabama scored 79 goals against opponents, and had 36 goals scored against them).

Write a program to print the name of the team with the smallest difference in 'for' and 'against' goals.

hockey.csv looks like this (sorry about the column headings)

Team	P	W	L	D	F	-	A	Pts
Alabama	38	26	9	3	79	-	36	87
Washinton	38	24	8	6	67	-	30	80
Indiana	38	24	5	9	87	-	45	77
Newcastle	38	21	8	9	74	-	52	71
Florida	38	18	12	8	53	-	37	66
Chelsea	38	17	13	8	66	-	38	64
West_Ham	38	15	8	15	48	-	57	53
New York	38	12	14	12	46	-	47	50
Sunderland	38	10	10	18	29	-	51	40
-------------------------------------------------------
Lova	38	9	9	20	41	-	64	36
Nevada	38	8	6	24	33	-	63	30
Boston	38	5	13	20	30	-	64	28

My solution:

The console App:

class Program

{

static void Main(string[] args)

{

string path = @"C:\....;

var resultEvaluator = new ResultEvaluator(string.Format(@"{0}\{1}",path, "football.csv"));

var team = resultEvaluator.GetTeamSmallestDifferenceForAgainst();

Console.WriteLine(

string.Format("Smallest difference in 'for' and 'against' goals > TEAM: {0}, GOALS DIF: {1}",

team.Name, team.Difference ));

Console.ReadLine();

}

public class Team

{

public Team(string name, int against, int @for)

{

Name = name;

Against = against;

For = @for;

}

public string Name { get; private set; }

public int Against { get; private set; }

public int For { get; private set; }

public int Difference

{

get { return (For - Against); }

}

public interface IResultEvaluator

{

Team GetTeamSmallestDifferenceForAgainst();

}

public interface ICsvExtractor

{

DataTable GetDataTable(string csvFilePath);

}

public class ResultEvaluator : IResultEvaluator

{

private static DataTable leagueDataTable;

private readonly string filePath;

private readonly ICsvExtractor csvExtractor;

public ResultEvaluator(string filePath){

this.filePath = filePath;

csvExtractor = new CsvExtractor();

}

private DataTable LeagueDataTable{

get

{

if (leagueDataTable == null)

{

leagueDataTable = csvExtractor.GetDataTable(filePath);

}

return leagueDataTable;

}

public Team GetTeamSmallestDifferenceForAgainst() {

var teams = GetTeams();

var lowestTeam = teams.OrderBy(p => p.Difference).First();

return lowestTeam;

}

private IEnumerable GetTeams() {

IList list = new List();

foreach (DataRow row in LeagueDataTable.Rows)

{

var name = row["Team"].ToString().Remove(0, 3).Trim();

var @for = int.Parse(row["F"].ToString());

var against = int.Parse(row["A"].ToString());

var team = new Team(name, against, @for);

list.Add(team);

}

return list;

}

public class CsvExtractor : ICsvExtractor

{

public DataTable GetDataTable(string csvFilePath)

{

var lines = File.ReadAllLines(csvFilePath);

string[] fields;

fields = lines[0].Split(new[] { ',' });

int columns = fields.GetLength(0);

var dt = new DataTable();

//always assume 1st row is the column name.

for (int i = 0; i < columns; i++)

{

dt.Columns.Add(fields[i].ToLower(), typeof(string));

}

DataRow row;

for (int i = 1; i < lines.GetLength(0); i++)

{

fields = lines[i].Split(new char[] { ',' });

if (fields.Count() == 1) // to elimate the bad data : temp solution

{

continue;

}

row = dt.NewRow();

for (int f = 0; f < columns; f++)

row[f] = fields[f];

dt.Rows.Add(row);

}

return dt;

}

The interviewer was only interested in the structure/design of the code and whether I can produce the correct result (i.e lowest difference). Can anyone please tell me any issues in the solution above? Much appreciated.

1 Reply

Know the answer? Post it — somebody with the same question will find it here.

Roy S14y ago
When you calculate the difference between for and against, you simple subtract them.

So if a team has 50 for and 59 against, that gives 59 - 50 = +9. Another team has 0 for and a million against, would give -1,000,000. In this case -1,000,000 is smaller than +9 but that doesn't mean that the second team has a lower difference.

In short: when getting the difference take the absolute value of it.

So instead of:
public int Difference
{
get { return (For - Against); }
}

Try this:
public int Difference
{
get { return Math.Abs(For - Against); }
}

This will produce the correct answer that team York has the least difference (of 1).