Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

Take a look at the following code and let me know if its right, and if there is any room for improvements.

Strategy Class

<?php
/**
 * Draw Strategy class
 */
abstract class DrawStrategy
{
    abstract function draw(Shape $shape);
}
/**
 * This is ConsoleDraw and takes the shape object and draws accordingly, Drawign Algo is
 * implemented
 * here
 */
class ConsoleDraw extends DrawStrategy
{
    public function draw(Shape $shape)
    {
        echo "---------------";
    }
}

class WebDraw extends DrawStrategy
{
    public function draw(Shape $shape)
    {
        echo "=-=-=-=-=-=-=-=-=-";
    }
}

Shape Class ( That Composites Strategy Class )

<?php
/**
 * Design Pattern Impl. of Composite Pattern.
 *
 * Background:
 *
 * Shape is the Object, It can be specialised as Square or Rectangle and It can be drawn on
 * Web, or on Console / commandline
 *
 * The Drawing Algo is abstracted in Strategy Class and its used in the Shape Class
 *
 * Shape Class Uses DrawingStrategy and used accordingly, Uses ConsoleDraw, WebDraw as required
 *
 *
 */
include "strategy.php";

/**
 * Abstract Base Class Shape (Generalisation)
 *
 */
abstract class Shape
{
    public $length = 0;

    public function __construct( $_length )
    {
        $this->length = $_length;
    }

    /**
     * [draw this abstracts draw functionality]
     * @return [type] [description]
     */
    abstract function draw();

    /**
     * [area This is another function that calculates area]
     * @return [type] [description]
     */
    abstract function area();

}

/**
 *  SQuare Class Extends Shape and defines the implementation,
 *  It uses DrawStrategy as a component and so that it can draw either on the console or
 *  on the web.
 *
 */
class Square extends Shape
{
    public $length = 0;

    private $drawstrategy;

    public function __construct( $_length , DrawStrategy $_strategy )
    {
        $this->length = $_length;

        $this->drawstrategy = $_strategy;
    }

    public function area(){
        return $this->length * $this->length;
    }
    /**
     * draw - Takes up the Draw Strategy and draw on the current object
     * wether it is on console or web
     * @return [type] [description]
     */
    public function draw()
    {
        $this->drawstrategy->draw( $this );
    }

}

class Rectangle extends Shape
{
    private $drawstrategy;

    public function __construct( $_length, $_breadth, DrawStrategy $_strategy )
    {
        $this->length = $_length;
        $this->breadth = $_breadth;

        $this->drawstrategy = $_strategy;
    }

    public function area()
    {
        return $this->length * $this->breadth;
    }

    public function draw()
    {
        $this->drawstrategy->draw( $this );
    }
}

$shape[] = new Square( 4 , new ConsoleDraw );
echo "Shape 0 " .$shape[0]->area();
$shape[0]->draw();

$shape[] = new Square( 10, new WebDraw );
echo "Shape 1";
$shape[1]->draw();
share|improve this question

I think there might be room for improvement.

First of all, Strategy pattern relies on interfaces / common classes and you are combining Shape with concrete Strategy passing it to the constructor.

To be more specific, here you bound Square with ConsoleDraw and WebDraw:

$shape[] = new Square( 10, new ConsoleDraw );
$shape[] = new Square( 10, new WebDraw );

The question is, what if I passed any DrawStrategy to the Square object? Should it work? Everytime? This is my point. I think you should hide allowed strategies on list behind the concrete Shapes and pass context.

Based on context, pick the right strategy (let's say, we are in Console Context - pick the ConsoleDraw. If we were on DesktopAppliction context - throw Exception "Unsupported" or don't draw at all).

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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