Jump to content


Photo

How best to implement slim with OOP


  • Please log in to reply
12 replies to this topic

#1 NotionCommotion

NotionCommotion
  • Members
  • PipPipPip
  • Advanced Member
  • 1,551 posts

Posted 11 February 2018 - 05:37 PM

Trying to improve my OOP skills, and revamping how I have previously implemented slim APIs.  Any general remarks plus responses for the below specific items would be appreciated.

  1. See any/many problems with my implementation?  I still don't know if I have function in the right locations, and I was fishing for this in my https://forums.phpfr...-with-entities/ post.
  2. How should errors be communicated from my individual classes back to slim?  I've tried multiple implementations, however, have not totally decided.
    • Exceptions.  Given the many negative sediments for doing so, I have to believe it is not a good idea, and have stopped doing so.
    • Have my class methods return something like [0,error message] and [1, success message].  Seems a little clunky.
    • Have my objects set some error property, have the method return true/false, and then if false, get the error.  I brought this approach up several days ago, and requinx felt is wasn't the best.
    • Have my class method return [content,httpCode].  A little clunky.
    • Put more validation in the main slim file and have some of my classes return an error array as I am doing below.  Don't know if I like it.
    • Pass the response to my classes and have them update it.  I am starting to think that this is the "right" way to do it, but I haven't seen it done which makes me suspect.
  3. How should my validation class be provided to the entity?  Injected, inheritance, as a trait, etc?
// ##### WEBPAGE VIEW RESPONSES #####

$app->get('/users', function (Request $request, Response $response) {
    return $this->view->render($response, 'users.html',[
        'users'=>$this->router->pathFor('users'),
        'menu'=>(new Html())->getMenu('/users')
    ]);
});

// ##### API RESPONSES #####
define('_VER_','/api/1.0');

$app->get(_VER_.'/users', function (Request $request, Response $response) {
    return $response->withJson((new UserMapper($this->get('pdo')))->get());
})->setName('users');

$app->post(_VER_.'/users', function (Request $request, Response $response) {
    $userMapper = new UserMapper($this->get('pdo'));
    $user = new User($request->getParsedBody());
    if($errors=$user->validate()) {
        return $response->withJson($errors,422);
    }
    else {
        $userProperties=$userMapper->create($user);
        return $response->withJson($userProperties);
    }
});

$app->get(_VER_.'/users/{id}', function (Request $request, Response $response, $arg) {
    return $user = (new UserMapper($this->get('pdo')))->getById($arg['id'])
    ?$response->withJson($user->getProperties())
    :$response->withJson(["User with id $arg[id] does not exist"],422);
});

$app->put(_VER_.'/users/{id}', function (Request $request, Response $response, $arg) {
    $userMapper = new UserMapper($this->get('pdo'));
    if($user = $userMapper->getById($arg['id'])) {
        //Or maybe use $user->validate() first and then have $user->update() return true/false?
        if($errors=$user->update($request->getParsedBody())) {
            return $response->withJson($errors,422);
        }
        else {
            $userProperties=$userMapper->update($user);
            return $response->withJson($userProperties);
        }
    }
    else $response->withJson(["User with id $arg[id] does not exist"],422);
});

$app->delete(_VER_.'/users/{id}', function (Request $request, Response $response, $arg) {
    $userMapper = new UserMapper($this->get('pdo'));
    if($user = $userMapper->getById($arg['id'])) {
        return $errors = $userMapper->delete($user)
        ?$response->withJson($errors,422)
        :$response->withJson($user->getProperties());
    }
    else $response->withJson(["User with id $arg[id] does not exist"],422);
});
interface EntityMapperInterface
{
    public function get();
    public function create(User $user);
    public function update(User $user);
    public function delete(User $user);
    public function getById($id);
}
class UserMapper extends EntityMapper implements EntityMapperInterface
{

    public function create(User $user)
    {
        $userProperties=$user->getProperties();
        $userProperties->password=password_hash($userProperties->password, PASSWORD_DEFAULT);
        $stmt=$this->pdo->execute($this->pdo('INSERT INTO users(id, username, role, password) VALUES (null, :username, :role, :password)'));
        if($stmt->execute((array) $userProperties)) {
            unset($userProperties->password);
            $userProperties->id=$this->pdo->lastInsertId();
            return $userProperties;
        }
        else throw new EntityMapperException('Unknown error creating User');
    }

