Schedule matches for a season or round robin tournament

Question

The algorithm comes from wiki.

The output from the algorithm is correct but not something that a person could (easily) understand. See the wiki link above.

Each team play every other team. Optionally play every team twice and reverse home / away.

Assume an even number of teams. I need to add a test for even.

Please review for style and any other advice.

public struct Match
{
    public int IDhome { get; }
    public int IDaway { get; }
    public int Week { get; }
    public override string ToString()
    {
        return $"week {Week}  IDhome {IDhome}  IDaway {IDaway}";
    }
    public Match(int idHome, int idAway, int week)
    {
        IDhome = idHome;
        IDaway = idAway;
        Week = week;
    }
}
public struct Season
{
    public int[,] ScheculeRaw { get; }
    int TeamCount { get; }
    public List<Match> Matches { get; }
    public override string ToString()
    {
        StringBuilder sb = new StringBuilder();
        foreach (Match m in Matches)
        {
            sb.AppendLine(m.ToString());
        }
        return sb.ToString();
    }
    public Season(int teamCount, bool awayHome = false)
    {
        ScheculeRaw = RoundRobinRaw(teamCount);
        TeamCount = teamCount;
        Matches = new List<Match>();
        for (int w = 0; w < TeamCount - 1; w++)
        {
            for (int p = 0; p < TeamCount/2; p++)
            {
                Matches.Add(new Match(ScheculeRaw[w, p] + 1, ScheculeRaw[w, teamCount - 1 - p] + 1, w + 1));
            }
        }
        if (awayHome)
        {
            for (int w = 0; w < TeamCount - 1; w++)
            {
                for (int p = 0; p < TeamCount / 2; p++)
                {
                    Matches.Add(new Match(ScheculeRaw[w, teamCount - 1 - p] + 1, ScheculeRaw[w, p] + 1, w + teamCount));
                }
            }
        }
    }

    public static int[,] RoundRobinRaw(int count)
    {
        int[] rr = new int[count];
        for (int i = 0; i < count; i++)
        {
            rr[i] = i;
        }
        int[,] sch = new int[count - 1, count];
        for (int w = 0; w < count - 1; w++)
        {
            for (int i = 0; i < count; i++)
            {
                sch[w, i] = rr[i];
            }

            //Debug.WriteLine("");
            //Debug.WriteLine("ww1");
            //for (int p = 0; p < count; p += 2)
            //{
            //    Debug.Write(sch[w, p] + ", ");
            //}
            //Debug.WriteLine("");
            //Debug.WriteLine("ww2");
            //for (int p = count - 1; p > 0; p -= 2)
            //{
            //    Debug.Write(sch[w, p] + ", ");
            //}

            // rotate rr 
            int temp = rr[count - 1];
            for (int r = count - 2; r > 0; r--)
            {
                rr[r + 1] = rr[r];
            }
            rr[1] = temp;

            //Debug.WriteLine("");
            //Debug.WriteLine("rr");
            //for (int r = 0; r < count; r++)
            //{
            //    Debug.Write(rr[r] + ", ");
            //}
            //Debug.WriteLine(".");
        }
        return sch;
    }
}

Answer 1

• Use readable variable names. I can guess at the meaning of sch (schedule?) and rr (round robin), but I'm at a loss for w (week?) and p. There's no reason to shorten the names, it does not affect the execution of the code; it does affect the readability. Everything might make sense to you today, but not to other people, or you in the future.
• Using arrays versus lists is (at least partially) subjective), but I do think that you should be using lists here. It keeps the syntax cleaner and doesn't require readers to keep track of the array indexes.
  • The for loops are adding so much to the complexity. w < TeamCount - 1 and p < TeamCount / 2 are not easy to understand the intention. If you stick to using lists, that guides you towards using a more readable foreach statement, e.g. foreach(week in weeks)
• bool awayHome = false is a very confusing name. A better name would be generateReverseMatch (or whatever designated name you prefer)
• Personally, I like to keep the constructor focused on property/field assignment. I would abstract the match generation into a private method, which can still be called from the constructor: Matches = GenerateMatches(teamCount);
• While not wrong per se, be on the lookout of overzealously using ToString() instead of a nicer PrintMatches() method. Most of the time, you want a custom layout, especially if you want to have different ways of showing the same season.

Switching the code over to using lists instead of arrays is going to have a strong impact on the posted code, so I'm not sure if further code review is going to be relevant here.