trjhtr

I wrote this basic calculator class using php.It will only execute the basic arithmetics operations so any suggestion for new functions add and improvements will be accepted. Thank you.

<?php

class Calculator

public $x , $y , $z; 

 public function __construct($x,$y,$z)
 $this->x = $x;
 $this->y = $y;
 $this->z = $z;
 

 public function calc()
 try
 if($this->x != 0 && $this->z != 0)

 switch($this->y)
 case '+': 
 return ($this->x + $this->z); 
 break;

 case '-':
 return ($this->x - $this->z);
 break;

 case '/':
 return ($this->x / $this->z);
 break; 

 case '*':
 return ($this->x * $this->z);
 break;

 
 else 

 throw new Exception('Operations with 0 not alloewd!'); 

 
 
 catch(Exception $e)
 return $e->getMessage(); 
 
 

?>

Im not going to comment on the implementation of the calculator (there are more well versed people out there) but I will comment on the coding style;

Return early (or in this case throw early)

I much prefer (I think most people do) to return or throw early to avoid more brackets and indentation.

So I would change the body of calc() to look like;

if($this->x == 0 && $this->z == 0)
 throw new Exception('Operations with 0 not allowed!'); 


switch($this->y)
 case '+': 
 return ($this->x + $this->z); 
 break;

 case '-':
 return ($this->x - $this->z);
 break;

 case '/':
 return ($this->x / $this->z);
 break; 

 case '*':
 return ($this->x * $this->z);
 break;

 default: 
 throw new Exception('Unknown operator');
 break;

Let Exceptions bubble up

If this is a calculator the excepted return is a number, not a string so doing the whole try catch and returning the $e->getMessage() is probably not what the developer expects to happen.

Variables In The Constructor

There is a lot of resources out there that suggest this is the correct way to pass data to a class.

To me variables passed to a constructor should be other classes this class depends on, the reason is so we don't have to add more manual definitions when using dependency injection (this is not true in every instance).

but here are some arguments why in this particular instance I wouldn't pass data to the constructor

• For dependency injection you are going to have to define where those variables are


• You will have to instantiate a new Calculator(x, y, z) every time you want to use it which seems wasteful as you return the values and not store them for another class to use

First thing that caught my eye was meaningless variable names: x, y, z. Although the class is extremely simple, it wouldn't hurt giving them proper names like operand, operator, etc.

Completely agree with Dan and burgeris on their comments. Just want to sum up everything and improve the solution.

First of all I have to notice that your code is not object-oriented, because it is not manipulating objects. In fact you're not creating any proper objects at all. If you want to develop it using OOP, you need to create at least 3 classes: Operand, Operator, Calculator. It is good to try it for educational purposes, so start google a good articles about OOP.

This task can also be done by using functions or by creating utility class. Let's try utility class approach. For this task it is natural for me to use static methods, so the code will look like this:

<?php

/**
 * Calculator. Not object-oriented utility class
 */
class Calculator float $operand2 Second operand
 * @return int

// Let's try to calculate something
echo Calculator::calculate(42, '*', 1);

How does this look like? For me code is: readable, well documented, easy to use. Variables has a proper names, comments are everywhere :) Good luck in learning!