2
\$\begingroup\$

I designed a framework to make the switch from procedural to OOP design. This question will contain multiple parts I will bold each question to make reading/dissecting easier. I attempted to look into these questions through Stack Overflow, Google, PHPReddit, etc. and I ran into a variety of answers, most of them bashed each other so I am currently torn in the direction to go/handle things.

Where should business logic be placed?

During my procedural programming learning I was used to placing all controller related logic within a specific required file that was just called by .htaccess rules. With my current approach to models (see next question), I was having trouble determining the proper place to define/set or call the business logic for the controller/page being viewed. I stumbled across services which is now where I place the logic for each controller method. Below is an example of my controller, and service layer for a page where users can accept, cancel, or deny friend requests.

Controller

<?php

/**
 *-----------------------------------------------------------------
 *  
 *  ACCOUNT PAGE
 * 
 */
namespace Controller\www;
use \Helper\Controller;

class Account extends Controller { 

    /**
     *  Friend Requests
     * 
     *  Users Can Accept, or Deny Incoming Friend Requests 
     *  Outgoing Friend Requests Can Be Cancelled
     */
    private function friendrequests() {

        // Page Service Layer    
        $service = $this->factory->make('AccountFriendRequests');


        /**
         *  Accept, Cancel, or Decline Friend Request Form Posted
         */
        if ($this->form->post('accept'))        {    $type = 'accept';     }
        elseif ($this->form->post('cancel'))    {    $type = 'cancel';     }
        elseif ($this->form->post('decline'))   {    $type = 'decline';    }

        if (isset($type)) {
            $service->post($type, $this->input->get($type, 'post', 'int'));
        }


        /**
         *  Return Rendered View
         */
        return $this->view->render('www/account/pages/friendrequests', [
            'received'  => $this->user->activeFriendRequests('receiver'),
            'sent'      => $this->user->activeFriendRequests('sender'),
            'expiration'=> $service->expiration
        ]);
    }
}

Service Layer

<?php

/**
 *-----------------------------------------------------------------
 * 
 *  ACCOUNT FRIEND REQUESTS SERVICE LAYER
 * 
 */
namespace Service\Account;
use \Helper\Service;

class FriendRequests extends Service {
    public $dependencies   = ['user', 'vars' => ['epoch']];

    // Time in Days to Expire Friend Requests
    public $expiration     = 7; 

    /**
     *  Set Class Dependencies
     * 
     *  After Dependencies Set Define Friend Requests Expiration 
     * 
     *  @param array $D         Assoc Array Containing the Dependencies Listed Above
     *                          Injected by Factory
     */ 
    public function __construct($D) {
        parent::__construct($D);

        // Set Expiration Values for Friend Requests
        $this->expiration = $this->config->setting('account.friendrequests.expiration',  $this->expiration);

        // Expire Old User Friend Request
        $this->user->expireFriendRequests($this->expiration);
    }


    /**
     *  Accept, Decline, Cancel Friend Requests
     *  
     *  @param string $type
     */
    public function post($type = 'accept', $id) {

        // Search DB For Friend Request ID  
        $request = $this->user->activeFriendRequest($id);


        /** 
         *  Request Exists - No Errors Produced - Complete Form Submission
         */
        if ($request && !$this->alert->errors()) {

            // Accept, Cancel, or Decline Request
            $message = $this->message($type);
            $this->user->updateFriendRequest($request['id'], $type);

            // Set Success Alert, Redirect
            $this->alert->success('Friend Invite ' . $message);          
            $this->redirect->to($this->config->site['uri']);   
        } 
        // Friend Request ID Invalid Set Error Alert
        elseif (!$request) {
            $this->alert->error('The Invite You Attempted To ' . $type . ' is Invalid or Does Not Exist. Please Try Again.');
        }
    }


    /**
     *  Determine Message To Set 
     * 
     *  @param string $type         Determines Message to Return
     */
    private function message($type) {
        if ($type == 'accept')      {    return 'Accepted Sucessfully!';    }
        elseif ($type == 'cancel')  {    return 'Has Been Cancelled';       }
        elseif ($type == 'decline') {    return 'Has Been Declined';        }
    }
}