    public function update(User $user)
    {
        //Not complete
    }
    public function delete(User $user)
    {
        $stmt=$this->pdo->prepare('DELETE FROM users WHERE id=?');
        try {
            $stmt->execute([$user->id]);
            return true;
        } catch(\PDOException $e){
            throw New EntityMapperException('Foreign key error?');
        }
    }

    public function getByUserName($name)
    {
        $stmt=$this->pdo->prepare("$this->getQuery() WHERE u.username=?");
        $stmt->execute([$name]);
        return $stmt->fetch();
    }

    protected function getQuery()
    {
        return 'SELECT u.id, u.username, u.role, u.password, ur.access_level FROM users u INNER JOIN user_role ur ON ur.role=u.role';
    }
}
abstract class EntityMapper
{

    protected $pdo;

    public function __construct(\PDO $pdo)
    {
        $this->pdo=$pdo;
    }

    public function get()
    {
        $stmt=$this->pdo->query($this->getQuery());
        return $stmt->fetchAll();
    }

    public function getById($id)
    {
        $stmt=$this->pdo->prepare("$this->getQuery() WHERE u.id=?");
        $stmt->execute([$id]);
        return $stmt->fetch();
    }

    abstract protected function getQuery();

}
interface EntityInterface
{
    public function update($data);
    public function validate();
    public function getProperties();
}
class User extends Entity implements EntityInterface
{

    protected $firstname,
    $lastname,
    $email,
    $role,
    $accessLevel,
    $validator;

    public function __construct($properties)
    {
        //Should Validator be injected, should this class should be extended by it, should it be a trait, or should some other means be used?
        $this->validator=new Validator([
            'firstname'=>"string",
            'lastname'=>"string",
            'email'=>"string:email",
            'role'=>"string:[user,admin]",
            'accessLevel'=>"integer:betweenValue,1,10",
        ]);
        foreach($this->validator->iterate($properties) as $name=>$value) {
            $this->$name=$value;
        }
    }
}
Abstract class Entity
{
    public function update($data)
    {
        foreach($this->validator->iterate($properties) as $name=>$value) {
            $this->$name=$value;
        }
        return $this->validate();
    }


    public function validate()
    {
        return $this->validator->validate($properties);
    }


    public function getProperties()
    {
        return $this->validator->getProperties();
    }
}

 


NotionCommotion

#2 ignace

ignace
  • Moderators
  • Now mod flavored
  • 6,430 posts
  • LocationBelgium

Posted 12 February 2018 - 08:29 PM

You should use exceptions in your model. To what this translates to is up to the application.

The entity should not contain the logic for validation. A separate object should do this.

The same goes for getProperties() simply create the array from the getters.

Also _VER_ does not work. What happens to anyone using /api/1.0 when you set _VER_ to /api/2.0?

#3 ignace

ignace
  • Moderators
  • Now mod flavored
  • 6,430 posts
  • LocationBelgium

Posted 12 February 2018 - 08:32 PM

Use DI whenever possible.

<?php

$container['userMapper'] = function($c) {
    return new UserMapper($c['db']);
};

$container['userValidator'] = function($c) {
    return new UserValidator();
};


#4 NotionCommotion

NotionCommotion
  • Members
  • PipPipPip
  • Advanced Member
  • 1,551 posts

Posted 12 February 2018 - 09:50 PM

Thank you ignace,

 

Regarding exceptions, would you consider my UserMapper the model?  Would you frown on exceptions being used to convey state of user input?

 

I expected to either create new endpoints with new resources to deal with /api/2.0, or have the new endpoint access the same resource if applicable, however, I haven't given it too much thought.  Another option is regex, but again haven't thought much about it.  Is there a typical approach for this?

 

Regarding using a separate object for validation and getting the properties and using DI where possible, I think this is finally starting to set in with me partially based on your and requnix's replies to my https://forums.phpfr...-with-entities/ post.  Let me give an example of how I previously did things and how I think I will be doing more often in the future.  Say I was making a website builder, and had pages which displayed a list of pages, a list of menus, a list of users, a list of images, or whatever.  Just things that are not really the same, but have common actions performed on them such as CRUD and maybe others such as changing permissions, etc.

 

