Return to Answer

This looks quite readable and easy to follow. I just have a few things that have stuck out to me:

• It would not be good to use a floating-point type for currency, and it's especially worse for a more accurate type like a double. For instance, you can wind up with a value like 0.0001, which is not an ideal value to deal with. Unfortunately, C++ doesn't have a standard library to deal with this, such as Java's BigDecimal. You need an integer type for this, which avoids these problems.

  Refer to this, this and this for more info. There are already many resources on this very topic.

• Your "getters" should be const as they're not supposed to modify data members:

   int getSomeMember() const {
   return someMember;
   }
  

  mMinDenominations should also be const for the same reason. Initializer lists do also allow initialization of const members (be aware that constructors cannot do this).

  Moreover, consider redesigning this to avoid getters. They (and also setters) are generally bad for encapsulation as they expose implementation details. You may not even need them here, either.

• You can replace displayBalance() with an overload of operator<<:

   friend std::ostream& operator<<(std::ostream& out, ATM const& obj) {
   return out << obj.mAccount.getBalance() << '\n';
   }
  

  Since mAccount is private, this will have to be defined within the class. This will still work, even if you remove the getters.

This looks quite readable and easy to follow. I just have a few things that have stuck out to me:

• It would not be good to use a floating-point type for currency, and it's especially worse for a more accurate type like a double. For instance, you can wind up with a value like 0.0001, which is not an ideal value to deal with. Unfortunately, C++ doesn't have a standard library to deal with this, such as Java's BigDecimal. You need an integer type for this, which avoids these problems.

  Refer to this, this and this for more info. There are already many resources on this very topic.

• Your "getters" should be const as they're not supposed to modify data members:

   int getSomeMember() const {
   return someMember;
   }
  

  mMinDenominations should also be const for the same reason. Initializer lists do also allow initialization of const members (be aware that constructors cannot do this).

  Moreover, consider redesigning this to avoid getters. They (and also setters) are generally bad for encapsulation as they expose implementation details. You may not even need them here, either.

• You can replace displayBalance() with an overload of operator<<:

   friend std::ostream& operator<<(std::ostream& out, ATM const& obj) {
   return out << obj.mAccount.getBalance() << '\n';
   }
  

  Since mAccount is private, this will have to be defined within the class. This will still work, even if you remove the getters.

This looks quite readable and easy to follow. I just have a few things that have stuck out to me:

• It would not be good to use a floating-point type for currency, and it's especially worse for a more accurate type like a double. For instance, you can wind up with a value like 0.0001, which is not an ideal value to deal with. Unfortunately, C++ doesn't have a standard library to deal with this, such as Java's BigDecimal. You need an integer type for this, which avoids these problems.

  Refer to this, this and this for more info. There are already many resources on this very topic.

• Your "getters" should be const as they're not supposed to modify data members:

   int getSomeMember() const {
   return someMember;
   }
  

  mMinDenominations should also be const for the same reason. Initializer lists do also allow initialization of const members (be aware that constructors cannot do this).

  Moreover, consider redesigning this to avoid getters. They (and also setters) are generally bad for encapsulation as they expose implementation details. You may not even need them here, either.

• You can replace displayBalance() with an overload of operator<<:

   friend std::ostream& operator<<(std::ostream& out, ATM const& obj) {
   return out << obj.mAccount.getBalance() << '\n';
   }
  

  Since mAccount is private, this will have to be defined within the class. This will still work, even if you remove the getters.

This looks quite readable and easy to follow. I just have a few things that have stuck out to me:

• It would not be good to use a floating-point type for currency, and it's especially worse for a more accurate type like a double. For instance, you can wind up with a value like 0.0001, which is not an ideal value to deal with. Unfortunately, C++ doesn't have a standard library to deal with this, such as Java's BigDecimal. You need an integer type for this, which avoids these problems.

  Refer to this, this and this for more info. There are already many resources on this very topic.

• Your "getters" should be const as they're not supposed to modify data members:

   int getSomeMember() const {
   return someMember;
   }
  

  mMinDenominations should also be const for the same reason. Initializer lists do also allow initialization of const members (be aware that constructors cannot do this).

  Moreover, consider redesigning this to avoid getters. They (and also setters) are generally bad for encapsulation as they expose implementation details. You may not even need them here, either.

• You can replace displayBalance() with an overload of operator<<:

   friend std::ostream& operator<<(std::ostream& out, ATM const& obj) {
   return out << obj.mAccount.getBalance() << '\n';
   }
  

  Since mAccount is private, this will have to be defined within the class. This will still work, even if you remove the getters.

This looks quite readable and easy to follow. I just have a few things that have stuck out to me:

• It would not be good to use a floating-point type for currency, and it's especially worse for a more accurate type like a double. For instance, you can wind up with a value like 0.0001, which is not an ideal value to deal with. Unfortunately, C++ doesn't have a standard library to deal with this, such as Java's BigDecimal. You need an integer type for this, which avoids these problems.

  Refer to this, this and this for more info. There are already many resources on this very topic.

• The names debit() and credit() should be in verb form. In general, functions should be named as such since they perform an action. This makes the intent clear to the reader.

• Your "getters" should be const as they're not supposed to modify data members:

   int getSomeMember() const {
   return someMember;
   }
  

  mMinDenominations should also be const for the same reason. Initializer lists do also allow initialization of const members (be aware that constructors cannot do this).

• You can replace displayBalance() with an overload of operator<<:

   friend std::ostream& operator<<(std::ostream& out, ATM const& obj) {
   return out << obj.mAccount.getBalance() << '\n';
   }
  

  Since mAccount is private, this will have to be defined within the class.

This looks quite readable and easy to follow. I just have a few things that have stuck out to me:

• It would not be good to use a floating-point type for currency, and it's especially worse for a more accurate type like a double. For instance, you can wind up with a value like 0.0001, which is not an ideal value to deal with. Unfortunately, C++ doesn't have a standard library to deal with this, such as Java's BigDecimal. You need an integer type for this, which avoids these problems.

  Refer to this, this and this for more info. There are already many resources on this very topic.

• Your "getters" should be const as they're not supposed to modify data members:

   int getSomeMember() const {
   return someMember;
   }
  

  mMinDenominations should also be const for the same reason. Initializer lists do also allow initialization of const members (be aware that constructors cannot do this).

  Moreover, consider redesigning this to avoid getters. They (and also setters) are generally bad for encapsulation as they expose implementation details. You may not even need them here, either.

• You can replace displayBalance() with an overload of operator<<:

   friend std::ostream& operator<<(std::ostream& out, ATM const& obj) {
   return out << obj.mAccount.getBalance() << '\n';
   }
  

  Since mAccount is private, this will have to be defined within the class. This will still work, even if you remove the getters.