Coupon code generator using the strategy pattern in PHP

Question

Below is the implementation in PHP. I aim to have a different coupon code and a different discount returned, based on the type of book (vintage or new).

<?php
interface BookOffer {
 function generateCoupon();
 function generateDiscount();
}
class VintageBookOffer implements BookOffer {
 function generateCoupon() {
 return "VINTAGECOUPONCODE";
 }
 function generateDiscount() {
 return 10.0;
 }
}
class NewBookOffer implements BookOffer {
 function generateCoupon() {
 return "NEWBOOKCOUPONCODE";
 }
 function generateDiscount() {
 return 5.0;
 }
}
class OfferGenerator {
 function generateOffer($bookType) {
 if($bookType == "vintage") {
 $bookObject = new VintageBookOffer();
 }
 else if($bookType == "newbook") {
 $bookObject = new NewBookOffer();
 }
 return $bookObject;
 }
}
$bookType1 = "vintage";
$bookType2 = "newbook";
$offerGenerator = new OfferGenerator();
$bookOffer1 = $offerGenerator->generateOffer($bookType1);
$bookOffer2 = $offerGenerator->generateOffer($bookType2);
echo "You chose book type " . $bookType1 . ". Your coupon code is " . $bookOffer1->generateDiscount() . ", and your discount is " . $bookOffer1->generateCoupon();
echo "You chose book type " . $bookType2 . ". Your coupon code is " . $bookOffer2->generateDiscount() . ", and your discount is " . $bookOffer2->generateCoupon();
?>

Does this look correct? The only reason I think it might be incorrect is because it does not use type hinting, which is frequently seen in the Strategy Pattern.

Answer 1

It's correct. But if you want to restrict return types to something specific (and I would, because db) you could write interface functions like this:

function generateCoupon(): string;
function generateDiscount(): int;

http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration

http://php.net/manual/en/functions.returning-values.php

Answer 2

Looking at the way how OfferGenerator initiate your Offers, I suggest instead of using if-else/switch statement, you can store all in some kind of collection or array. Here is an example:

# BookOffer.php
interface BookOffer {
 function generateCoupon();
 function generateDiscount();
}
# VintageBookOffer.php
class VintageBookOffer implements BookOffer {
 function generateCoupon() {
 return "VINTAGECOUPONCODE";
 }
 function generateDiscount() {
 return 10.0;
 }
}
# NewBookOffer.php
class NewBookOffer implements BookOffer {
 function generateCoupon() {
 return "NEWBOOKCOUPONCODE";
 }
 function generateDiscount() {
 return 5.0;
 }
}
# OfferGenerator.php
class OfferGenerator {
 protected $offerCollection = [];
 public function __construct()
 {
 $this->offerCollection = require('BookTypeOfferMapper.php');
 }
 function generateOffer($bookType) {
 return new $this->offerCollection[$bookType];
 }
}
# BookTypeOfferMapper.php
return [
 'vintage' => VintageBookOffer::class,
 'newbook' => NewBookOffer::class,
];
$bookType1 = "vintage";
$bookType2 = "newbook";
$offerGenerator = new OfferGenerator();
$bookOffer1 = $offerGenerator->generateOffer($bookType1);
$bookOffer2 = $offerGenerator->generateOffer($bookType2);
echo "You chose book type " . $bookType1 . ". Your coupon code is " . $bookOffer1->generateDiscount() . ", and your discount is " . $bookOffer1->generateCoupon();
echo "You chose book type " . $bookType2 . ". Your coupon code is " . $bookOffer2->generateDiscount() . ", and your discount is " . $bookOffer2->generateCoupon();

With this, whenever you have to add new type of books, you don't need to touch your OfferGenerator class and can just add into TypeOfferMapper file. No matter how many times you change/add/remove, your OfferGenerator will not change. As long as the new Offer adhere to the BookOffer interface.

IMO, this is better way than having to adjust your if-else/switch statement.