In the past, I would create an abstract list class, and then extend that class using PagesList, MenusList, UsersList, etc.  Sometimes, this worked well, other times, not so much.

 

Now I am thinking I should sometimes just create a single List class which they all directly use, but will pass its constructor a PagesMapper, MenusMapper, UsersMapper object to make it specific (should I be referring to this object as something different than "mapper"?).  Then I could have common code to say list all the entities, but this object would act differently to specifically performed the desired scope.

 

So, for validation, I would pass some more generic class a UserValidator object (which is probably extended from Validator), and that object would be fully accountable to perform all validation needs?

 

 

I hope this is not completely off base on what you are recommending...


NotionCommotion

#5 ignace

ignace
  • Moderators
  • Now mod flavored
  • 6,430 posts
  • LocationBelgium

Posted 13 February 2018 - 08:54 AM

I think you are caught in a loop. Most programmers fall pray to this, because all guru's recommend and promote the use of heavily marketed patterns, brainwashing anyone who reads their articles, so that anyone who doesn't use them feel out of the loop or stupid.

 

They are correct to use these patterns and their carefully crafted examples have a certain truth to them. Don't get too caught up in naming and adhere to programming principles more than naming. 

  1. Try to keep related things together. Like querying (CRUD) users from a DB. Sending out e-mails. 
  2. Don't think "in the future i might need". You don't.
  3. Don't be afraid to create "microscopic" classes like RegisterUser or SendRegistrationMail or whatever you can think up. Make sure it does only 1 thing.

I am speaking from experience here. I was caught in that same loop always trying to use patterns whenever I could without fully understanding them, I still don't understand most of them nor do I desire to do so. I let my code do the talking.

<?php

// Here's an example using the Action-Domain-Responder from Slim
// https://www.slimframework.com/docs/cookbook/action-domain-responder.html

$api->put(_VER_ . '/users', function(Request $request, Response $response) {
  try {
  
    $newUser = $this->apiUserService->create($request->getParsedBody());
  
    return $this->apiUserResponder->wasCreated($newUser); // 201
  
  } catch (PermissionDeniedError $e) {
    return $this->apiUserResponder->notAllowed(); // 401
  
  } catch (UserValidationError $e) {
    return $this->apiUserResponder->containsErrors($e);
  }
});

Simple to understand, easy to maintain. The ApiUserService does all the model related work for users, CRUD, checking permissions, validating, .. it delegates the actual task mind you to the appropriate class.

 

The Service class is merely a Facade (in guru speak).



#6 NotionCommotion

NotionCommotion
  • Members
  • PipPipPip
  • Advanced Member
  • 1,551 posts

Posted 13 February 2018 - 12:22 PM

*gasp*  You are using exceptions to valid user input....  The horror! The horror!
 
That being said, your code is very simple and easy to understand especially by using descriptive exceptions such as PermissionDeniedError and UserValidationError, and provides intuitive multiple dimension error feedback which solves all of my "how should errors be communicated" dilemmas.  I should not have bowed to pubic pressure that exceptions should only be used for "exceptional" behavior and never, ever user for user input, bla, bla, bla....
 
I am not instantly flip-flopping on my newly rekindled belief that exceptions can be used to communicate user behavior, but perhaps a cleaner Slim specific implementation might also use the $response to communicate state and look like the following:
 
$container['userMapper'] = function($c) {
    return new UserMapper($c['db']);
};

$container['userValidator'] = function($c) {
    return new UserValidator();
};

$container['apiUserService'] = function($c) {
    return new ApiUserService($this->userMapper, $this->userValidator);
};

$app->group('', function () use ($app) {

    //All endpoints within this group are limited to administrators only

    $api->post('/users', function(Request $request, Response $response) {
        return $this->apiUserService->create($request->getParsedBody(), $response);
    });

})->add(new Authenticator($container->get('settings')['jwt'], 'admin'));
class ApiUserService extends ApiService
{
    public function __construct(Mapper $userMapper, Validator $userValidator)
    {
        $this->mapper=$userMapper;
        $this->validator=$userValidator;
        /* maybe also include the response? */
    }