Is there anything wrong with this approach? Particularly some standard I am breaking, or an issue that I can or will see in the future? Is there a better way to set all of this up?

How far should models go? Is my controller to model interaction correct? Am I overloading/including too much in my models?

My idea of a model is a mix between db interaction and output sanitation, while a few include setting date such as current user ID, etc. Below is an example of a model for custom pages (terms of use, privacy policy, etc.)

<?php

/**
 *-----------------------------------------------------------------
 *  
 *  CUSTOM PAGES MODEL
 * 
 */
namespace Model;
use \Helper\Model;

class CustomPages extends Model {         
    private $setValues = ['name', 'url', 'banner', 'content'];    


    /** 
     *  Fetch Custom Page From DB
     * 
     *  @param  mixed $input        String used to search for a custom page
     *  @return array               DB Results
     */  
    public function info($input = '') {    
        $params             = is_numeric($input) ? ['id' => (int) $input] : ['url' => $input];
        $params['status']   = 0;

        return $this->setPageData($this->db->fetch('pages', $params));
    }


    /**
     *  Set Page DB Data for Output Using Array Helper
     * 
     *  @param  array $page         Page Data from DB
     *  @return array               Page Data Set by Array Helper
     */
    private function setPageData($page = []) {
        if ($page) {
            $page = $this->array->setValues($this->setValues, $page);           

            // Define Full Banner Path and BBCode Content
            $page['banner']     = PAGES . $page['banner'];                  
            $page['content']    = $this->input->bbc($page['content']);      
        }
        return $page;
    }
}

There is not much going on here since it is an extremely basic page. This is how I structure my controllers.

$dependencies var is used in my own version of a dependency injector.

<?php

/**
 *-----------------------------------------------------------------
 *  
 *  CUSTOM PAGES CONTROLLER
 * 
 *  Future Plans:       Possibility of Adding HTML Via AdminCP
 * 
 */
namespace Controller\www;
use \Helper\Controller;

class CustomPages extends Controller {
    public $dependencies = ['custompage'];

    /**
     *  Custom Pages        About Us 
     *                      Terms of Use
     *                      etc.
     */
    public function index($url) {          

        // Search for Page Being Viewed If Empty Log Error & Display 404
        $page = $this->custompage->info($url);                             
        if(!$page) {                                                
            $this->error->display404('Missing or Disabled Custom Page');
        }                 


        /**
         *  Define Site Title & Display Page
         */
        $this->view->sitetitle('custompages',   ['pagename' => $page['name']]);
        $this->view->display('www/custompages', $page);
    }
}

Again, this is a very simple example to determine if I am properly implementing the OOP/MVC design pattern.

Where I begin to have a bit of issues/confusion with is when I have something like a logged in user or a page that requires much more data. Each model currently interacts with the database or sanitizes output, should I also add/allow vars to be assigned to the same model or would I create a entirely different class/model? My current user model interacts with the database but I also use it to set the current logged in user.

 <?php

/**
 *-----------------------------------------------------------------
 * 
 *  USER MODEL
 * 
 */
namespace Model;
use \Helper\Model;

class User extends Model {
    public $dependencies = ['config', 'time', 'vars' => ['epoch']];

    public $id      = 0;
    public $ip      = 'Undefined';
    public $browser = 'Undefined';

    public $is      = [
        'banned'    => false,
        'staff'     => false
    ];

    public $teams   = [];
    public $name, 
           $avatar, 
           $banner,
           $email;

    private $setValues = [
        'info'      => ['username', 'url', 'email', 'avatar', 'banner', 'ip'],
        'settings'  => ['timezone', 'gamertag', 'psn', 'league', 'facebook', 'twitter', 'twitch'],
        'location'  => ['name', 'address', 'country', 'city', 'state', 'zip']
    ];


