Skip to main content
minor wording improvements
Source Link
ChrisWue
  • 20.6k
  • 4
  • 43
  • 107
  1. Some minor syntax detail: C# has collection initializers so one can write:

     List<string> listOfGestures = new List<string> {
         "rock", "paper", "scissors", "lizard", "spock" 
     }
    
  2. You use magic strings for yourthe gestures all over the place. This can pose a problem if you want to change them. You could do a find and replace but youthis could inadvertently change a part of a method or variable name which happens to include "rock" if you are not careful.

In other solutions to the problem posted before enums were used instead. If you insist on using strings you shouldthen make them named constants and use those instead

    const string Rock = "rock";
    const string Paper = "paper";
    ... etc.
  1. YourThe main problem is that the entire implementation is basically just one big blob of code in the main method. Logic and input/output are totally intertwined. Assume you have to write an automated test case for this - it will drive you insane.

If I were to encounter code like this in production I would very likely kill it and rewrite it - the problem is fairly simple and given it's untestable there aren't any unit tests anyway. If the code were too much to rewrite then I'd start refactoring it. Possible steps:

  1. Create a Game class which effectively encapsulates the main while loop up until the line Console.WriteLine ("Your Score is (W:L:T:) : {0}:{1}:{2}", win, lose, tie);
  • Dump everything into one method into that class for starters. - The Game class should also keep track of the score. - Override ToString to print out the current scores. - Move the "ask user if he wants to play another round" into a method on the Game class

    After that yourthe Main should look somewhat like this:

      var game = new Game();
      var continuePlaying = true;
      while (continuePlaying)
      {
          game.Play();
          Console.WriteLine(game.ToString());
          continuePlaying = game.ShouldContinue();
      }
    
  1. Next step would be to start refactoring the Game class and the methods within.
    • Move the decision logic into it's own internal private method to reduce the bulk of the Play() method.

    • Define an interface for getting user input and another one for writing output. Pass these interfaces into the Game constructor. Provide a console implementation for both interfaces. Change all the input and output to use the interfaces rather than the console.

    • Now Game is decoupled from the specific input and output methods and you can start writing unit tests for the Play() and the ShouldContinue() methods. Once you have done that you can continue refactoring those methods. And because you have just created unit tests you should be able to detect if you create any regressions (introducing bugs) while doing so.

    • Wash, Rinse, Repeat

  1. Some minor syntax detail: C# has collection initializers so can write:

     List<string> listOfGestures = new List<string> {
         "rock", "paper", "scissors", "lizard", "spock" 
     }
    
  2. You use magic strings for your gestures all over the place. This can pose a problem if you want to change them. You could do a find and replace but you could inadvertently change a part of a method or variable name which happens to include "rock" if you are not careful.

In other solutions to the problem posted before enums were used instead. If you insist on using strings you should make them named constants and use those instead

    const string Rock = "rock";
    const string Paper = "paper";
    ... etc.
  1. Your main problem is that the entire implementation is basically just one big blob of code in the main method. Logic and input/output are totally intertwined. Assume you have to write an automated test case for this - it will drive you insane.

If I were to encounter code like this in production I would very likely kill it and rewrite it - the problem is fairly simple and given it's untestable there aren't any unit tests anyway. If the code were too much to rewrite then I'd start refactoring it. Possible steps:

  1. Create a Game class which effectively encapsulates the main while loop up until the line Console.WriteLine ("Your Score is (W:L:T:) : {0}:{1}:{2}", win, lose, tie);
  • Dump everything into one method into that class for starters. - The Game class should also keep track of the score. - Override ToString to print out the current scores. - Move the "ask user if he wants to play another round" into a method on the Game class

    After that your Main should look somewhat like this:

      var game = new Game();
      var continuePlaying = true;
      while (continuePlaying)
      {
          game.Play();
          Console.WriteLine(game.ToString());
          continuePlaying = game.ShouldContinue();
      }
    
  1. Next step would be to start refactoring the Game class and the methods within.
    • Move the decision logic into it's own internal private method to reduce the bulk of the Play() method.

    • Define an interface for getting user input and another one for writing output. Pass these interfaces into the Game constructor. Provide a console implementation for both interfaces. Change all the input and output to use the interfaces rather than the console.

    • Now Game is decoupled from the specific input and output methods and you can start writing unit tests for the Play() and the ShouldContinue() methods. Once you have done that you can continue refactoring those methods. And because you have just created unit tests you should be able to detect if you create any regressions (introducing bugs) while doing so.

    • Wash, Rinse, Repeat

  1. Some minor syntax detail: C# has collection initializers so one can write:

     List<string> listOfGestures = new List<string> {
         "rock", "paper", "scissors", "lizard", "spock" 
     }
    
  2. You use magic strings for the gestures all over the place. This can pose a problem if you want to change them. You could do a find and replace but this could inadvertently change a part of a method or variable name which happens to include "rock" if you are not careful.

