Skip to main content
added 125 characters in body
Source Link
KIKO Software
  • 6.1k
  • 15
  • 24

A very short answer with a few things I noticed.

  • The __destruct() function in class calculator doesn't do anything useful.
  • The methods __construct(), getNumOne(), setNumOne(), getNumTwo() and setNumTwo() are the same in all five classes. You could use inheritance here, but to be honest, these methods are not really needed.
  • There is no separation between input, processing and output. See: Separation of concerns.
  • Classes should be flexible. Your calculator class allows for the addition of another operator: Simply create a new class, and use it in the main calculator class. However, adding an extra number will become quite complex. In other words, the class is inflexible with respect to the amount of numbers used. A method like newNumber(), and the use of an array, could solve that problem.
  • Inside the calculator::calculate() method, there is a lot of repetition of code. There must be a better way to do that.

A very short answer with a few things I noticed.

  • The __destruct() function in class calculator doesn't do anything useful.
  • The methods __construct(), getNumOne(), setNumOne(), getNumTwo() and setNumTwo() are the same in all five classes. You could use inheritance here, but to be honest, these methods are not really needed.
  • There is no separation between input, processing and output. See: Separation of concerns.
  • Classes should be flexible. Your calculator class allows for the addition of another operator: Simply create a new class, and use it in the main calculator class. However, adding an extra number will become quite complex. In other words, the class is inflexible with respect to the amount of numbers used. A method like newNumber(), and the use of an array, could solve that problem.

A very short answer with a few things I noticed.

  • The __destruct() function in class calculator doesn't do anything useful.
  • The methods __construct(), getNumOne(), setNumOne(), getNumTwo() and setNumTwo() are the same in all five classes. You could use inheritance here, but to be honest, these methods are not really needed.
  • There is no separation between input, processing and output. See: Separation of concerns.
  • Classes should be flexible. Your calculator class allows for the addition of another operator: Simply create a new class, and use it in the main calculator class. However, adding an extra number will become quite complex. In other words, the class is inflexible with respect to the amount of numbers used. A method like newNumber(), and the use of an array, could solve that problem.
  • Inside the calculator::calculate() method, there is a lot of repetition of code. There must be a better way to do that.
Source Link
KIKO Software
  • 6.1k
  • 15
  • 24

A very short answer with a few things I noticed.

  • The __destruct() function in class calculator doesn't do anything useful.
  • The methods __construct(), getNumOne(), setNumOne(), getNumTwo() and setNumTwo() are the same in all five classes. You could use inheritance here, but to be honest, these methods are not really needed.
  • There is no separation between input, processing and output. See: Separation of concerns.
  • Classes should be flexible. Your calculator class allows for the addition of another operator: Simply create a new class, and use it in the main calculator class. However, adding an extra number will become quite complex. In other words, the class is inflexible with respect to the amount of numbers used. A method like newNumber(), and the use of an array, could solve that problem.