7
\$\begingroup\$

I am building a game where two players have to pronounce name of a city. The main rule is better to explain with the following example:

  • computer starts with "New York"
  • player tells "Kopenhagen", as he needs to tell a name which starts from "K", because "K" is the last letter in "New York"
  • computer tells "Nairobi" and so on and so forth. Any city name cannot be mentioned twice.

When a player doesn't know any city name which starts from the appropriate letter, he leaves the game. The winner is apparently the last man standing.

Below is the code I've written for rules engine of the game. I make it staless, so the previous turns and the entire computer knowledge base caches are AppService layer responsibility. Also there is a need to verify if user doesn't put a name which is not a city at all. Currently I left such a function as input parameter _validateCityExists.

public class GameCitiesRules
{
    private readonly Func<string, bool> _validateCityExists;
    private static IEnumerable<string> _cityNamesKnowledgeBase;
    private static IEnumerable<string> _previousTurns;

    //none word can start with these letters. So if a city name ends with such a letter - we use the second one from the end as target letter  
    private static readonly IEnumerable<char> NonMeaningfulLetters = new List<char>()
        {
            'Ы',
            'Ъ',
            'Ь',
        };

    protected GameCitiesRules(IEnumerable<string> previousTurns,
        IEnumerable<string> cityNamesKnowledgeBase, 
        Func<string, bool> validateCityExists)
    {
        _cityNamesKnowledgeBase = cityNamesKnowledgeBase.Select(c => c.ToUpper(CultureInfo.InvariantCulture));
        _previousTurns = previousTurns.Select(s => s.ToUpper(CultureInfo.InvariantCulture));
        _validateCityExists = validateCityExists;
    }

    public static GameCitiesRules Create(IEnumerable<string> previousTurns,
        IEnumerable<string> cityNamesKnowledgeBase,
        Func<string, bool> validateCityExists)
    {
        Require.ArgNotNull(previousTurns, "previous turns");
        Require.ArgNotNull(cityNamesKnowledgeBase, "city names knowledge base");
        Require.That(() => cityNamesKnowledgeBase.Any(), "knowledge base cannot be empty");
        Require.ArgNotNull(validateCityExists, "city verification function");
        return new GameCitiesRules(previousTurns, cityNamesKnowledgeBase, validateCityExists);
    }

    public void StartNew(bool computerFirst, out string answer)
    {
        Require.That(() => !_previousTurns.Any(), "Game cannot be started twice");

        if (computerFirst)
        {
            var firstWordIndex = new Random((int)DateTime.Now.Ticks)
                                    .Next(_cityNamesKnowledgeBase.Count());
            answer = _cityNamesKnowledgeBase.ElementAt(firstWordIndex);
        }
        else
        {
            answer = string.Empty;
        }
    }

    // can throw GameRuleViolationException, so client can catch and handle it
    public bool TryExecute(string userInput, 
        out string answer)
    {
        Require.NotNullOrWhitespace(userInput, "city name");

        userInput = userInput.ToUpper(CultureInfo.InvariantCulture);

        GameRequire.That(() =>
            {
                if (!_previousTurns.Any())
                {
                    return true;
                }

                var letter = userInput.FirstLetter();
                var desiredLetter = GetLastMeaningfulLetter(_previousTurns.Last());
                return letter == desiredLetter;
            }, 
            "user stated city which starts with wrong letter",
            GamesViolationCodes.CitiesStartsWithWrongLetter);

        GameRequire.That(() => !_previousTurns.Contains(userInput),
            "user stated city which had already been mentioned",
            GamesViolationCodes.CitiesAlreadyMentioned);

        GameRequire.That(() => _validateCityExists(userInput),
            "user stated an unrecognized city",
            GamesViolationCodes.CitiesNotACity);

        var newDesiredLetter = GetLastMeaningfulLetter(userInput).ToString();
        answer = _cityNamesKnowledgeBase.FirstOrDefault(x => x.StartsWith(newDesiredLetter) 
                                            && !_previousTurns.Contains(x)
                                            && !string.Equals(userInput, x));

        return !string.IsNullOrEmpty(answer);
    }