In other solutions to the problem posted before enums were used instead. If you insist on using strings then make them named constants and use those instead

    const string Rock = "rock";
    const string Paper = "paper";
    ... etc.
  1. The main problem is that the entire implementation is basically just one big blob of code in the main method. Logic and input/output are totally intertwined. Assume you have to write an automated test case for this - it will drive you insane.

If I were to encounter code like this in production I would very likely kill it and rewrite it - the problem is fairly simple and given it's untestable there aren't any unit tests anyway. If the code were too much to rewrite then I'd start refactoring it. Possible steps:

  1. Create a Game class which effectively encapsulates the main while loop up until the line Console.WriteLine ("Your Score is (W:L:T:) : {0}:{1}:{2}", win, lose, tie);
  • Dump everything into one method into that class for starters. - The Game class should also keep track of the score. - Override ToString to print out the current scores. - Move the "ask user if he wants to play another round" into a method on the Game class

    After that the Main should look somewhat like this:

      var game = new Game();
      var continuePlaying = true;
      while (continuePlaying)
      {
          game.Play();
          Console.WriteLine(game.ToString());
          continuePlaying = game.ShouldContinue();
      }
    
  1. Next step would be to start refactoring the Game class and the methods within.
    • Move the decision logic into it's own internal private method to reduce the bulk of the Play() method.

    • Define an interface for getting user input and another one for writing output. Pass these interfaces into the Game constructor. Provide a console implementation for both interfaces. Change all the input and output to use the interfaces rather than the console.

    • Now Game is decoupled from the specific input and output methods and you can start writing unit tests for the Play() and the ShouldContinue() methods. Once you have done that you can continue refactoring those methods. And because you have just created unit tests you should be able to detect if you create any regressions (introducing bugs) while doing so.

    • Wash, Rinse, Repeat

added 2 characters in body
Source Link
Heslacher
  • 51k
  • 5
  • 83
  • 177
  1. Some minor syntax detail: C# has collection initializers so can write:

     List<string> listOfGestures = new List<string> {
         "rock", "paper", "scissors", "lizard", "spock" 
     }
    
  2. You use magic strings for your gestures all over the place. This can pose a problem if you want to change them. You could do a find and replace but you could inadvertently change a part of a method or variable name which happens to include "rock" if you are not careful.

In other solutions to the problem posted before enums were useused instead. If you insist on using strings you should make them named constantconstants and use those instead

    const string Rock = "rock";
    const string Paper = "paper";
    ... etc.
  1. Your main problem is that the entire implementation is basically just one big blob of code in the main method. Logic and input/output are totally intertwined. Assume you have to write an automated test case for this - it will drive you insane.

