How best to handle the options method?

With a colleague, a dispute arose on the subject of designing your classes.

There is a class to calculate the delivery of goods in a certain city two delivery services: sdek and EMC (for simplicity, only two. There may be more).

the class has two dependencies, the calculator sdaca and calculator EMC. How they work in this case is not important.

There is a public method calculate, which takes two parameters: an array of goods and the city.

There are also some internal methods that are responsible for the calculation of specific services cdekCourier cdekPickup emsPickup.
Each of these methods are required for calculation of the same goods and the city.

The essence of the dispute:
One finds that in each method (cdekCourier cdekPickup emsPickup) you need to pass parameters.
The other said that the goods and the city is better to write to properties, and then refer to them from these methods.

To argue each other your variant can't.

The class itself in two variants given below.

The question is, which option is better and why? Or maybe they're both bad?)

Option 1

class DeliveryCalculator1
{
 protected $cdekCalculator;
 protected $emsCalculator;

 public function __construct($cdekCalculator, $emsCalculator)
{
 $this->cdekCalculator = $cdekCalculator;
 $this->emsCalculator = $emsCalculator;
}

 public function calculate($products, $city): array
{
 $conditions = [];

 if ($condition = $this->cdekCourier($products, $city)) {
 $conditions[] = $condition;
}

 if ($condition = $this->cdekPickup($products, $city)) {
 $conditions[] = $condition;
}

 if ($condition = $this->emsPickup($products, $city)) {
 $conditions[] = $condition;
}

 return $conditions;
}

 protected function cdekCourier($products, $city): array
{
 //here is some logic, which defines the terms of delivery if it is possible at all
 $condition = $this->cdekCalculator->courier($products, $city);

 return $condition ?: null;
}

 protected function cdekPickup($products, $city): array
{
 //here is some logic, which defines the terms of delivery if it is possible at all
 $condition = $this->cdekCalculator->pickup($products, $city);

 return $condition ?: null;
}

 protected function emsPickup($products, $city): array
{
 //here is some logic, which defines the terms of delivery if it is possible at all
 $condition = $this->emsCalculator->pickup($products, $city);

 return $condition ?: null;
}
}

Option 2

class DeliveryCalculator2
{
 protected $cdekCalculator;
 protected $emsCalculator;

 protected $products;
 protected $city;

 public function __construct($cdekCalculator, $emsCalculator)
{
 $this->cdekCalculator = $cdekCalculator;
 $this->emsCalculator = $emsCalculator;
}

 public function calculate($products, $city): array
{
 $this->products = $products;
 $this->city = $city;

 $conditions = [];

 if ($condition = $this->cdekCourier()) {
 $conditions[] = $condition;
}

 if ($condition = $this->cdekPickup()) {
 $conditions[] = $condition;
}

 if ($condition = $this->emsPickup()) {
 $conditions[] = $condition;
}

 return $conditions;
}

 protected function cdekCourier(): array
{
 //here is some logic, which defines the terms of delivery if it is possible at all
 $condition = $this->cdekCalculator->courier($this->products, $this->city);

 return $condition ?: null;
}

 protected function cdekPickup(): array
{
 //here is some logic, which defines the terms of delivery if it is possible at all
 $condition = $this->cdekCalculator->pickup($this->products, $this->city);

 return $condition ?: null;
}

 protected function emsPickup(): array
{
 //here is some logic, which defines the terms of delivery if it is possible at all
 $condition = $this->emsCalculator->courier($this->products, $this->city);

 return $condition ?: null;
}
}

April 7th 20 at 10:58
2 answers
April 7th 20 at 11:00
Reason is quite simple and even to refer to specific paragraphs from the book Implementing domain-driven design, namely, the fact that the author writes about services. The author of the book quite clearly explains why the services should not have state.

You have a classic service and you have a problem, just because in principle you don't have a good design of your service.

First you need to design the interface CalculatorInterface

interface CalculatorInterface {
 public function calculate($products, $city);
}


Next you need to create two implementations of this interface CdekCalculator and EmsCalculator

CdekCalculator class implements CalculatorInterface {
 public function calculate($products, $city) {
 // implementation of the settlement through cdek
}
}

EmsCalculator class implements CalculatorInterface {
 public function calculate($products, $city) {
 // implementation of the settlement through ems
}
}


And now your class of shipping will look like this

class Delivery {
 private $calculator;

 public function __construct(CalculatorInterface $calculator) {
 $this->calculator = $calculator;
}
 public function calculate($products, $city) {
 //here is some logic, which defines the terms of delivery if it is possible at all
 return $this->calculator->calculate($products, $city);
}
}


The subject matter is no more. You simply pass in the class of Delivery desired implementation of the calculator. > Pseudo code above is not php at all and only a basic idea of how it should be. You probably immediately the question arises, how do you then display all calculations delivery to your UI. But sorry can't write everything for you app.

This is basic stuff in an object-oriented programming. Polymorphism is called subtype. Without this knowledge it is impossible to write good object oriented code. Better both come to the conclusion that you urgently need to read some books about object-oriented programming and start with the basics, what is polymorphism, particularly a polymorphism of subtypes.

At all protected/private methods and branching of conditionals is almost always an indicator of spaghetti code and the first bell that you are writing procedural code using OOP syntax. You come to this conclusion if you develop using TDD, and then be able to Google your suggestions from other programmers.
Thank you for the detailed response. Yes, indeed, I often find myself thinking that we just write in OOP syntax, not in the PLO. A lot protected methods from which to remove while lack of knowledge and experience.

About the condition is understandable, but with examples not.

Two calculators, implemented as a single interface, OK. But what is the significance of Delivery class? We need it for multiple services at once, not one at a time. - Ewald97 commented on April 7th 20 at 11:03
PS moved in response to the TC wanted to write a comment

A good example, but it seems to me the author not satisfied, I understand that the author uses these classes in the estimator, where the shopping cart displays a list of available shippingof and respective quotas.
Described adapter may be confused by the topic starter which is likely according to his logic will be something like this
<?php
EstimateShipping function($shippings, $products, $city) 
{
$responseForView = [];
foreach($shippings as $shipping) {
 if (!$shipping->allowed()) {
continue;
}
 $cost = (new Delivery ($shipping->getCalculator()))->calculate($products, $city);
 if ($cost === false) {
continue;
}
 $responseForView[] = ['id'=> $shipping->getId(), 'name'=> $shipping->getName(), 'cost' => $cost];
}
return $responseForView;
}

and the question may arise as in the joke about camels - "Why do we need all this tuning in a zoo" - declan.Kulas commented on April 7th 20 at 11:06
@declan.Kulas, in this case, you can use the pattern composite. The basic idea is the same built on the polymorphism of the types, like nearly all design patterns.

I just wanted to convey the idea that when there is something of the same type but different implementation - that's the story of the polymorphism in any way. Balls, sticks, delivery, Keshi or logger doesn't matter. The important thing is that we must separate the different algorithms of the same type from the client through the interface. - isidro85 commented on April 7th 20 at 11:09
April 7th 20 at 11:02
Uncle Bob said "The best functions are those with no parameters!". So properties.

Find more questions by tags PHPPatterns of designing