Console app to teach bitwise operators

Question

I'm creating a console app that has the purpose to teach you how to use C# bitwise operators. I have several classes which contain info about the different operators and I instantiate/add them in the main class like this:

var trainingDefinitions = new TrainingDefinitions();
var generalDefinitions = new GeneralDefinitions();
var circularShiftDefinitions = new TrainingCircularShiftDefinitions();

foreach (Training training in trainingDefinitions.AllTrainings.Select(definition => new Training(definition)))
{
    AddCommand(TrainingCommandsList, training);
}

foreach (General general in generalDefinitions.AllGenerals.Select(definition => new General(definition)))
{
    AddCommand(general);
}

foreach (TrainingBinaryCircularShift circShift in circularShiftDefinitions.AllTrainings.Select(definition => new TrainingBinaryCircularShift(definition)))
{ 
    AddCommand(TrainingCommandsList, circShift);
}

Where the AddCommand method just adds them to a list where I keep all the available commands:

public static readonly List<ICommand> TutorialCommandsList = new List<ICommand>();
public static readonly List<ICommand> AllCommandsList = new List<ICommand>();
public static readonly List<ICommand> TrainingCommandsList = new List<ICommand>();

public static void AddCommand(ICommand newCommand)
{
    AllCommandsList.Add(newCommand);
}
public static void AddCommand(ICollection<ICommand> commandList, ICommand newCommand)
{
    AllCommandsList.Add(newCommand);
    commandList.Add(newCommand);
}

Now my main problem is that those foreach loops are really similar and I want to refactor it into a method. I'm not sure how I can pass them as parameters (I want just 1 method not a static polymorphism).

Here's how they look like:

public class GeneralDefinitions
{
    private readonly Text _text = new Text();
    public IEnumerable<GeneralDefinition> AllGenerals => new[]
    {
        BinaryAND, BinaryOR, BinaryXOR, BinaryNOT, BinaryLeftShift, BinaryRightShift, BitwiseOperators, BinaryCircularShift
    };

    public GeneralDefinition BinaryAND => new GeneralDefinition
    {
        CommandAccessor = _text.GeneralBinaryANDAccessors,
        CommandInfo = _text.GeneralBinaryANDInfo,
        OperationInfo = _text.GeneralBinaryANDOpInfo
    };
 }
    // all the other operators are done the same way

The General Class :

private readonly GeneralDefinition _definition;

public string[] CommandAccessor
{
    get { return _definition.CommandAccessor; }
    set { _definition.CommandAccessor = value; }
}

public string CommandInfo
{
    get { return _definition.CommandInfo; }
    set { _definition.CommandInfo = value; }
}

public string OperationInfo
{
    get { return _definition.OperationInfo; }
    set { _definition.OperationInfo = value; }
}

public General(GeneralDefinition definition)
{
    _definition = definition;
}

public bool IsThisCommand(string inputCommand)
{
    return CommandAccessor.Contains(inputCommand);
}

public void Display()
{
    Console.WriteLine(_definition.OperationInfo);
}

Training definitions almost the same as general definitions:

public class TrainingDefinitions
{
    private readonly Text _text = new Text();
    public IEnumerable<TrainingDefinition> AllTrainings => new[]
    {
        BinaryAND, BinaryOR, BinaryXOR,BinaryLeftShift,BinaryRightShift
    };

    public TrainingDefinition BinaryAND => new TrainingDefinition
    {
        CommandAccessor = _text.TrainingBinaryANDAccessors,
        CommandInfo = _text.TrainingBinaryANDInfo,
        Operation = "&",
        OperationLetters = "Binary AND",
        BitOperator = (a, b) => a & b,
    };
}

Is it even acceptable to keep them like this (using the foreach loops)? I'm planning to have a few more of those and it will not look at well.

Training class :

public class Training :ITraining, ICommand
{
    public Func<int, int, int> BitOperator => definition.BitOperator;
    public string OperationInfo { get; set; }

    public string[] CommandAccessor
    {
        get { return definition.CommandAccessor; }
        set { definition.CommandAccessor = value; }
    }

    public string CommandInfo
    {
        get { return definition.CommandInfo; }
        set { definition.CommandInfo = value; }
    }

    private readonly TrainingDefinition definition;

    public Training(TrainingDefinition definition)
    {
        this.definition = definition;
    }

    public void Display()
    {
        DisplayTraining(definition.Operation, definition.OperationLetters);
    }


    private static bool ExitWithCommand(string tempInput)
    {
        if (tempInput != "/exit" && !MainScreen.IsCommand(tempInput)) return false;
        Console.WriteLine("You exited succesfully");
        return true;
    }

    public bool GetValue(string tempInput, ref int b)
    {
        while (!int.TryParse(tempInput, out b))
        {
            if (ExitWithCommand(tempInput)) return true;
            Console.ForegroundColor = ConsoleColor.Red;
            Console.WriteLine("Input can be only numbers !");
            Console.ForegroundColor = Settings.DefaultColor;
            tempInput = Console.ReadLine();
        }
        return false;
    }

    public int BitOperationResult(int input1, int input2)
    {
        return BitOperator(input1, input2);
    }