If I were to encounter code like this in production I would very likely kill it and rewrite it - the problem is fairly simple and given it's untestable there aren't any unit tests anyway. If the code were too much to rewrite then I'd start refactoring it. Possible steps:

  1. Create a Game class which effectively encapsulates the main while loop up until the line Console.WriteLine ("Your Score is (W:L:T:) : {0}:{1}:{2}", win, lose, tie);
  • Dump everything into one method into that class for starters. - The Game class should also keep track of the score. - Override ToString to print out the current scores. - Move the "ask user if he wants to play another round" into a method on the Game class

    After that your Main should look somewhat like this:

      var game = new Game();
      var continuePlaying = true;
      while (continuePlaying)
      {
          game.Play();
          Console.WriteLine(game.ToString());
          continuePlaying = game.ShouldContinue();
      }
    
  1. Next step would be to start refactoring the Game class and the methods within.
    • Move the decision logic into it's own internal private method to reduce the bulk of the Play() method.

    • Define an interface for getting user input and another one for writing output. Pass these interfaces into the Game constructor. Provide a console implementation for both interfaces. Change all the input and output to use the interfaces rather than the console.

    • Now Game is decoupled from the specific input and output methods and you can start writing unit tests for the Play() and the ShouldContinue() methods. Once you have done that you can continue refactoring those methods. And because you have just created unit tests you should be able to detect if you create any regressions (introducing bugs) while doing so.

    • Wash, Rinse, Repeat

  1. Some minor syntax detail: C# has collection initializers so can write:

     List<string> listOfGestures = new List<string> {
         "rock", "paper", "scissors", "lizard", "spock" 
     }
    
  2. You use magic strings for your gestures all over the place. This can pose a problem if you want to change them. You could do a find and replace but you could inadvertently change a part of a method or variable name which happens to include "rock" if you are not careful.

In other solutions to the problem posted before enums were use instead. If you insist on using strings you should make them named constant and use those instead

    const string Rock = "rock";
    const string Paper = "paper";
    ... etc.
  1. Your main problem is that the entire implementation is basically just one big blob of code in the main method. Logic and input/output are totally intertwined. Assume you have to write an automated test case for this - it will drive you insane.

If I were to encounter code like this in production I would very likely kill it and rewrite it - the problem is fairly simple and given it's untestable there aren't any unit tests anyway. If the code were too much to rewrite then I'd start refactoring it. Possible steps:

  1. Create a Game class which effectively encapsulates the main while loop up until the line Console.WriteLine ("Your Score is (W:L:T:) : {0}:{1}:{2}", win, lose, tie);
  • Dump everything into one method into that class for starters. - The Game class should also keep track of the score. - Override ToString to print out the current scores. - Move the "ask user if he wants to play another round" into a method on the Game class

    After that your Main should look somewhat like this:

      var game = new Game();
      var continuePlaying = true;
      while (continuePlaying)
      {
          game.Play();
          Console.WriteLine(game.ToString());
          continuePlaying = game.ShouldContinue();
      }
    
  1. Next step would be to start refactoring the Game class and the methods within.
    • Move the decision logic into it's own internal private method to reduce the bulk of the Play() method.

    • Define an interface for getting user input and another one for writing output. Pass these interfaces into the Game constructor. Provide a console implementation for both interfaces. Change all the input and output to use the interfaces rather than the console.

    • Now Game is decoupled from the specific input and output methods and you can start writing unit tests for the Play() and the ShouldContinue() methods. Once you have done that you can continue refactoring those methods. And because you have just created unit tests you should be able to detect if you create any regressions (introducing bugs) while doing so.

    • Wash, Rinse, Repeat

  1. Some minor syntax detail: C# has collection initializers so can write:

     List<string> listOfGestures = new List<string> {
         "rock", "paper", "scissors", "lizard", "spock" 
     }
    
  2. You use magic strings for your gestures all over the place. This can pose a problem if you want to change them. You could do a find and replace but you could inadvertently change a part of a method or variable name which happens to include "rock" if you are not careful.

In other solutions to the problem posted before enums were used instead. If you insist on using strings you should make them named constants and use those instead

    const string Rock = "rock";
    const string Paper = "paper";
    ... etc.
  1. Your main problem is that the entire implementation is basically just one big blob of code in the main method. Logic and input/output are totally intertwined. Assume you have to write an automated test case for this - it will drive you insane.