    public function create($dirtyUserInput, Response $response)
    {
        try {

            $cleanUserInput = $this->validator->sanitize($dirtyUserInput);
            $this->validator->validate($cleanUserInput);
            $user=$this->mapper->create($cleanUserInput);
            return $response->write($user)->withStatus(201);

        } catch (UserValidationError $e) {
            return $response->write($this->getStdErrorMsg(422))->withStatus(422);

        } catch (SomeOtherError $e) {
            return $response->write('as applicable')->withStatus(422);
        }

    }
}
Maybe/probably not a big deal, but $response is immutable so it uses a little more memory.  One part I am not sure about is whether the response should be passed to the create() method or the constructor.
 
While I agree that we all can get a little OCD about attempting to utilize some of these worshiped design patterns, one which I do think I need to better understand and utilize is how passing an object to a class can be used to modify the classes behavior.  On my previous post, I was attempting to inquire about this concept.  Before going too far down this rabbit hole, I would like some confirmation that it can end up somewhere good, and would appreciate any thoughts.
 
Thanks!

NotionCommotion

#7 ignace

ignace
  • Moderators
  • Now mod flavored
  • 6,430 posts
  • LocationBelgium

Posted 13 February 2018 - 01:13 PM

I would not pass any application specifics to the model.
 

<?php

// assume
interface UserMapper {}
class ApiUserMapper implements UserMapper {
  ..
}

class ApiUserService {
  public function __construct(UserMapper $userMapper, Validator $validator) {
    // ..
  }
  
  public function createUser(array $userData): User {
    if (!$this->validator->isValid($userData)) {
      throw new UserValidationError($this->validator, $userData);
    }

    return $this->userMapper->add($userData);
  }
}

The comment about not using exceptions for validation has some truth to it.

 

However the data passed must be valid for your model to do any work. It's possible to extract out the validation into a Form component and do a isSubmitted() && isValid() before calling your service but this means that EVERYWHERE in your application you first need to call the Form component before being able to call your service. And I'm far too lazy to add all these dependencies and make all those calls EVERYWHERE.

 

I use a Service because it's convenient, it does everything for me. No matter if that be in an API, Web, or CLI.



#8 NotionCommotion

NotionCommotion
  • Members
  • PipPipPip
  • Advanced Member
  • 1,551 posts

Posted 13 February 2018 - 04:23 PM

I suppose your comment about not passing ss any application specifics to the model makes sense.

 

Don't start getting me worried again about exceptions and validation!

 

I didn't previously pay attention to your remarks about it being a Service.  A "Service" is just a class/object with a common interface available to whatever needs to use it?


NotionCommotion

#9 kicken

kicken
  • Gurus
  • Wiser? Not exactly.
  • 3,361 posts
  • LocationBonita, FL

Posted 13 February 2018 - 04:59 PM

*gasp* You are using exceptions to valid user input.... The horror! The horror!


The main "problem" I have with when people use exceptions to validate user input is them throwing an exception for each validation step. For example:
public function validate(){
    if (empty($this->firstName)){
        throw new ValidationException('First name is required');
    }

    if (empty($this->lastName)){
        throw new ValidationException('Last name is required');
    }

    if (empty($this->email)){
        throw new ValidationException('Email is required');
    } else if (!filter_var($this->email, FILTER_VALIDATE_EMAIL)){
        throw new ValidationException('Email must be a valid email address.');
    }

    //...
}
That makes for a terrible experience because if there are multiple errors you only get the know about them one at a time.

If you're going to use exceptions for validating user input, at least do it in a way that lets you get all the error information at once.
public function validate(){
    $Errors = [];
    if (empty($this->firstName)){
        $Errors[] = 'First name is required';
    }

    if (empty($this->lastName)){
        $Errors[] = 'Last name is required';
    }

    if (empty($this->email)){
        $Errors[] = 'Email is required';
    } else if (!filter_var($this->email, FILTER_VALIDATE_EMAIL)){
        $Errors[] = 'Email must be a valid email address.';
    }

    if ($Errors){
        throw new ValidationException($Errors);
    }
}