    // returns the current desired letter 
    public string Explain()
    {
        if (_previousTurns.Any())
        {
            return GetLastMeaningfulLetter(_previousTurns.Last()).ToString();
        }

        return string.Empty;
    }

    protected char GetLastMeaningfulLetter(string word)
    {
        return word.Last(letter => !NonMeaningfulLetters.Contains(letter));
    }
}

There are a few helper classes to arrange multiple checks required in the game:

public class GameRequire
{
    public static void That(Func<bool> condition, 
        string errorMesssage,
        GamesViolationCodes violationCode)
    {
        if (condition == null)
        {
            throw new InvalidOperationException("Game rule condition must be set");
        }

        if (!condition())
        {
            throw new GameRulesViolationException(errorMesssage, violationCode);
        }
    }   
}

public enum GamesViolationCodes
{
    CitiesStartsWithWrongLetter = 0,
    CitiesAlreadyMentioned = 1,
    CitiesNotACity = 2,
}

public class Require
{
    public static void That(bool condition, string errorMessage)
    {
        if (!condition)
        {
            throw new DomainRuleViolationException(errorMessage);
        }
    }

    public static void That(Func<bool> condition, string errorMessage)
    {
        if (condition == null)
        {
            throw new InvalidOperationException("Domain rule condition must be set");
        }

        if (!condition())
        {
            throw new DomainRuleViolationException(errorMessage);
        }
    }
}
\$\endgroup\$
3
  • 1
    \$\begingroup\$ I would strongly recommend using Dependency Injection for injecting rules, strategies, game-state repository, and your city repository. Now you can also test these separately. And strongly recommend to work Test-First -> you have pretty specific questions to ask your code, like do I get the proper GamesViolationCodes, proper exceptions, does my game keep state properly, and that just works stupidly much better with UnitTesting. So look into Unity/Ninject for DI, NUnit for testing, and Moq for mocks. \$\endgroup\$
    – Arwin
    Jun 7, 2017 at 6:45
  • \$\begingroup\$ @Artem - where are the GameRulesViolationException and DomainRuleViolationException defined? I'm having trouble compiling. \$\endgroup\$
    – BenKoshy
    Jun 7, 2017 at 8:27
  • \$\begingroup\$ @Artem - this is not compiling. 'CityNamesGame.Require' does not contain a definition for 'argnotnull' there are too many compile errors on my end friend etc etc \$\endgroup\$
    – BenKoshy
    Jun 7, 2017 at 8:32

1 Answer 1

6
\$\begingroup\$
public void StartNew(bool computerFirst, out string answer)

We usually don't use out parameters if it's more appropriate to return a value and it this case it is.


// can throw GameRuleViolationException, so client can catch and handle it
public bool TryExecute(string userInput, out string answer)

This method violates the try-do-something pattern where we do not want a method to throw but instead use the bool result to tell whether it worked or not. Throwing an exception is redundant and makes the bool unnecessary. You should use either one but not both at the same time.


private static IEnumerable<string> _cityNamesKnowledgeBase;
private static IEnumerable<string> _previousTurns;

Why are these non-readonly fields static in a non-static class? You should always avoid static because it's hard to test and debug unless you have a good reason for it.


private static IEnumerable<string> _previousTurns;

Why is this field called previousTurns if it never grows? You never add any new elements to it. I looks more like last-turn. Each call to new GameCitiesRules(previousTurns, ...) resets its content. The same applies to cityNamesKnowledgeBase. Seeing this I doubt that the requirement of preventing same cities works.


if (condition == null)
{
    throw new InvalidOperationException("Game rule condition must be set");
}

This is an ArgumentNullException not an InvalidOperationException which is used for when

The exception that is thrown when a method call is invalid for the object's current state.

but there is no state yet.


public enum GamesViolationCodes
{
    CitiesStartsWithWrongLetter = 0,
    CitiesAlreadyMentioned = 1,
    CitiesNotACity = 2,
}

Error codes are an ancient era. Each of these values should be a separate exception that is easier to catch and analyze and setup and log then enum values.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.