If I were to encounter code like this in production I would very likely kill it and rewrite it - the problem is fairly simple and given it's untestable there aren't any unit tests anyway. If the code were too much to rewrite then I'd start refactoring it. Possible steps:

  1. Create a Game class which effectively encapsulates the main while loop up until the line Console.WriteLine ("Your Score is (W:L:T:) : {0}:{1}:{2}", win, lose, tie);
  • Dump everything into one method into that class for starters. - The Game class should also keep track of the score. - Override ToString to print out the current scores. - Move the "ask user if he wants to play another round" into a method on the Game class

    After that your Main should look somewhat like this:

      var game = new Game();
      var continuePlaying = true;
      while (continuePlaying)
      {
          game.Play();
          Console.WriteLine(game.ToString());
          continuePlaying = game.ShouldContinue();
      }
    
  1. Next step would be to start refactoring the Game class and the methods within.
    • Move the decision logic into it's own internal private method to reduce the bulk of the Play() method.

    • Define an interface for getting user input and another one for writing output. Pass these interfaces into the Game constructor. Provide a console implementation for both interfaces. Change all the input and output to use the interfaces rather than the console.

    • Now Game is decoupled from the specific input and output methods and you can start writing unit tests for the Play() and the ShouldContinue() methods. Once you have done that you can continue refactoring those methods. And because you have just created unit tests you should be able to detect if you create any regressions (introducing bugs) while doing so.

    • Wash, Rinse, Repeat

fixed typos
Source Link
ChrisWue
  • 20.6k
  • 4
  • 43
  • 107
  1. Some minor syntax detail: C# has collection initializers so can write:

     List<string> listOfGestures = new List<string> {
         "rock", "paper", "scissors", "lizard", "spock" 
     }
    
  2. You use magic strings for your gestures all over the place. This can pose a problem if you want to change them. You could do a find and replace but you could inadvertently change a part of a method or variable name which happens to include "rock" if you are not careful.

In other solutions to the problem posted before enums were use instead. If you insist on using strings you should make them named constant and use those instead

    const string Rock = "rock";
    const string Paper = "paper";
    ... etc.
  1. Your main problem is that the entire implementation is basically just one big blob of code in yourthe main method. Logic and input/output are totally intertwined. Assume you have to write an automated test case for this - it will drive you insane.

If I were to encounter code like this in production I would very likely kill it and rewrite it - the problem is fairly simple and given it's untestable there aren't any unit tests anyway. If the code were too much to rewrite thethen I'd start refactoring it. Possible steps:

  1. Create a Game class which effectively encapsulates the main while loop upup until the line Console.WriteLine ("Your Score is (W:L:T:) : {0}:{1}:{2}", win, lose, tie);
  • Dump everything into one method into that class for starters. - The Game class should also keep track of the score. - Override ToString to print out the current scores. - Move the "ask user if he wants to play another round" into a method on the Game class

    After that your Main should look somewhat like this:

      var game = new Game();
      var continuePlaying = true;
      while (continuePlaying)
      {
          game.Play();
          Console.WriteLine(game.ToString());
          continuePlaying = game.ShouldContinue();
      }
    
  1. Next step would be to start refactoring the Game class and the methods within.
    • Move the decision logic into it's own internal private method to reduce the bulk of the Play() method.

    • Define an interface for getting user input and another one for writing output. Pass these interfaces into the Game constructor. Provide a console implementation for both interfaces. Change all the input and output to use the interfaces rather than the console.

    • Now Game is decoupled from the specific input and output methods and you can start writing unit tests for the Play() and the ShouldContinue() methods. Once you have done that you can continue refactoring those methods. And because you have just created unit tests you should be able to detect if you create anany regressions (introducing bugs) while doing so.

    • Wash, Rinse, Repeat

  1. Some minor syntax detail: C# has collection initializers so can write:

     List<string> listOfGestures = new List<string> {
         "rock", "paper", "scissors", "lizard", "spock" 
     }
    
  2. You use magic strings for your gestures all over the place. This can pose a problem if you want to change them. You could do a find and replace but you could inadvertently change a part of a method or variable name which happens to include "rock" if you are not careful.

