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

Client to connect to server and show a text menu

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

Problem

First of all: Sorry for my English, that said...

I am developing an obligatory for my University, so far it has a class that provides the user with options to connect to a server (once connected it should offer other functionalities, but I only reached the connection option so far), it only shows console options, and checks they are correctly chosen, nothing else.

The class has everything (including itself) static, the reason is simple: I didn't considered necessary to instantiate that class, not even once (what is allowed by using Singleton Pattern), and so I decided going everything static inside it.

I showed my code to my teacher, who said It was an awful design more or less :(
well, he suggested I used Singleton and I explained that I understood Singleton to be only useful when you wanted (needed) only one instance for the class, I don't need even one!

He said static variables are global, which could cause conflict with other variable names and then I would have to remember the static variable names I used so as to avoid conflict, so I highlighted that the class has everything private (except a method called ShowStartMenu() which is internal, and is the one called in the Main) and then he almost exploded...

He told me it wasn't a good design because it didn't allow extensibility (what if I wanted to add more options to the menu, I had to add them in that class... which is exactly what I intended, is that wrong?), and had other disadvantages I could ask my design teacher ¬¬

So, I want to show you my code and ask you what do you think about it.

```
using System;

namespace ARIP.Client.Console
{
internal static class ClientOptions
{
private const int START_MENU_CONNECT_TO_SERVER_OPTION = 1;
private const int START_MENU_END_APPLICATION_OPTION = 0;

private static int _minOption;
private static int _maxOption;
private static int _givenOption;

#region General Client-Option's Functions

private sta

Solution

Regarding static solution, I agree with your teacher. This design makes handling nested menus virtually impossible. Once you consider submenus, it becomes obvious that menu title, texts of menu choices, option limits, etc are instance members, and ShowStartMenu should naturally iterate over choices instead of hardcoding them.

Two other design choices make me very uncomfortable.

First, goal of menu is to compute the user choice. What the rest of applications is going to do with this choice is not of menu's business. It means that _givenOption doesn't belong to the ClientOption class, but better be returned from ShowStartMenu to the caller.

Second, the recursive nature of error handling is totally uncalled for. Consider instead

private static int askUserOptionForStartMenu()
            {
                while (true) {
                    askForUserOption();
                    try {
                        int user_choice = readUserOption();
                        checkUserOptionIsBetweenMinAndMaxOptions(user_choice);
                        return user_choice;
                    } catch (FormatException) {
                        ....
                    } catch (RangeException) {
                        ....
                    }
                }
            }

Code Snippets

private static int askUserOptionForStartMenu()
            {
                while (true) {
                    askForUserOption();
                    try {
                        int user_choice = readUserOption();
                        checkUserOptionIsBetweenMinAndMaxOptions(user_choice);
                        return user_choice;
                    } catch (FormatException) {
                        ....
                    } catch (RangeException) {
                        ....
                    }
                }
            }

Context

StackExchange Code Review Q#140961, answer score: 6

Revisions (0)

No revisions yet.