Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

In addition to what the other reviewers have mentioned

  • It's always better to restrict your Console.WriteLine to your Main method. Hence, you wouldn't require a method such as askForUserOption to print to the console. Likewise showInvalidOptionMessage
  • If you ask me _givenOption should have been implemented as a property rather than a variable as you use it to get and set the content in startMenuResponsalToOptionGiven and readUserOption methods
  • I'm afraid this class breaks Single Responsibility of SOLID PRINCIPLE; the class is responsible for reading, writing and handling exceptions
  • As @I'll add comments tomorrow @I'll add comments tomorrow indicated the recursion achieved by methodCaller can be replaced using a while..loop. This will avoid readUserOption, checkUserOptionIsBetweenMinAndMaxOptions and showInvalidOptionMessage methods
  • I hate to be a spoiler ...but I don't see this code as being unit-testable. Almost all the methods are making several method calls. You want to separate printing to the screen and reading from the screen i.e ShowStartMenu

In addition to what the other reviewers have mentioned

  • It's always better to restrict your Console.WriteLine to your Main method. Hence, you wouldn't require a method such as askForUserOption to print to the console. Likewise showInvalidOptionMessage
  • If you ask me _givenOption should have been implemented as a property rather than a variable as you use it to get and set the content in startMenuResponsalToOptionGiven and readUserOption methods
  • I'm afraid this class breaks Single Responsibility of SOLID PRINCIPLE; the class is responsible for reading, writing and handling exceptions
  • As @I'll add comments tomorrow indicated the recursion achieved by methodCaller can be replaced using a while..loop. This will avoid readUserOption, checkUserOptionIsBetweenMinAndMaxOptions and showInvalidOptionMessage methods
  • I hate to be a spoiler ...but I don't see this code as being unit-testable. Almost all the methods are making several method calls. You want to separate printing to the screen and reading from the screen i.e ShowStartMenu

In addition to what the other reviewers have mentioned

  • It's always better to restrict your Console.WriteLine to your Main method. Hence, you wouldn't require a method such as askForUserOption to print to the console. Likewise showInvalidOptionMessage
  • If you ask me _givenOption should have been implemented as a property rather than a variable as you use it to get and set the content in startMenuResponsalToOptionGiven and readUserOption methods
  • I'm afraid this class breaks Single Responsibility of SOLID PRINCIPLE; the class is responsible for reading, writing and handling exceptions
  • As @I'll add comments tomorrow indicated the recursion achieved by methodCaller can be replaced using a while..loop. This will avoid readUserOption, checkUserOptionIsBetweenMinAndMaxOptions and showInvalidOptionMessage methods
  • I hate to be a spoiler ...but I don't see this code as being unit-testable. Almost all the methods are making several method calls. You want to separate printing to the screen and reading from the screen i.e ShowStartMenu
Source Link
Tolani
  • 2.5k
  • 7
  • 31
  • 49

In addition to what the other reviewers have mentioned

  • It's always better to restrict your Console.WriteLine to your Main method. Hence, you wouldn't require a method such as askForUserOption to print to the console. Likewise showInvalidOptionMessage
  • If you ask me _givenOption should have been implemented as a property rather than a variable as you use it to get and set the content in startMenuResponsalToOptionGiven and readUserOption methods
  • I'm afraid this class breaks Single Responsibility of SOLID PRINCIPLE; the class is responsible for reading, writing and handling exceptions
  • As @I'll add comments tomorrow indicated the recursion achieved by methodCaller can be replaced using a while..loop. This will avoid readUserOption, checkUserOptionIsBetweenMinAndMaxOptions and showInvalidOptionMessage methods
  • I hate to be a spoiler ...but I don't see this code as being unit-testable. Almost all the methods are making several method calls. You want to separate printing to the screen and reading from the screen i.e ShowStartMenu
lang-cs

AltStyle によって変換されたページ (->オリジナル) /