\$\begingroup\$
\$\endgroup\$
1
I am learning decorator design pattern in PHP. Please see what i am doing is correct or wrong, would appreciate your review, feedback, comments, suggestions.
<?php
/**
* NotifierService interface is used as abstraction for Notification Mechanism
*
* It can be later implemented as EmailNotifierService, PushNotifierService, or
* any other Notification SErvice
*/
interface NotifierService {
public function Notify();
}
/**
* EmailNotifierService - is Email Decorator
* here the EmailSending is Implemented, and it can be used wherever it is
* required, Different Unique services that doesn not have common behaviours
* are utilised using decorator patterns
*/
class EmailNotifierService implements NotifierService {
public function __construct(){
$this->to = '[email protected]' ;//$toarr;
$this->from = '[email protected]' ;//$from;
$this->msg = 'This is a test message';//$msg
}
/**
* [sendMail Actual Implementation of Sending Email is put here]
* Kept simple for demo purposes
*/
private function sendMail(){
echo "Sending Email To" .PHP_EOL;
print_r($this->to);
echo "From: " .PHP_EOL;
print_r($this->from);
echo "Msg: " .PHP_EOL;
print_r($this->msg);
}
/**
* [Notify - Implementation of the interface method - Notify]
*/
public function Notify(){
echo "Notify function called" .PHP_EOL;
$this->SendMail();
}
}
class User implements NotifierService{
var $users = [];
var $notifierservice;
//Passing NotifierService Object, So that we can utilise its service
public function __construct(NotifierService $notifierservice){
echo "New User constructed";
$this->notifierservice = $notifierservice;
}
/**
* [adduser This function creates a new user, like new user signup,
* Whenever a new user signup, a Email is sent to the Website Admin]
* @param [Username] $u Username
*/
public function adduser($u){
$this->users[]=$u;
$this->Notify();// Calls the Notification
}
public function Notify(){
$this->notifierservice->Notify();
}
}
// New User is created, EmailNotificationService is wrapped in it
// So that whenever a new user is created, Admin gets an Email
$u1 = new User(new EmailNotifierService());
$u1->adduser('Ramkumar');
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Apr 3, 2017 at 11:51
-
\$\begingroup\$ What you may and may not do after receiving answers. I've rolled back Rev 2 → 1. \$\endgroup\$200_success– 200_success2017年04月06日 06:19:37 +00:00Commented Apr 6, 2017 at 6:19
1 Answer 1
\$\begingroup\$
\$\endgroup\$
1
- Just because a class like
User
has a dependency on another class (like an objects instance of typeNotifierService
) it does not mean that class has to implement that same interface. Here it seems thatUser
should not implement this interface at all. You can remove theNotify()
method on this class and just call theNotify()
method on the passed dependency in youradduser()
method. - I don't think decorator pattern makes sense at all for what you are trying to do here. You are not really trying to "decorate" objects adhering to
NotifierService
interface with additional functionality related to that interface, but rather simply consume a class of this interface inUser
. This is a straight dependency injection situation. - The class implementation of
EmailNotifierService
is pretty trivial here with hard-coded to/from/message values, almost to the point that this code does not meet code review criteria. Surely this is not something that is production ready. - Don't echo out output from classes such as this. Leave that to some other functionality. If you simply want to "log" out class activity, then actually log it (you know to application/server logs).
- Why does a class called
User
deal with multiple users? Should this class be renamed? - Don't start method/function names with uppercase letters like you do here with
Notify()
andSendMail()
. This should benotify()
andsendMail()
. $notifierservice
should probably be$notifierService
(camel-cased).adduser()
should beaddUser()
.- You should consider adding validation on public methods, either through type hinting of parameters (like you do for the notifier dependency) or through guarding clauses at beginning of method. Here, for example, you do nothing to ensure you are getting a valid user passed to your
adduser()
method.
answered Apr 3, 2017 at 13:58
-
\$\begingroup\$ Thanks a lot for every of your comments. +1 for Add validation on public methods with typehinting. \$\endgroup\$indianwebdevil– indianwebdevil2017年04月06日 03:40:36 +00:00Commented Apr 6, 2017 at 3:40
lang-php