4
\$\begingroup\$

I started learning OOP programming style in PHP. After end of the course, I wrote a simple calculator in OOP style. Can some one check my code and give me feedback?

//Calculator Class
<?php

use lib\calculator\classes as classes;

spl_autoload_register(function ($className) {
    require_once $className . ".php";
});

class calculator
{
    private $numOne, $numTwo;

    /**
     * @return mixed
     */
    public function getNumOne()
    {
        return $this->numOne;
    }

    /**
     * @param mixed $numOne
     */
    public function setNumOne($numOne)
    {
        $this->numOne = $numOne;
    }

    /**
     * @return mixed
     */
    public function getNumTwo()
    {
        return $this->numTwo;
    }

    /**
     * @param mixed $numTwo
     */
    public function setNumTwo($numTwo)
    {
        $this->numTwo = $numTwo;
    }

    /**
     * calculator constructor.
     * @param $numOne
     * @param $numTwo
     */
    public function __construct($numOne, $numTwo)
    {
        $this->numOne = $numOne;
        $this->numTwo = $numTwo;
        echo "Welcome to my simple calculator!" . $this->HtmlLineBreak();
        echo "This project's goal is use OOP and namespaces to simulate a simple calculator!" . $this->HtmlLineBreak();
        echo "number one is: " . $this->numOne . " number two is: " . $this->numTwo . $this->HtmlLineBreak();
    }


    /**
     * @return string
     */
    private function HtmlLineBreak()
    {
        return "<br>";
    }

    /**
     * @param $operator
     */
    public function calculate($operator)
    {
        if (isset($this->numOne, $this->numTwo)) {
            echo "<body style='background-color: black; color: white'>";
            switch ($operator) {
                case "+":
                    echo "<span style=\"color: green;\">********** PLUS **********</span>" . $this->HtmlLineBreak();
                    echo "Number one: " . $this->numOne . " number two: " . $this->numTwo . $this->HtmlLineBreak();
                    echo "Try to create plus object ..." . $this->HtmlLineBreak();
                    $plus = new classes\plus\plus($this->numOne, $this->numTwo);
                    echo "Plus object created!" . $this->HtmlLineBreak();
                    echo "Plus result is:" . $this->HtmlLineBreak();
                    echo $plus->calculate() . $this->HtmlLineBreak();
                    break;
                case "-":
                    echo "<span style=\"color: green;\">********** MINUS **********</span>" . $this->HtmlLineBreak();
                    echo "Number one: " . $this->numOne . " number two: " . $this->numTwo . $this->HtmlLineBreak();
                    echo "Try to create minus object ..." . $this->HtmlLineBreak();
                    $minus = new classes\minus\minus($this->numOne, $this->numTwo);
                    echo "Minus object created!" . $this->HtmlLineBreak();
                    echo "Minus result is:" . $this->HtmlLineBreak();
                    echo $minus->calculate() . $this->HtmlLineBreak();
                    break;
                case "*":
                    echo "<span style=\"color: green;\">********** MULTIPLE **********</span>" . $this->HtmlLineBreak();
                    echo "Number one: " . $this->numOne . " number two: " . $this->numTwo . $this->HtmlLineBreak();
                    echo "Try to create multiple object ..." . $this->HtmlLineBreak();
                    $multiple = new classes\multiple\multiple($this->numOne, $this->numTwo);
                    echo "Multiple object created!" . $this->HtmlLineBreak();
                    echo "Multiple result is:" . $this->HtmlLineBreak();
                    echo $multiple->calculate() . $this->HtmlLineBreak();
                    break;
                case "/":
                    echo "<span style=\"color: green;\">********** DIVISION **********</span>" . $this->HtmlLineBreak();
                    echo "Number one: " . $this->numOne . " number two: " . $this->numTwo . $this->HtmlLineBreak();
                    echo "Try to create division object ..." . $this->HtmlLineBreak();
                    $division = new classes\division\division($this->numOne, $this->numTwo);
                    echo "Division object created!" . $this->HtmlLineBreak();
                    echo "Division result is:" . $this->HtmlLineBreak();
                    echo $division->calculate() . $this->HtmlLineBreak();
                    break;
                default:
                    echo "<span style=\"color:red;\">********** ERROR **********</span>" . $this->HtmlLineBreak();
                    echo "Operator ($operator) not supported! please use: + - * / (four simple calculation operators) Thank you!" . $this->HtmlLineBreak();
                    echo "<span style=\"color:red;\">********** ERROR **********</span>" . $this->HtmlLineBreak();
            }
            echo "</body>";
        } else {
            echo "First of all you must set your two numbers!" . $this->HtmlLineBreak();
        }
    }

