Skip to main content
added 138 characters in body
Source Link
Morwenn
  • 20k
  • 3
  • 67
  • 127

I have a few tidbits that could help. Nothing big, but things that you can improve as you code without having to overthink them:

  • Use constexpr to represent compile-time constants. static const works but constexpr pops into to head and screams "compile time!" when you read it:

      static constexpr std::size_t mDim = 3;
    
  • column::operator() should be const-qualified since it does not alter the state of column:

      bool operator() (int number) const
      { 
          return (number % dim == colNum); 
      }
    
  • unary_function and binary_function are deprecated since C++11 and will be removed from the standard in C++17. Therefore, you should avoid using them. The easy fix is to define result_type and argument_type by hand (which is rahter quick) or to simply use the types at the call point where you probably know them.

  • You could consider making the types column and pairCondition immutable by making their members const. Beforehand, really think about whether you want them to be immutable. It could make sense for column since functions are often immutable and this is a functor type.

  • I don't think that you want to represent any other cells thant "no player", "player 1" and "player 2". Therefore, you could probably use an enum struct to represent the accepted values in your grid instead of a char:

      enum struct player: char
      {
          none = '-',
          first = 'X',
          second = 'O'
      };
    

    Using an enum struct would ensure that you don't use any other magic value and would answer the question "what does this char represent?" when reading the code. If you really need to access the underlying type of the enum at different places, then use a simple enum instead of an enum struct.

I have a few tidbits that could help. Nothing big, but things that you can improve as you code without having to overthink them:

  • Use constexpr to represent compile-time constants. static const works but constexpr pops into to head and screams "compile time!" when you read it:

      static constexpr std::size_t mDim = 3;
    
  • column::operator() should be const-qualified since it does not alter the state of column:

      bool operator() (int number) const
      { 
          return (number % dim == colNum); 
      }
    
  • unary_function and binary_function are deprecated since C++11 and will be removed from the standard in C++17. Therefore, you should avoid using them. The easy fix is to define result_type and argument_type by hand (which is rahter quick) or to simply use the types at the call point where you probably know them.

  • You could consider making the types column and pairCondition immutable by making their members const. Beforehand, really think about whether you want them to be immutable. It could make sense for column since functions are often immutable and this is a functor type.

  • I don't think that you want to represent any other cells thant "no player", "player 1" and "player 2". Therefore, you could probably use an enum struct to represent the accepted values in your grid instead of a char:

      enum struct player: char
      {
          none = '-',
          first = 'X',
          second = 'O'
      };
    

    Using an enum struct would ensure that you don't use any other magic value and would answer the question "what does this char represent?" when reading the code.

I have a few tidbits that could help. Nothing big, but things that you can improve as you code without having to overthink them:

  • Use constexpr to represent compile-time constants. static const works but constexpr pops into to head and screams "compile time!" when you read it:

      static constexpr std::size_t mDim = 3;
    
  • column::operator() should be const-qualified since it does not alter the state of column:

      bool operator() (int number) const
      { 
          return (number % dim == colNum); 
      }
    
  • unary_function and binary_function are deprecated since C++11 and will be removed from the standard in C++17. Therefore, you should avoid using them. The easy fix is to define result_type and argument_type by hand (which is rahter quick) or to simply use the types at the call point where you probably know them.

  • You could consider making the types column and pairCondition immutable by making their members const. Beforehand, really think about whether you want them to be immutable. It could make sense for column since functions are often immutable and this is a functor type.

  • I don't think that you want to represent any other cells thant "no player", "player 1" and "player 2". Therefore, you could probably use an enum struct to represent the accepted values in your grid instead of a char:

      enum struct player: char
      {
          none = '-',
          first = 'X',
          second = 'O'
      };
    

    Using an enum struct would ensure that you don't use any other magic value and would answer the question "what does this char represent?" when reading the code. If you really need to access the underlying type of the enum at different places, then use a simple enum instead of an enum struct.

Source Link
Morwenn
  • 20k
  • 3
  • 67
  • 127

I have a few tidbits that could help. Nothing big, but things that you can improve as you code without having to overthink them:

  • Use constexpr to represent compile-time constants. static const works but constexpr pops into to head and screams "compile time!" when you read it:

      static constexpr std::size_t mDim = 3;
    
  • column::operator() should be const-qualified since it does not alter the state of column:

      bool operator() (int number) const
      { 
          return (number % dim == colNum); 
      }
    
  • unary_function and binary_function are deprecated since C++11 and will be removed from the standard in C++17. Therefore, you should avoid using them. The easy fix is to define result_type and argument_type by hand (which is rahter quick) or to simply use the types at the call point where you probably know them.

  • You could consider making the types column and pairCondition immutable by making their members const. Beforehand, really think about whether you want them to be immutable. It could make sense for column since functions are often immutable and this is a functor type.

  • I don't think that you want to represent any other cells thant "no player", "player 1" and "player 2". Therefore, you could probably use an enum struct to represent the accepted values in your grid instead of a char:

      enum struct player: char
      {
          none = '-',
          first = 'X',
          second = 'O'
      };
    

    Using an enum struct would ensure that you don't use any other magic value and would answer the question "what does this char represent?" when reading the code.