4

I'm working on a console extender project and I'm getting to a point where I feel I need some refactoring.

The only public class in the project is called ExConsole, and to make it just as easy to use as the standard Console class, it's also a static class - however, I'm starting to think that's not such a great idea after all.

Currently, the code just looks like this:

public static class ExConsole
{
    public static void Write(string markup) { /* ... */ }

    public static int Menu(string title, params string[] items) { /* ... */ }

    public static void ClearLastLines(int linesToClear) { /* ... */ }

    public static bool ReadBool(string title, ConsoleKey keyForTrue) { /* ... */ }

    // and so on...
}

There are two main problems I see in my code now:

  1. The only way to extend the ExConsole is by inheritance - I would have much preferred it if I could use extension methods (but of course, that's impossible on a static class...)

  2. The code is getting quite big, and I'm thinking of making it bigger (since it's hard to extend, I would like to have a strong base line, meaning lots of functionality.

I can divide most of the methods into two groups:

One group is for writing stuff to console (Clear lines are considered as writing),
and the other group is for reading stuff from console. (Menu is considered as reading)

So I'm thinking about restructuring the ExConsole class like this:

public static class ExConsole
{
    static ExConsole
    {
        Writer = new Writer();
        Reader = new Reader(Writer);
    }

    public static IWriter Writer {get;}
    public static IReader Reader {get;}
}

It would allow easy extension methods for the Writer and Reader, but it would also make the usage more awkward and kinda silly:

ExConsole.Writer.Write("seems silly, right?");
ExConsole.Reader.ReadInt("Please enter an integer");

at last, compared to what I have now, which is

ExConsole.Write("You know you don't want to see code like `Writer.Write`, right?");
ExConsole.ReadInt("At least the `Reader.ReadInt` can be renamed to `Reader.Int`...");

So, how would you advise me to re-design it in such a way that will:

  1. Keep the usage as easy as possible.
  2. Allow extending.
  3. Will not end up with a massive class (around 30-40 methods (including overloads)).

To address Helena's comments:

Why do you want to allow extensions? What would be examples of extensions you want to provide for?

I want to allow users to extend this class because of two main reasons:

  1. I don't have the time to add everything I think it should contain, and
  2. I can't know in advance what the users will need.

An example of such an extension - adding a ReadDateTime method - I'm imagining a title, a string representing the format and perhaps cultureInfo as input parameters, and a DateTime as a return value - something like this:

public DateTime? ReadDateTime(string source, string format, IFormatProvider provider, DateTimeStyles style, string quitValue)
{
  /* 
    implementation: Show title, and loop until the user enters a value that either can be parsed to DateTime using the specified format or is the quitValue (and return null)
  */
}

All the methods in your example are public, are they called from any outside module, or could they be private instead?

They are public by design. The purpose if this project is to be referenced from other projects to make it easier to work with console apps.

Can you give an example where Menu, ClearLastLines or ReadBool are used?

An example of the usage for the Menu class:

static void Main(string[] args)
{
    var result = ExConsole.Menu(
            "Please select an action:", 
            "<c f='green'>Quit</c>", 
            "Do this", 
            "Do that",
            "Do the other thing");

    switch (result)
    {
        case 1:
            DoThis();
            break;
        case 2:
            DoThat();
            break;
        case 3:
            DoTheOtherThing();
            break;
        default:
            return;
    }
}

That will show menu on the console, titled Please select an action:, with a numbered list starting with 0. quit (where the quit's fore color is green) and ending with 3. Do the other thing, and a line asking the user to select an item from the menu: Please select an item from the menu. Only when the user will enter a number between 0 and 3 (inclusive) the Menu method will return the value the user entered.
Once the user selects an option, the menu disappears from the console screen.

The menu method actually uses three of ExConsole's methods: WriteLine, ReadInt and ClearLastLines.

  • 1
    Possible duplicate of OO - are large classes acceptable? – gnat Aug 29 at 8:18
  • Why do you want to allow extensions? What would be examples of extensions you want to provide for? – Helena Aug 30 at 14:18
  • All the methods in your example are public, are they called from any outside module, or could they be private instead? – Helena Aug 30 at 14:24
  • Can you give an example where Menu, ClearLastLines or ReadBool are used? – Helena Aug 30 at 14:27
  • @helena I'll be happy to but I'll only be near my computer two days from now.... – Zohar Peled Aug 30 at 18:14
6

30-40 methods is not that many - from the view point of users of such a class. They will probably still find the method/function they are looking for easily, as long as you stick to some rigid naming conventions like using prefixes Read and Write for each method which does reading or writing. And if you think ExConsole.Reader.ReadInt won't bring any benefit for the users compared to ExConsole.ReadInt, let the interface as it is.

A different question is how ExConsole is organized internally. If it pays off to split it into smaller classes (and let ExConsole just be a facade, delegating calls to those internal classes), or if it is not worth that effort, depends a lot on how large and complex the implementation currently is. Are the methods are mostly independent and one- or two-line implementations? Or are they calling each other, using lots of private stuff, which can be grouped into smaller classes? Do you really have trouble in maintaining and unit testing that code, or are you only thinking you need to make the class smaller because some "Uncle Bob" has written something about clean code in a book?

For a static class with lots of simple methods, with no internal state to manage, having lots of methods in one class is usually not that much of an issue compared to a situation of a stateful class with complex functions. Breaking classes into parts is not an end in itself, it is a means to an end, which is "managing complexity". But when there is not much complexity, you probably don't need this instrument.

But let us assume your goal is to give the internal implementation a little bit more structure, consider the following options:

  • make ExConsole a partial class, and put the "Reader" parts in one file, the "Writer" parts in another. That may help you to find keep the code a little bit more organized.

  • create helper classes for selected parts. For example, if your method Menu calls some other private methods with a certain complexity, consider to make a MenuHelper class and implement ExConsole.Menu like

    Menu(string title, params string[] items){return menuHelper.Menu(title,items);}
    

    (the same approach can be used for internal Reader and Writer objects, if you think that makes your code more maintainable).

Concerning extensibility: it seems you want to have a "core" ExConsole which goes into a core library, and multiple programs, maybe written by different members of your team, potentially adding different extensions to this core. For this scenario, you should probably replace the static class by a non-static one, to allow the use of extension methods, even if the class (currently) has no state to manage. Usage of such a class is still almost as simple as with the current approach: ExConsole.ReadInt becomes just exConsole.ReadInt, where exConsole is an object of type ExConsole.

For other scenarios, where you are the only maintainer of those ExConsole methods and extensions, or where every member of the team can directly modify the "core" library, consider to simply make extensions by adding methods directly to ExConsole. Using inheritance to extend a static class is usually not very beneficial, since you cannot take advantage of polymorphism for such a class. Sure, it can be used to get access to some protected helper functions from the base class for the implementation of the derived class, but I guess that is none of your requirements.

  • Yes, extendibility is the main concern here. I can change it from a static class to a "normal" class, but there's no state to maintain, so a static class seems like a more natural choice here. I'm not a big fan of uncle bob, and unit testing is problematic with this class anyway, since it uses Console for most if not all methods. Class size is just a small concern here - that can always be solved using partial classes or simply regions. – Zohar Peled Aug 31 at 8:12
  • @ZoharPeled: you still did not tell us what kind of extensibility you require. Another point is, when you have a static class with lots of simple methods, with no internal state to manage, then having lots of methods in one class is usually not that much an issue than in a situation where you have a stateful class with complex functions. Breaking classes into parts is not an end in itself, it is a means to an end, which is "managing complexity". But when there is not much complexity, you probably don't need this instrument. – Doc Brown Aug 31 at 9:38
  • I think a little demonstration might help explain things best. I'll edit my question hopefully later today. Meanwhile, I totally agree that having small classes is not an end in itself. – Zohar Peled Aug 31 at 12:27
  • I've edited my question to address Helena's questions, I think it will also answer the question you've asked : "you still did not tell us what kind of extensibility you require". I would love to get your insight on that, thanks in advance. – Zohar Peled Sep 1 at 5:21
  • @ZoharPeled: thanks - and it seems I don't have to add much to my answer. For the kind of extensibility you want, you should change to a non-static class, even if there (currently) is no state to manage. static classes have some restrictions in C# "by design"; you should not try to fight that design. – Doc Brown Sep 1 at 6:52
2
  1. Keep the usage as easy as possible.

Your code is not object oriented, but a collection of independent static helper methods. And that is ok; it actually makes the usage very easy, and I don't see that you gain anything you want from changing to use objects.

  1. Allow extending.
  2. Will not end up with a massive class (around 30-40 methods (including overloads)).

As your don't have objects (and don't seem to need them), you could just make a new static classes that adds take some of the functionality.

It could be like this:

public static class ExOutput
{
    public static void Write(string markup) { /* ... */ }

    public static int Menu(string title, params string[] items) { /* ... */ }

    public static void ClearLastLines(int linesToClear) { /* ... */ }

    // and so on...
}

public static class ExInput
{
    public static bool ReadBool(string title, ConsoleKey keyForTrue) { /* ... */ }

    // and so on...
}

The same is true for extending. As long as the new code does not interact much with the old methods, you don't have a reason to use inheritance. Just create a new class with new static methods.

From the scenarios you described, OOP doesn't solve any problems you couldn't solve in a more simple way. That said, I can see situations in which OOP could come in handy: - You want a Writer that works with different types of output. Then you would initialize your Writer object with a TextWriter instance. - If some of your methods reuse other methods, e.g. if 'Menu' internally calls 'Write', you might want to change 'Write' and implicitly change 'Menu'.

But if you want to enable these things, these must be explicitly planned. You don't just get extendability for free, if you use objects.

So my suggestion is to keep the class simple, unless you have a reason not to.

  • Thanks, but basically, almost anything is using write or writeline. I agree that keeping things as simple as possible, but I think doc brown had a good point about converting the static class to a 'normal' class. – Zohar Peled Sep 1 at 17:24
  • Thanks to your questions and yours and Doc Brown's answer I was able to complete and upload to GitHub the first version of my ExtendedConsole project. Though your answer was helpful, I found it was your questions that got my thinking more focused on what I wanted for this project - and combining that insight (which I'm not sure I would get to without your questions) with Doc Brown's answer - I think I finally got it right. Thank you for all your help. – Zohar Peled Sep 1 at 19:41

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.