5
\$\begingroup\$

I was inspired by Number Wizard game question and I decided to write my own version of the game. My goals were to follow the SOLID principles and make the logic UI independent. I'm a bit confused about the outcome, though.

I think my solutions reads much easier than the version in original question, but on the other hand it's also much more complex on the design side. Have I over complicated things?

Interfaces:

namespace NumberWizard.Api
{
    public interface IMessenger
    {
        IEnumerable<string> BeforeStateEnterMessages();
        IEnumerable<string> InStateMessages();
    }
}

namespace NumberWizard.Api
{
    public interface IState
    {
        void Increase();
        void Decrease();
        int Value { get; }

    }
}

Useful Abstractions:

using NumberWizard.Api;
using System;

namespace NumberWizard
{
    public class SteppedValueChanger
    {
        private int _value = 0;
        private readonly int _step;
        public SteppedValueChanger(int step)
        {
            _step = step;
        }


        public void Increase()
        {
            _value += _step;
        }

        public void Decrease()
        {
            _value = Math.Max(0, _value - _step);
        }

        public int Value
        {
            get { return _value; }
        }
    }
}

namespace NumberWizard
{
    public class Range
    {
        public int Min { get; private set; }
        public int Max {get; private set;}

        public Range() : this(0, 0)
        {

        }

        public Range(int min, int max)
        {
            Min = min;
            Max = max;
        }

        internal void SetMin(int min)
        {
            Min = min;
        }

        internal void SetMax(int max)
        {
            Max = max;
        }
    }
}

States:

using NumberWizard.Api;
using System.Collections.Generic;

namespace NumberWizard.State
{

    class SetupMax : IState, IMessenger
    {
        private readonly SteppedValueChanger _valueModifier = new SteppedValueChanger(100);

        public IEnumerable<string> BeforeStateEnterMessages()
        {
            return new[] { "Welcome to Number Wizard", "Set max number!" };
        }       

        public IEnumerable<string> InStateMessages()
        {
            return new[] { string.Format("Max: {0}", Value) };
        }

        public void Increase()
        {
            _valueModifier.Increase();
            Value = _valueModifier.Value;
        }

        public void Decrease()
        {
            _valueModifier.Decrease(); 
            Value = _valueModifier.Value;
        }

        public int Value { get; private set; }
    }
}

using NumberWizard.Api;
using System.Collections.Generic;

namespace NumberWizard.State
{
    class SetupMin : IState, IMessenger
    {
        private readonly SteppedValueChanger _valueModifier = new SteppedValueChanger(1);

        public IEnumerable<string> BeforeStateEnterMessages()
        {
            return new[] { "Set min number!" };
        }        

        public IEnumerable<string> InStateMessages()
        {
            return new[] { string.Format("Min: {0}", Value) };
        }

        public void Increase()
        {
            _valueModifier.Increase(); 
            Value = _valueModifier.Value;
        }

        public void Decrease()
        {
            _valueModifier.Decrease();
            Value = _valueModifier.Value;
        }

        public int Value { get; private set; }
    }
}



using NumberWizard.Api;
using System.Collections.Generic;

namespace NumberWizard.State
{
    class Guess : IState, IMessenger
    {
        private Range _guessingRange;
        private int _guess;

        public Guess(Range range)
        {
            _guessingRange = range;
            _guess = GuessNext();
        }
        public void Increase()
        {
            _guessingRange = new Range(_guess, _guessingRange.Max);
            _guess = GuessNext();           
        }

        public void Decrease()
        {
            _guessingRange = new Range(_guessingRange.Min, _guess);
            _guess = GuessNext();

        }

        private int GuessNext()
        {
           return (_guessingRange.Max + _guessingRange.Min) / 2;
        }


        public IEnumerable<string> BeforeStateEnterMessages()
        {
            return new[]{
                "Pick a number in your head, but don't tell me!",
                string.Format("The highest number you can pick is {0}", _guessingRange.Max),
                string.Format("The lowest number you can pick is {0}", _guessingRange.Min),
                string.Format("Is the number higher or lower than {0}?", Value) 
            };
        }       

        public IEnumerable<string> InStateMessages()
        {
            return new[] { 
                string.Format("Is the number higher or lower than {0}?", Value) };
        }

        public int Value { get { return _guess; } }
    }
}

'State Machine':

