Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

I will expand on this when I have more time, but a few things jumped out:


get_M/get_L/get_N should return a std::size_t, not an int.


get_M/get_L/get_N should be const.


It might make sense in your field of study, but to me, NAN is weird for the initial value. I would expect the initial values to be either undefined or 0. Whatever works best for you though. Initializing to NAN would, after all, provide a few convenient properties.


All of the size_t in the header should be std::size_t std::size_t.


Rather than duplicate the exact same code, the constructor should use reset.


Be careful with using namespace std;, even in implementation files. If you want to read way more than you ever wanted to on it, see this this. The "tl;dr" of it is that a potentially application breaking bug might happen years down the road do to a change a dozen files away, but that can all be avoided with 5 extra characters (std::).


When outputting potentially huge chunks of data, try not to flush until you either need to or the end (in single threaded applications, that's almost always going to be the end).

In particular, std::endl is not equivalent to "\n". std::cout << "\n"; pushes a new line into std::couts buffer. std::cout << std::endl; pushes a newline into std::cout's buffer and then flushes the buffer. This means that your operator<< isn't really using buffered IO since it flushes after every output.

Simple fix: use "\n" and then just do std::flush(std::cout); (or std::cout << std::flush) at the end.


cout.width(10); in your operator<< should be os.width(10).

I will expand on this when I have more time, but a few things jumped out:


get_M/get_L/get_N should return a std::size_t, not an int.


get_M/get_L/get_N should be const.


It might make sense in your field of study, but to me, NAN is weird for the initial value. I would expect the initial values to be either undefined or 0. Whatever works best for you though. Initializing to NAN would, after all, provide a few convenient properties.


All of the size_t in the header should be std::size_t.


Rather than duplicate the exact same code, the constructor should use reset.


Be careful with using namespace std;, even in implementation files. If you want to read way more than you ever wanted to on it, see this. The "tl;dr" of it is that a potentially application breaking bug might happen years down the road do to a change a dozen files away, but that can all be avoided with 5 extra characters (std::).


When outputting potentially huge chunks of data, try not to flush until you either need to or the end (in single threaded applications, that's almost always going to be the end).

In particular, std::endl is not equivalent to "\n". std::cout << "\n"; pushes a new line into std::couts buffer. std::cout << std::endl; pushes a newline into std::cout's buffer and then flushes the buffer. This means that your operator<< isn't really using buffered IO since it flushes after every output.

Simple fix: use "\n" and then just do std::flush(std::cout); (or std::cout << std::flush) at the end.


cout.width(10); in your operator<< should be os.width(10).

I will expand on this when I have more time, but a few things jumped out:


get_M/get_L/get_N should return a std::size_t, not an int.


get_M/get_L/get_N should be const.


It might make sense in your field of study, but to me, NAN is weird for the initial value. I would expect the initial values to be either undefined or 0. Whatever works best for you though. Initializing to NAN would, after all, provide a few convenient properties.


All of the size_t in the header should be std::size_t.


Rather than duplicate the exact same code, the constructor should use reset.


Be careful with using namespace std;, even in implementation files. If you want to read way more than you ever wanted to on it, see this. The "tl;dr" of it is that a potentially application breaking bug might happen years down the road do to a change a dozen files away, but that can all be avoided with 5 extra characters (std::).


When outputting potentially huge chunks of data, try not to flush until you either need to or the end (in single threaded applications, that's almost always going to be the end).

In particular, std::endl is not equivalent to "\n". std::cout << "\n"; pushes a new line into std::couts buffer. std::cout << std::endl; pushes a newline into std::cout's buffer and then flushes the buffer. This means that your operator<< isn't really using buffered IO since it flushes after every output.

Simple fix: use "\n" and then just do std::flush(std::cout); (or std::cout << std::flush) at the end.


cout.width(10); in your operator<< should be os.width(10).

added 623 characters in body
Source Link
Corbin
  • 10.6k
  • 2
  • 31
  • 51

I will expand on this when I have more time, but a few things jumped out:


get_M/get_L/get_N should return a std::size_t, not an int.


get_M/get_L/get_N should be const.


It might make sense in your field of study, but to me, NAN is weird for the initial value. I would expect the initial values to be either undefined or 0. Whatever works best for you though. Initializing to NAN would, after all, provide a few convenient properties.


All of the size_t in the header should be std::size_t.


Rather than duplicate the exact same code, the constructor should use reset.


Be careful with using namespace std;, even in implementation files. If you want to read way more than you ever wanted to on it, see this. The "tl;dr" of it is that a potentially application breaking bug might happen years down the road do to a change a dozen files away, but that can all be avoided with 5 extra characters (std::).


When outputting potentially huge chunks of data, try not to flush until you either need to or the end (in single threaded applications, that's almost always going to be the end).

In particular, std::endl is not equivalent to "\n". std::cout << "\n"; pushes a new line into std::couts buffer. std::cout << std::endl; pushes a newline into std::cout's buffer and then flushes the buffer. This means that your operator<< isn't really using buffered IO since it flushes after every output.

Simple fix: use "\n" and then just do std::flush(std::cout); (or std::cout << std::flush) at the end.


cout.width(10); in your operator<< should be os.width(10).

I will expand on this when I have more time, but a few things jumped out:


get_M/get_L/get_N should return a std::size_t, not an int.


get_M/get_L/get_N should be const.


It might make sense in your field of study, but to me, NAN is weird for the initial value. I would expect the initial values to be either undefined or 0. Whatever works best for you though. Initializing to NAN would, after all, provide a few convenient properties.


All of the size_t in the header should be std::size_t.


Rather than duplicate the exact same code, the constructor should use reset.


Be careful with using namespace std;, even in implementation files. If you want to read way more than you ever wanted to on it, see this. The "tl;dr" of it is that a potentially application breaking bug might happen years down the road do to a change a dozen files away, but that can all be avoided with 5 extra characters (std::).

I will expand on this when I have more time, but a few things jumped out:


get_M/get_L/get_N should return a std::size_t, not an int.


get_M/get_L/get_N should be const.


It might make sense in your field of study, but to me, NAN is weird for the initial value. I would expect the initial values to be either undefined or 0. Whatever works best for you though. Initializing to NAN would, after all, provide a few convenient properties.


All of the size_t in the header should be std::size_t.


Rather than duplicate the exact same code, the constructor should use reset.


Be careful with using namespace std;, even in implementation files. If you want to read way more than you ever wanted to on it, see this. The "tl;dr" of it is that a potentially application breaking bug might happen years down the road do to a change a dozen files away, but that can all be avoided with 5 extra characters (std::).


When outputting potentially huge chunks of data, try not to flush until you either need to or the end (in single threaded applications, that's almost always going to be the end).

In particular, std::endl is not equivalent to "\n". std::cout << "\n"; pushes a new line into std::couts buffer. std::cout << std::endl; pushes a newline into std::cout's buffer and then flushes the buffer. This means that your operator<< isn't really using buffered IO since it flushes after every output.

Simple fix: use "\n" and then just do std::flush(std::cout); (or std::cout << std::flush) at the end.


cout.width(10); in your operator<< should be os.width(10).

Source Link
Corbin
  • 10.6k
  • 2
  • 31
  • 51

I will expand on this when I have more time, but a few things jumped out:


get_M/get_L/get_N should return a std::size_t, not an int.


get_M/get_L/get_N should be const.


It might make sense in your field of study, but to me, NAN is weird for the initial value. I would expect the initial values to be either undefined or 0. Whatever works best for you though. Initializing to NAN would, after all, provide a few convenient properties.


All of the size_t in the header should be std::size_t.


Rather than duplicate the exact same code, the constructor should use reset.


Be careful with using namespace std;, even in implementation files. If you want to read way more than you ever wanted to on it, see this. The "tl;dr" of it is that a potentially application breaking bug might happen years down the road do to a change a dozen files away, but that can all be avoided with 5 extra characters (std::).

lang-cpp

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