In other solutions to the problem posted before enums were use instead. If you insist on using strings you should make them named constant and use those instead

    const string Rock = "rock";
    const string Paper = "paper";
    ... etc.
  1. Your main problem is that the entire implementation is basically just one big blob of code in your main method. Logic and input/output are totally intertwined. Assume you have to write an automated test case for this - it will drive you insane.

If I were to encounter code like this in production I would very likely kill it and rewrite it - the problem is fairly simple and given it's untestable aren't any unit tests anyway. If the code were too much to rewrite the I'd start refactoring it. Possible steps:

  1. Create a Game class which effectively encapsulates the main while loop up until the line Console.WriteLine ("Your Score is (W:L:T:) : {0}:{1}:{2}", win, lose, tie);
  • Dump everything into one method into that class for starters. - The Game class should also keep track of the score. - Override ToString to print out the current scores. - Move the "ask user if he wants to play another round" into a method on the Game class

    After that your Main should look somewhat like this:

      var game = new Game();
      var continuePlaying = true;
      while (continuePlaying)
      {
          game.Play();
          Console.WriteLine(game.ToString());
          continuePlaying = game.ShouldContinue();
      }
    
  1. Next step would be to start refactoring the Game class and the methods within.
    • Move the decision logic into it's own internal private method to reduce the bulk of the Play() method.

    • Define an interface for getting user input and another one for writing output. Pass these interfaces into the Game constructor. Provide a console implementation for both interfaces. Change all the input and output to use the interfaces rather than the console.

    • Now Game is decoupled from the specific input and output methods and you can start writing unit tests for the Play() and the ShouldContinue() methods. Once you have done that you can continue refactoring those methods. And because you have just created unit tests you should be able to detect if you create an regressions (introducing bugs) while doing so.

    • Wash, Rinse, Repeat

  1. Some minor syntax detail: C# has collection initializers so can write:

     List<string> listOfGestures = new List<string> {
         "rock", "paper", "scissors", "lizard", "spock" 
     }
    
  2. You use magic strings for your gestures all over the place. This can pose a problem if you want to change them. You could do a find and replace but you could inadvertently change a part of a method or variable name which happens to include "rock" if you are not careful.

In other solutions to the problem posted before enums were use instead. If you insist on using strings you should make them named constant and use those instead

    const string Rock = "rock";
    const string Paper = "paper";
    ... etc.
  1. Your main problem is that the entire implementation is basically just one big blob of code in the main method. Logic and input/output are totally intertwined. Assume you have to write an automated test case for this - it will drive you insane.

If I were to encounter code like this in production I would very likely kill it and rewrite it - the problem is fairly simple and given it's untestable there aren't any unit tests anyway. If the code were too much to rewrite then I'd start refactoring it. Possible steps:

  1. Create a Game class which effectively encapsulates the main while loop up until the line Console.WriteLine ("Your Score is (W:L:T:) : {0}:{1}:{2}", win, lose, tie);
  • Dump everything into one method into that class for starters. - The Game class should also keep track of the score. - Override ToString to print out the current scores. - Move the "ask user if he wants to play another round" into a method on the Game class

    After that your Main should look somewhat like this:

      var game = new Game();
      var continuePlaying = true;
      while (continuePlaying)
      {
          game.Play();
          Console.WriteLine(game.ToString());
          continuePlaying = game.ShouldContinue();
      }
    
  1. Next step would be to start refactoring the Game class and the methods within.
    • Move the decision logic into it's own internal private method to reduce the bulk of the Play() method.

    • Define an interface for getting user input and another one for writing output. Pass these interfaces into the Game constructor. Provide a console implementation for both interfaces. Change all the input and output to use the interfaces rather than the console.

    • Now Game is decoupled from the specific input and output methods and you can start writing unit tests for the Play() and the ShouldContinue() methods. Once you have done that you can continue refactoring those methods. And because you have just created unit tests you should be able to detect if you create any regressions (introducing bugs) while doing so.

    • Wash, Rinse, Repeat

Source Link
ChrisWue
  • 20.6k
  • 4
  • 43
  • 107
Loading