Skip to main content
added 16 characters in body
Source Link
Deduplicator
  • 19.9k
  • 1
  • 32
  • 65

First, congratulations on getting it to work. Most falter earlier.
Now, let's see what you can improve:

  1. Generally, you use multiple files to enable separate compilation, and in line with that you don't #include source-files, only header-files.

  2. Please, please ensure your files are consistently lower-case, even if Windows doesn't care. It's not quite as bad for source-files as for header-files, as nobody should #include them.

  3. using namespace std; is plain evil. That namespace is not designed for inclusion, thus there is no comprehensive, fixed and reliable list of its contents.
    See "Why is “using namespace std;” considered bad practice?" for details.

  4. You don't use anything from <iomanip>, so don't include it.

  5. usrs is pretty useless. Just use a std::array<User, 2> and be done with it. As a bonus, you no longer need a std::list.

  6. Try to initialize variables on definition. As a bonus, you can use auto, thus avoiding error-prone repetition of useless trivia.

  7. There are far simpler and more efficient ways to parse a number than creating and destroying a std::stringstream. For example std::stoi().

  8. Never assume input is valid. And evenEven if it hasshould have the right format, a surprise, it might still ask for an illegal operation.

  9. A block is a full statement. Adding an additional semicolon is valid most of the time, because it is its own empty statement. You will get problems with else-branches though.

  10. If you output single characters, prefer character literals to length-1 string-literals. It might be marginally more efficient.

  11. It's acceptable to indent access-specifiers one level. But then all members should be indented two levels, not only those after the first access-specifier.

  12. Use the ctor-init-list to initialise members, though prefer in-class-initialisers if you can. Inside a ctor-body is far too late for initialisation.

That should be enough to point you in the right direction.

First, congratulations on getting it to work. Most falter earlier.
Now, let's see what you can improve:

  1. Generally, you use multiple files to enable separate compilation, and in line with that you don't #include source-files, only header-files.

  2. Please, please ensure your files are consistently lower-case, even if Windows doesn't care. It's not quite as bad for source-files as for header-files, as nobody should #include them.

  3. using namespace std; is plain evil. That namespace is not designed for inclusion, thus there is no comprehensive, fixed and reliable list of its contents.
    See "Why is “using namespace std;” considered bad practice?" for details.

  4. You don't use anything from <iomanip>, so don't include it.

  5. usrs is pretty useless. Just use a std::array<User, 2> and be done with it. As a bonus, you no longer need a std::list.

  6. Try to initialize variables on definition. As a bonus, you can use auto, thus avoiding error-prone repetition of useless trivia.

  7. There are far simpler and more efficient ways to parse a number than creating and destroying a std::stringstream. For example std::stoi().

  8. Never assume input is valid. And even if it has the right format, it might still ask for an illegal operation.

  9. A block is a full statement. Adding an additional semicolon is valid most of the time, because it is its own empty statement. You will get problems with else-branches though.

  10. If you output single characters, prefer character literals to length-1 string-literals. It might be marginally more efficient.

  11. It's acceptable to indent access-specifiers one level. But then all members should be indented two levels.

  12. Use the ctor-init-list to initialise members, though prefer in-class-initialisers if you can. Inside a ctor-body is far too late for initialisation.

That should be enough to point you in the right direction.

First, congratulations on getting it to work. Most falter earlier.
Now, let's see what you can improve:

  1. Generally, you use multiple files to enable separate compilation, and in line with that you don't #include source-files, only header-files.

  2. Please, please ensure your files are consistently lower-case, even if Windows doesn't care. It's not quite as bad for source-files as for header-files, as nobody should #include them.

  3. using namespace std; is plain evil. That namespace is not designed for inclusion, thus there is no comprehensive, fixed and reliable list of its contents.
    See "Why is “using namespace std;” considered bad practice?" for details.

  4. You don't use anything from <iomanip>, so don't include it.

  5. usrs is pretty useless. Just use a std::array<User, 2> and be done with it. As a bonus, you no longer need a std::list.

  6. Try to initialize variables on definition. As a bonus, you can use auto, thus avoiding error-prone repetition of useless trivia.

  7. There are far simpler and more efficient ways to parse a number than creating and destroying a std::stringstream. For example std::stoi().

  8. Never assume input is valid. Even if it should have the right format, a surprise, it might still ask for an illegal operation.

  9. A block is a full statement. Adding an additional semicolon is valid most of the time, because it is its own empty statement. You will get problems with else-branches though.

  10. If you output single characters, prefer character literals to length-1 string-literals. It might be marginally more efficient.

  11. It's acceptable to indent access-specifiers one level. But then all members should be indented two levels, not only those after the first access-specifier.

  12. Use the ctor-init-list to initialise members, though prefer in-class-initialisers if you can. Inside a ctor-body is far too late for initialisation.

That should be enough to point you in the right direction.

Source Link
Deduplicator
  • 19.9k
  • 1
  • 32
  • 65

First, congratulations on getting it to work. Most falter earlier.
Now, let's see what you can improve:

  1. Generally, you use multiple files to enable separate compilation, and in line with that you don't #include source-files, only header-files.

  2. Please, please ensure your files are consistently lower-case, even if Windows doesn't care. It's not quite as bad for source-files as for header-files, as nobody should #include them.

  3. using namespace std; is plain evil. That namespace is not designed for inclusion, thus there is no comprehensive, fixed and reliable list of its contents.
    See "Why is “using namespace std;” considered bad practice?" for details.

  4. You don't use anything from <iomanip>, so don't include it.

  5. usrs is pretty useless. Just use a std::array<User, 2> and be done with it. As a bonus, you no longer need a std::list.

  6. Try to initialize variables on definition. As a bonus, you can use auto, thus avoiding error-prone repetition of useless trivia.

  7. There are far simpler and more efficient ways to parse a number than creating and destroying a std::stringstream. For example std::stoi().

  8. Never assume input is valid. And even if it has the right format, it might still ask for an illegal operation.

  9. A block is a full statement. Adding an additional semicolon is valid most of the time, because it is its own empty statement. You will get problems with else-branches though.

  10. If you output single characters, prefer character literals to length-1 string-literals. It might be marginally more efficient.

  11. It's acceptable to indent access-specifiers one level. But then all members should be indented two levels.

  12. Use the ctor-init-list to initialise members, though prefer in-class-initialisers if you can. Inside a ctor-body is far too late for initialisation.

That should be enough to point you in the right direction.