3
\$\begingroup\$

I have something like a shop. It is limited to a checkout page, which will send an email. Nothing big, but a nice thing to learn more OOP.

Almost every time I develop new classes I am wondering how to organize the flow of depending methods. Here is an example, a first draft of what I have:

<?php

// Outside the class:
if (!empty($cms->request->post('add_to_cart'))) {
    $cart->addStrt();
}

OPTION A

// OPTION A: Build a "method chain" and move from one to the other, to the next …
Class Cart extends Shop {

    // This is the entry point and starts the "add process"
    public function addStart() {
        $this->checkQuantity(); // if qty is between min/max
        return false;
    }
    // Step 2
    public function checkQuantity() {
        if ($quantity < $max && $quantity > $min) {
            $this->checkInStock(); // move on to the next step
        }
        return false;
    }
    // Step 3
    public function checkInStock() {
        if ($availabe >= $qty_in_basket) {
            $this->addToCart(); // move on to the final step
        }
        return false;
    }
    // Step 4
    public function addToCart() {
        // Final step, all requirements ok
        $product_id = $this->request->post->product_id;
        $cart = $this->getCart();
        $product = $shop->getProduct($product_id);

        $cart->items->add($product);
        $added = $cart->save();

        return $added;
    }
}

OPTION B

// OPTION B: Have a single "add" method and check all requirements, sanitize, etc from a single method
Class Cart extends Shop {

    public function add() {
        $continue = $this->checkQuantity(); // if qty is between min/max
        if ($continue == false) return false;

        $continue = $this->checkInStock(); // if product still available
        if ($continue == false) return false;

        // Perhaps more checks, sanitize, etc.

        // Everythiing mus be ok, put item into cart
        $product_id = $this->request->post->product_id;
        $cart = $this->getCart();
        $product = $shop->getProduct($product_id);

        $cart->items->add($product);
        $added = $cart->save();

        return $added;
    }
}

I have read tutorials on some OOP patterns but can't seem to find which fits to my question. While Option B seems to be cleaner (to understand/extend) one thing I always read is never put multiple tasks into one method. Every method should only handle one specific task. But how/where do I control the flow?

Is this "Encapsulation" or "Decorator pattern" or "Dependency Injection"? I have no idea what I should google.

\$\endgroup\$
  • \$\begingroup\$ I think the question might be clearer if you took the common code out of option a and created a section called common code, then the options just contain the code that is different and it is easier to compare. The suggestion in the answer to look up the Single Responsibility Principle is a good one, You also might want to look up SOLID programming. \$\endgroup\$ – pacmaninbw Dec 31 '19 at 15:55
  • \$\begingroup\$ Out of curiosity, why are most of your methods always returning false? \$\endgroup\$ – Magnus Eriksson Jan 2 at 20:17
  • \$\begingroup\$ You can simplify statements like $continue = $this->checkInStock(); if ($continue == false) return false; to just if (!$this->checkInStock()) return false; by the way. \$\endgroup\$ – BadHorsie Jan 3 at 13:27
  • \$\begingroup\$ @MagnusEriksson It's fine for a method like checkInStock() to return a boolean which tells you if it's in stock or not. The methods in option A all returning false makes no sense though. \$\endgroup\$ – BadHorsie Jan 3 at 13:31
2
\$\begingroup\$

It's good to have single method to control flow as you have in B, but this method should be mostly calling other methods of your class (you also call there checkQuantity for example, which is also in A). That way add is still very flexible - you can override those methods (probably protected methods, not public), that it calls or as last resort override that method itself.

If you think B is more readable, imagine, there would be something like this->addToCart instead of this bit (and that extracted to separate method):

   $product_id = $this->request->post->product_id;
    $cart = $this->getCart();
    $product = $shop->getProduct($product_id);

    $cart->items->add($product);
    $added = $cart->save();

I'd google for "single responsibility principle".

| improve this answer | |
\$\endgroup\$
0
\$\begingroup\$

The problem I have with both versions is that you are adding functionality from all different items into 1 place.

The first red flag for me is

Class Cart extends Shop {

Does a Cart really extend a Shop? You also have functionality for an order, a product etc. inside the Cart.

What I have started to do is to break this down into what can be tangible classes, the scope of these classes will (hopefully) be clear enough to be extended to uses beyond what is here, but the idea is to look at what is responsible for what...

Class Shop  {
    // Details including name, address, contact details etc.

    public function getProduct ( int $productID ) : Product {

    }
}

// Details of a particular Products
Class Product   {
    // Product description etc.

The next method can be used in a multitude of situations, so in itself it is worth becoming a public method...

    public function getStock()   {
        return $this->quantity;
    }
}

An Order is created once a user has saved a Cart. You can use a Builder pattern to create an Order from all of the details required, you can include things like payment status/method, Customer details, delivery & special instructions etc.

For this example just say it is a Cart

// A user places an Order
class Order {
    public function __construct( Cart $cart )   {
        $this->cart = $cart;
    }

Again an Order can be processed all over the place, so other methods which allow you to update the items in a cart, change delivery details etc. can be used in different pages. Once they have been updated, the next method can process save of the Order.

    // Save order - returns true/false depending on if saved OK
    public function save()  {
        // Save order - all parts including items in the order, delivery details etc.
        return true;
    }
}

// A Cart is a list of items the customer wants to buy
Class Cart  {
    private $items = [];

    // Returns list of items out of stock or true if OK.
    public function checkStock ()   {
        $errors = [];
        // Check that each item in the cart has enough stock
        foreach ( $items as $item ) {
            if ( $item["product"]->getStock() > $item["quantity"] ) {
                // Store list of messages about item being out of stock
                $errors[] = 'Not enough stock';
            }
        }

        return !empty($errors) ? $errors : true;
    }

    // Add a particular item to a cart
    public function addItem ( Product $product, $quantity ) : bool {
        $this->items[] = [ "product" => $product, "quantity" => $quantity];

        return true;
    }
}

The above class are some of the tangible objects in the system, others may include Customers, Delivery types and anything else.

You then need something to put these together. This is the controller in MVC, and links to the actions the user makes in the front end.

When you click on save a cart (or depending on your requirements after payment etc.) This will call checkoutCart(). This uses the methods in the various other class, so asks a Cart to check the stock (which then calls the Product methods), it then creates an Order and assuming everything goes OK it calls the Order method to save the Order.

// Controller for Checkout page
Class CheckoutController  {
    // Final save to create an Order
    public function checkoutCart()  {
        // Retrieve cart for session
        $checkCart = $cart->checkStock();
        if ( $checkCart !== true ) {
            // Stock levels no longer can fulfill order
            // Exit with information about items no longer available
        }
        $order = new Order($cart);
        if ( $order->save() )   {
            // Message order created
        }
        else    {
            // Message order not created
        }
    }
}

// Controller for Item page
Class ItemPageController  {
    public function addItemToCart() {
        // Retrieve cart/shop for session

        $product = $shop->getProduct($this->request->post->product_id);
        $cart->addItem($product, $this->request->post->quantity);
        // Message to say item added
    }
}
| improve this answer | |
\$\endgroup\$

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.