HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Is this following Separation of Concerns and PHP OOP standards?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisphpconcernsfollowingstandardsandoopseparation

Problem

I've been working with PHP for a while now, but unfortunately haven't delved into the OO side of it until recently. I've been reading up on separation of concerns and OOP best practices, and I think I have an idea of what is expected, but there's always room to improve. At any rate, is this heading in the right direction for PHP OOP?

The Class (profile.php)

```
firstName;
$fullName .= ($this->middleName!="") ? " " . $this->middleName . " " : " ";
$fullName .= $this->lastName;
$fullName .= ($this->suffix!="") ? ", " . $suffix : "";
return $fullName;
}

/**
* @desc wraps email in anchor tag
* @param - n/a
* @return string - link to email
*/
public function display_email() {
return "eMail . "'>". $this->eMail ."";
}

/**
* @desc splits phone number, adds hyphens and parenthesis
* @param - n/a
* @return string - formatted phone number
*/
public function display_phone() {
return "(" . substr($this->phoneNumber, 0, 3) . ") " . substr($this->phoneNumber, 3, 3) . "-" . substr($this->phoneNumber, 6, 4);
}

/**
* @desc splits fax number, adds hyphens and parenthesis
* @param - n/a
* @return string - formatted fax number
*/
public function display_fax() {
return "(" . substr($this->faxNumber, 0, 3) . ") " . substr($this->faxNumber, 3, 3) . "-" . substr($this->faxNumber, 6, 4);
}

/**
* @desc wraps profilePhoto in image tag, using the display_name as alt text
* @param - n/a
* @return string - image tag
*/
public function display_photo() {
return "display_name() . "' src='/profileImages/". $this->profilePhoto . "' class='profilePhoto'>";
}

/**
* @desc wraps profileWebsite in anchor tag
* @param - n/a
* @return string - link to profileWebsite
*/
public function display_website() {
return "profileWebsite . "' target='_blank'>". $this->profileWebsite ."";
}

/**
* @desc displays the complete address, wrapped in a p tag
* @param - n/a

Solution

Your profile.php file is doing way too much. A good test to see if you are adhering to the Separation of Concerns principle is: Do one thing, and do it well.

It seems profile.php is doing the following things:

  • Gathering information from the HTTP request params



  • Defining a "model" class called Employee



  • The Employee class is formatting HTML



  • The bottom of the file is querying the database for the Employee record



You should read up on the Model-View-Controller programming pattern. Since you are writing PHP, check out PHP Objects, Patterns and Practices. It's a great read about OOP in PHP and how you can apply many of the common programming patterns.

Context

StackExchange Code Review Q#48165, answer score: 2

Revisions (0)

No revisions yet.