using NumberWizard.Api;
using NumberWizard.State;
using System.Collections.Generic;
using System.Linq;

namespace NumberWizard
{
    public class Game
    {       
        private Range guessingRange = new Range();       

        private IState _gameState;

        public IEnumerable<string> Decrease()
        {
            _gameState.Decrease();
            return GetInStateMessages(_gameState);
        }

        public IEnumerable<string> Increase()
        {
            _gameState.Increase();
            return GetInStateMessages(_gameState);
        }

        private IEnumerable<string> GetInStateMessages(IState gameState)
        {
            var messenger = gameState as IMessenger;
            if (messenger == null) { return Enumerable.Empty<string>(); }

            return messenger.InStateMessages();
        }

        private IEnumerable<string> GetBeforeStateEnterMessages(IState gameState)
        {
            var messenger = gameState as IMessenger;
            if (messenger == null) { return Enumerable.Empty<string>(); }

            return messenger.BeforeStateEnterMessages();
        }

        public IEnumerable<string> ChangeState()
        {
            if (_gameState == null)
            {                
                _gameState = new SetupMax();

                return GetBeforeStateEnterMessages(_gameState);
            }
            if (_gameState is SetupMax)
            {
                guessingRange.SetMax(_gameState.Value);
                _gameState = new SetupMin();                

                return GetBeforeStateEnterMessages(_gameState);
            }
            if (_gameState is SetupMin)
            {
                guessingRange.SetMin(_gameState.Value);
                _gameState = new Guess(guessingRange);

                return GetBeforeStateEnterMessages(_gameState);
            }
            if (_gameState is Guess)
            {
                _gameState = null;
                return new[] { "I guessed Correctly!", "Press 'c' to start over or 'q' to quit" };
            }
            return Enumerable.Empty<string>();
        }       
    }
}

Main:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace NumberWizard
{
    class Program
    {
        static void Main(string[] args)
        {
            var game = new Game();
            IEnumerable<string> messages = game.ChangeState();
            PrintMessages(messages);
            var key = Console.ReadKey(true);
            while (true)
            {
                switch (key.KeyChar)
                {
                    case 'u':
                        messages = game.Increase();                        
                        break;
                    case 'd':
                        messages = game.Decrease();
                        break;
                    case 'c':
                        messages = game.ChangeState();
                        break;
                    case 'q':
                        Environment.Exit(0);
                        break;
                }
                PrintMessages(messages);

                key = Console.ReadKey(true);                
            }
        }

        private static void PrintMessages(IEnumerable<string> messages)
        {
            foreach (var m in messages)
            {
                Console.WriteLine(m);
            }
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$

You're asking yourself the right questions about over complicating things. True for most small challenges the solution will often be small, after all not many want to build a enterprise application as a "fun challenge". Many of those challenges can be used as opportunities to try out a new idea, or improve as a developer in ways that you don't get while working on production code. So on to the code. The minor things that I saw were inconsistent Access modifiers. your Range you mark Min and Max as private set, and yet you made SetMin and Max internal. The class is public though. If you were making tests that used Range you'd only be able to create Range once, and not change the values. Probably not a horrible by-product but also seems strange. To that point why do you create a new range in Guess when you can just set the min and max?

I think you might have a violation of the L in your code in both SetupMax and SetupMin. You have a class that has same methods as IState, yet does not implement IState. Granted it doesn't seem like it should, but it has the same methods and you have a direct implementation of it in both without the ability to put in a custom one.

Overall I think you did a good job. I would recommend writing some tests to make sure that the logic you wish is working as intended. Granted the SOLID principles don't speak about writing tests, but they do make writing tests easier. Tests are a fast way to verify that your code is working as intended and it will help you ask the questions that you need to ask to stay with the SOLID principles.

\$\endgroup\$
  • \$\begingroup\$ Thank you, @Robert Snyder, for a detailed review. Thanks for pointing out the access modifiers issue and the creation of the Range in Guess state. Initially, I designed Range to be immutable, but it didn't make much sense, so I added the SetMax/SetMin methods, but forgot to modify the Guess state. I'm not sure what do you mean in your comment about SetupMax/SetupMin states. They both implement IState. Could you elaborate on that one? And I totally agree with you on the tests(TDD) approach \$\endgroup\$ – Daniel Sokolov Jun 3 '15 at 2:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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