    public void DisplayTraining(string operation, string operationLetters)
    {
        DisplayWelcomeMessage(operation, operationLetters);
        while (true)
        {
            int a = 0;
            int b = 0;
            var result = 0;
            Console.Write("Enter a = ");
            string tempInput = Console.ReadLine();
            if (GetValue(tempInput, ref a)) break;
            Console.Write("Enter b = ");
            tempInput = Console.ReadLine();
            if (GetValue(tempInput, ref b)) break;
            Console.WriteLine("The equation looks like this now : {0} {2} {1}", a, b,operation);
            Console.Write("Now enter the answer you think is right : {0} {2} {1} = ", a, b,operation);
            tempInput = Console.ReadLine();
            if (GetValue(tempInput, ref result)) break;

            CheckAnswer(result, a, b);
            Console.ForegroundColor = Settings.DefaultColor;
            Console.WriteLine("Now let's try again");
            Console.WriteLine();
        }
    }

    public void CheckAnswer(int result, int a, int b)
    {
        if (result == BitOperator(a, b))
        {
            Console.ForegroundColor = ConsoleColor.Green;
            Console.Write("Correct ! ");
            Console.Write("The answer is = {0} ", result);
        }
        else
        {
            Console.ForegroundColor = ConsoleColor.Red;
            Console.WriteLine("Wrong .. the correct answer is {0} ", BitOperator(a, b));
        }
    }

    public void DisplayWelcomeMessage(string operation, string operationLetters)
    {
        string[] welcomeMessage =
        {
            @"Welcome to the Testing Area for the " + operationLetters + " " + operation + " .",
            @"Here you can enter 2 values and try to calculate the result.",
            @"A message will be shown whether you got the correct answer or no."
        };
        foreach (var s in welcomeMessage)
        {
            Console.WriteLine(s);
        }
        Console.WriteLine(
            @"You can exit at any time by typing /exit or redirect yourself to any other tutorial/training");
        Console.WriteLine();
        Console.WriteLine("This is how the {0} operation looks like : a {1} b", operationLetters, operation);
    }

    public bool IsThisCommand(string inputCommand)
    {
        return CommandAccessor.Contains(inputCommand);
    }
}

ITraining Interface :

    public interface ITraining
{
    void DisplayWelcomeMessage(string operation, string operationLetters);

    void CheckAnswer(int result, int a, int b);

    void DisplayTraining(string operation, string operationLetters);

    bool GetValue(string tempInput, ref int b);

    int BitOperationResult(int input1, int input2);

    Func<int, int, int> BitOperator { get;}
}

ICommand Interface :

public interface ICommand
{
    string[] CommandAccessor { get; set; }
    string CommandInfo { get; set; }

    bool IsThisCommand(string inputCommand);

    void Display();
}

TrainingDefintion :

public class TrainingDefinition
{
    public string[] CommandAccessor { get; set; }
    public string CommandInfo { get; set; }
    public string Operation { get; set; }
    public string OperationLetters { get; set; }
    public Func<int, int, int> BitOperator { get; set; }
}

Answer 1

You do need polymorphism even though you say you don't. There is no pretty way to eliminate those almost-exactly-the-same loops, unless you extract a common interface. In your case it should be easy to do.

---

This should be refactored into single list:

public static readonly List<ICommand> TutorialCommandsList = new List<ICommand>();
public static readonly List<ICommand> AllCommandsList = new List<ICommand>();
public static readonly List<ICommand> TrainingCommandsList = new List<ICommand>();

You can always use LINQ queries (Where, TypeOf, etc.) if you need a specific subset later on.

---

This looks weird:

public string[] CommandAccessor
{
    get { return _definition.CommandAccessor; }
    set { _definition.CommandAccessor = value; }
}

public string CommandInfo
{
    get { return _definition.CommandInfo; }
    set { _definition.CommandInfo = value; }
}

public string OperationInfo
{
    get { return _definition.OperationInfo; }
    set { _definition.OperationInfo = value; }
}

You should just expose your definition as a property (again, using common interface).

---

ITraining interface does not make much sense. It contains presentation logic (DisplayWelcomeMessage and DisplayTraining). It calculates and checks the result of some operation (CheckAnswer and BitOperationResult), so that's business logic. It also "gets" some value from some string, so I guess it does some sort of conversion? No way to tell just by looking at method name. It also has a BitOperator, which might or might not do the same thing as BitOperationResult. If it does the same thing, then why is it there? If it doesn't, then what is the difference? Again, no way to tell. This interface is extremely confusing and violates SRP severely.

---

I can't say I fully understand your design, since you don't really give any explanations as to why you implemented it the way you did. But if I were to suggest a better design off the top of my head, I would probably go for something like this:

interface ISession
{
    SessionResult Display();
}

interface ILessonDescription
{
    string Operator { get; }
    string Description { get; }
    // all the other information you might need to construct an ILesson
}

interface ILesson
{
    ILessonDescription Description { get; }

    ISession Tutorial { get; }
    ISession Exercise { get; }
}

interface ILessonFactory
{
    ILesson Create(ILessonDescription description);
}

And the main method might look like this:

var factory = new LessonFactory();
var descriptions = new ILessonDescription[] { .... };
while (true)
{
    Console.WriteLine("Select lesson number from the list:");
    Concole.WriteLine(String.Join("\n", descriptions.Select((d, i) => String.Format("{0}) {1}.", i+1, d.Description))));

    var input = Console.ReadLine();
    int selectedNumber;
    int.TryParse(input, out selectedNumber);
    var description = descriptions[selectedNumber - 1];
    var lesson = factory.Create(description);
    lesson.Tutorial.Display();
    lesson.Exercise.Display();
}