    /** 
     *  Set User
     * 
     *  @param int    $userid       Userid To Set Within Model
     *  @param string $type         Determines Data Being Set
     */
    function set($userid, $type = '') {

        $this->id       = $userid;                                  
        $this->info     = $this->info();                            
        $this->settings = $this->settings();                       

        $this->name     = $this->info['username'];   
        $this->url      = $this->info['url'];       

        $this->avatar   = $this->info['avatar'];                    
        $this->banner   = $this->info['banner'];   

        $this->email    = $this->info['email'];                 


        // If User is Being Authenticated Define User Accounts
        if ($type == 'auth') {  

            // Define, and Iterate Through User Accounts                                   
            $accounts = $this->accounts();                        
            foreach ($accounts as $account => $value) {             
                $this->{$account} = $value;                         
            } 

            // Grab User Teams
            $this->teams        = $this->teams();

            // Is User Staff
            $this->is['staff']  = $this->staff('ip');            
        }
    }


    /** 
     *  Main User Information
     * 
     *  @param  mixed  $input       Input to Search For if Provided
     *  @param  string $column      Determines Column to Search For
     *  @param  string $rules       Additional Sql Rules Passed if Set
     *  @return array               User Location Data
     */
    public function info($input = '', $column = 'userid', $rules = '') {
        $input  = $input ? $input : $this->id;
        $data   = $this->db->fetch('users', [$column => $input], $rules);

        if ($data) {
            $data = $this->array->setValues($this->setValues['info'], $data);

            // Define User Url
            $data['url']    = $this->config->site['url']['www'] . $data['url'];

            // Define Full Avatar, and Banner Path
            $data['avatar'] = USER_AVATAR . $data['avatar'];
            $data['banner'] = USER_BANNER . $data['banner'];
        }
        return $data;
    }
}

There is much more methods/functions within this model but I shortened it for this question so ignore the missing account method etc.

At the start of my application an AuthHelper instantiates the user model and sets the logged in user if specific credentials are met. The application is heavily db/user driven so this approach just made sense for me but I am unsure if I am violating any rules/guidelines. I don't want to get used to programming incorrectly and be forced to redesign the app in say a month or so.

Why or should naming conventions such as IndexController or IndexModel be used?

I would understand if all of the files were just thrown into a folder and this was the only way to differentiate the classes within the directory but with namespaces I feel like adding the suffix is a bit redundant. I only ask because I see this in a lot of the books/tutorials that I have read.

Based on the questions above and the code, is there anything that I should be doing differently?

When setting the view file for the page, should the HTML be echo'd out from within the view class or returned to the index/bootstrap file to echo?

The root index file is the Bootstrap of my application. It instantiates a Controller, which sets the View. Currently my View helper echo's the HTML within the $this->view->display(); method. Is there any reason to return it to the Controller which will return it to the Index/Bootstrap File then echo?

\$\endgroup\$
  • \$\begingroup\$ The desire to improve code is implied for all questions on this site. Question titles should reflect the purpose of the code, not how you wish to have it reworked. See How to Ask. \$\endgroup\$ – Jamal Jun 19 '15 at 18:29
  • 1
    \$\begingroup\$ This title is still too generic. It's obviously an implementation in PHP. The code's specific purpose is what should be in the title. \$\endgroup\$ – Jamal Jun 19 '15 at 18:53
  • \$\begingroup\$ @jamal is that better? \$\endgroup\$ – ICJ Jun 19 '15 at 19:15
  • \$\begingroup\$ Just three notes on your question: 1. The question is very long, which means that it will likely take longer for you to get an answer. 2. There are many different variants of MVC out there, you should probably go with what feels best for you. 3. Stating that it is "sample code" might trigger some people to close vote your question, as example code is off-topic here. Is it example code or more of an example project? Does the code you have posted here work and actually accomplish anything? \$\endgroup\$ – Simon Forsberg Jun 19 '15 at 19:47
  • \$\begingroup\$ I was going to split it into different questions but i thought it would be easier to add them into one so i can show the direction i have gone in and i can receive the appropriate feedback. By sample code i meant the code i have provided as a sample this is all currently working on gamrs.net i am cleaning it up and i want to make sure i am doing things properly for the next update. \$\endgroup\$ – ICJ Jun 19 '15 at 19:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.