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

Console app to teach bitwise operators

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

Problem

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 TutorialCommandsList = new List();
public static readonly List AllCommandsList = new List();
public static readonly List TrainingCommandsList = new List();

public static void AddCommand(ICommand newCommand)
{
    AllCommandsList.Add(newCommand);
}
public static void AddCommand(ICollection 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 AllGenerals => new[]
{
BinaryAND, BinaryOR, BinaryXOR, BinaryNOT, BinaryLeftShift, BinaryRightShift, BitwiseOperators, BinaryCircularShift
};

public GeneralDefinition BinaryAND => new Ge

Solution

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 TutorialCommandsList = new List();
public static readonly List AllCommandsList = new List();
public static readonly List TrainingCommandsList = new List();


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();
}

Code Snippets

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 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; }
}
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);
}
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();
}

Context

StackExchange Code Review Q#124018, answer score: 3

Revisions (0)

No revisions yet.