    public function __destruct()
    {
        echo "DESTRUCTING EVERYTHING ..." . $this->HtmlLineBreak();
        if (isset($plus)) {
            unset($plus);
        }
        if (isset($minus)) {
            unset($minus);
        }
        if (isset($multiple)) {
            unset($multiple);
        }
        if (isset($division)) {
            unset($division);
        }
        echo "Done!" . $this->HtmlLineBreak();
        echo "******* DESTRUCTED *******" . $this->HtmlLineBreak();
    }
}

$calculator = new calculator(2.2, 4.3);
$calculator->calculate("+");
$calculator->calculate("-");
$calculator->calculate("*");
$calculator->calculate("/");
$calculator->calculate("!");
//Plus Class
<?php


namespace lib\calculator\classes\plus;


class plus
{
    private $numOne, $numTwo;

    /**
     * plus constructor.
     * @param $numOne
     * @param $numTwo
     */
    public function __construct($numOne, $numTwo)
    {
        $this->numOne = $numOne;
        $this->numTwo = $numTwo;
    }

    /**
     * @return mixed
     */
    public function getNumOne()
    {
        return $this->numOne;
    }

    /**
     * @param mixed $numOne
     */
    public function setNumOne($numOne)
    {
        $this->numOne = $numOne;
    }

    /**
     * @return mixed
     */
    public function getNumTwo()
    {
        return $this->numTwo;
    }

    /**
     * @param mixed $numTwo
     */
    public function setNumTwo($numTwo)
    {
        $this->numTwo = $numTwo;
    }

    /**
     * @return false
     */
    public function calculate()
    {
        if (isset($this->numOne, $this->numTwo)) {
            return $this->numOne + $this->numTwo;
        }else{
            return false;
        }
    }
}
//Multiple Class
<?php


namespace lib\calculator\classes\multiple;


class multiple
{
    private $numOne, $numTwo;

    /**
     * multiple constructor.
     * @param $numOne
     * @param $numTwo
     */
    public function __construct($numOne, $numTwo)
    {
        $this->numOne = $numOne;
        $this->numTwo = $numTwo;
    }

    /**
     * @return mixed
     */
    public function getNumOne()
    {
        return $this->numOne;
    }

    /**
     * @param mixed $numOne
     */
    public function setNumOne($numOne)
    {
        $this->numOne = $numOne;
    }

    /**
     * @return mixed
     */
    public function getNumTwo()
    {
        return $this->numTwo;
    }

    /**
     * @param mixed $numTwo
     */
    public function setNumTwo($numTwo)
    {
        $this->numTwo = $numTwo;
    }

    /**
     * @return false|float|int
     */
    public function calculate()
    {
        if (isset($this->numOne, $this->numTwo)) {
            return $this->numOne * $this->numTwo;
        }else{
            return false;
        }
    }

}
//Minus Class
<?php


namespace lib\calculator\classes\minus;


class minus
{
    private $numOne, $numTwo;

    /**
     * minus constructor.
     * @param $numOne
     * @param $numTwo
     */
    public function __construct($numOne, $numTwo)
    {
        $this->numOne = $numOne;
        $this->numTwo = $numTwo;
    }

    /**
     * @return mixed
     */
    public function getNumOne()
    {
        return $this->numOne;
    }

    /**
     * @param mixed $numOne
     */
    public function setNumOne($numOne)
    {
        $this->numOne = $numOne;
    }

    /**
     * @return mixed
     */
    public function getNumTwo()
    {
        return $this->numTwo;
    }

    /**
     * @param mixed $numTwo
     */
    public function setNumTwo($numTwo)
    {
        $this->numTwo = $numTwo;
    }

    /**
     * @return false
     */
    public function calculate()
    {
        if (isset($this->numOne, $this->numTwo)) {
            return $this->numOne - $this->numTwo;
        } else {
            return false;
        }
    }
}
//Division Class
<?php


namespace lib\calculator\classes\division;


class division
{
    private $numOne, $numTwo;

    /**
     * division constructor.
     * @param $numOne
     * @param $numTwo
     */
    public function __construct($numOne, $numTwo)
    {
        $this->numOne = $numOne;
        $this->numTwo = $numTwo;
    }

    /**
     * @return mixed
     */
    public function getNumOne()
    {
        return $this->numOne;
    }

    /**
     * @param mixed $numOne
     */
    public function setNumOne($numOne)
    {
        $this->numOne = $numOne;
    }

    /**
     * @return mixed
     */
    public function getNumTwo()
    {
        return $this->numTwo;
    }

    /**
     * @param mixed $numTwo
     */
    public function setNumTwo($numTwo)
    {
        $this->numTwo = $numTwo;
    }

    /**
     * @return false|float|int
     */
    public function calculate()
    {
        if (isset($this->numOne, $this->numTwo)) {
            return $this->numOne / $this->numTwo;
        } else {
            return false;
        }
    }
}

Note: namespace standards followed by code!

\$\endgroup\$
1
  • 2
    \$\begingroup\$ Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Please check that I haven't misrepresented your code, and correct it if I have. \$\endgroup\$ Jun 13 at 7:17
2
\$\begingroup\$

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.
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.