HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Simplifying `if` statements in Tic Tac Toe function

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
toestatementsticfunctiontacsimplifying

Problem

I'm currently trying to improve my coding skills, so I'm trying to code a Tic Tac Toe game. At one point a "win check" is needed.
I'm using this code to check:

public void CheckWin(States state)
{   
    if(fieldA1.GetState == state && fieldA2.GetState == state && fieldA3.GetState == state)
        ReportWin(currentPlayer);
    else if(fieldB1.GetState == state && fieldB2.GetState == state && fieldB3.GetState == state)
        ReportWin(currentPlayer);
    else if (fieldC1.GetState == state && fieldC2.GetState == state && fieldC3.GetState == state)
        ReportWin(currentPlayer);
    else if (fieldA1.GetState == state && fieldB1.GetState == state && fieldC1.GetState == state)
        ReportWin(currentPlayer);
    else if (fieldA2.GetState == state && fieldB2.GetState == state && fieldC2.GetState == state)
        ReportWin(currentPlayer);
    else if (fieldA2.GetState == state && fieldB2.GetState == state && fieldC2.GetState == state)
        ReportWin(currentPlayer);
    else if (fieldA3.GetState == state && fieldB3.GetState == state && fieldC3.GetState == state)
        ReportWin(currentPlayer);
    else if (fieldA1.GetState == state && fieldB2.GetState == state && fieldC3.GetState == state)
        ReportWin(currentPlayer);
    else if (fieldA3.GetState == state && fieldB2.GetState == state && fieldC1.GetState == state)
        ReportWin(currentPlayer);
    else
        PlayerSwitch();
}


This code looks very unclean to me. Is there a better way to handle these if instructions?

Solution

There should only be eight ways to win; you have nine ReportWin()s. This one got listed twice:

fieldA2.GetState == state && fieldB2.GetState == state && fieldC2.GetState == state


Considering that you've named your squares as independent variables, there's not much you can do to generalize CheckWin(). Start by turning the board into an array or two-dimensional array, then read the advice in Tic-Tac-Toe design . Personally, I like the suggestion to list all eight winning configurations, as it results in the least code.

Code Snippets

fieldA2.GetState == state && fieldB2.GetState == state && fieldC2.GetState == state

Context

StackExchange Code Review Q#37891, answer score: 8

Revisions (0)

No revisions yet.