As far as your overall validation / entity management implementations, I'd refer you to Symfony Validator and Doctrine ORM for inspriation as they seem to match up well to what you are wanting to do.
Did I help you out? Feeling generous? I accept tips via Bitcoin @ 14mDxaob8Jgdg52scDbvf3uaeR61tB2yC7
Kicken's World⦄ ⦃Recycle old CD's

#10 NotionCommotion

NotionCommotion
  • Members
  • PipPipPip
  • Advanced Member
  • 1,551 posts

Posted 14 February 2018 - 01:00 AM

The main "problem" I have with when people use exceptions to validate user input is them throwing an exception for each validation step. For example:

 

I didn't even conceive of doing such.  Guess that is a good thing :)


NotionCommotion

#11 ignace

ignace
  • Moderators
  • Now mod flavored
  • 6,430 posts
  • LocationBelgium

Posted 14 February 2018 - 07:58 AM

I didn't previously pay attention to your remarks about it being a Service.  A "Service" is just a class/object with a common interface available to whatever needs to use it?

 

A service is a direct answer to a question. It encapsulates all logic usually found in a controller so that this logic becomes reusable in other parts of the application. And that's the main reason I use services.

 

Now to what I mean with it being an answer to a question. Like the examples I gave above it could create a new user directly from the input. The same goes for getting, updating, and deleting.

<?php

interface UserService {
  public function get(int $id): ?User;
  public function create(array $data): User
  public function update(array $data, int $id): User
  public function delete(int $id): void;
}

It's a simplified interface which doesn't require any previous querying. If you apply CQRS, you would have (notice how all methods on UserCommandService return void):

<?php

interface UserQueryService {
  public function get(int $id): User
}

interface UserCommandService {
  public function create(array $data): void;
  public function update(array $data, int $id): void;
  public function delete(int $id): void;
}

Edited by ignace, 14 February 2018 - 08:01 AM.


#12 NotionCommotion

NotionCommotion
  • Members
  • PipPipPip
  • Advanced Member
  • 1,551 posts

Posted 14 February 2018 - 01:25 PM

Gotcha!

 

Also, just learned two new items:

  1. http://php.net/manua...ype-declaration
  2. https://en.wikipedia...uery_separation

What is the purpose of the question mark?  Assuming not a typo, where is it documented in the manual?  If I were to guess, it indicates User or NULL.  Thanks

public function get(int $id): ?User;

EDIT.  Would you ever consider catching all exceptions in the Slim handler instead of for each route?

$api->put(_VER_ . '/users', function(Request $request, Response $response) {
try {

$newUser = $this->apiUserService->create($request->getParsedBody());

return $this->apiUserResponder->wasCreated($newUser); // 201

} catch (PermissionDeniedError $e) {
return $this->apiUserResponder->notAllowed(); // 401

} catch (UserValidationError $e) {
return $this->apiUserResponder->containsErrors($e);
}

Edited by NotionCommotion, 14 February 2018 - 01:37 PM.

NotionCommotion

#13 kicken

kicken
  • Gurus
  • Wiser? Not exactly.
  • 3,361 posts
  • LocationBonita, FL

Posted 14 February 2018 - 03:09 PM

What is the purpose of the question mark?  Assuming not a typo, where is it documented in the manual?  If I were to guess, it indicates User or NULL.  Thanks

As you guessed, the ? means it can be null. Seems the manual hasn't been updated with that info yet, though a user note mentions it.
 

EDIT.  Would you ever consider catching all exceptions in the Slim handler instead of for each route?


If you can handle an exception, you should catch it at whatever level is appropriate. For example if a PermissionDeniedException is thrown then you'd likely handle that the same way regardless of the current route, so it makes more sense to catch it at a higher level rather than repeat the code over and over.

If you handle a UserValidationError exception the same for every route then you'd want to bump it up a level also, otherwise keep it at a route level so you can handle it in a manner appropriate for that route.
Did I help you out? Feeling generous? I accept tips via Bitcoin @ 14mDxaob8Jgdg52scDbvf3uaeR61tB2yC7
Kicken's World⦄ ⦃Recycle old CD's




0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users