Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Better way to populate entity from request?

I'm working with a Symfony 2.1 application and I have a lot of parameters being sent via a POST request and I'm looking for a smarter way to take each request parameter and populate the my entity class. I'm looking to avoid writing $entity->setMyParam($my_param) expressions for n request parameters. For example, here is a snippet of my entity:

namespace Brea\ApiBundle\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Validator\Constraints as Assert;

/**
 * Brea\ApiBundle\Entity\Distributions
 *
 * @ORM\Table(name="distributions")
 * @ORM\Entity
 */
class Distributions
{
  /**
   * @var string $recordType
   *
   * @ORM\Column(name="record_type", type="string", nullable=false)
   * @Assert\NotBlank()
   * @Assert\Choice(choices = {"a", "b", "c", "d", "e"}, message = "Choose a valid record type")
   */
  private $recordType;

  /**
   * Set recordType
   *
   * @param string $recordType
   */
  public function setRecordType($recordType)
  {
    $this->recordType = $recordType;
  }

  /**
   * Get recordType
   *
   * @return string 
   */
  public function getRecordType()
  {
    return $this->recordType;
  }
}

My controller attempts to take each request, camelcase the parameters and set the value of the request param to the entity:

public function createRecordAction(Request $request, $id)
{
  $distribution = new Distributions();
  $params = $request->request;

  foreach ($request->request->all() as $param=>$value)
  {
    if ($param == "_method")
      continue;

    $function = "set".str_replace(' ','',ucwords(preg_replace('/[^A-Z^a-z^0-9]+/',' ',$param)));
    $distribution->$function($value);
  }
}

It works, but my misgiving about this approach is that I would need to run this code in every controller that does a similar thing. I can refactor it into a parent class as a method to avoid duplicating code, but I'm curious if this is a good practice. I looked for something in the Symfony framework that does this already but all I could find were examples of binding the request to a form.

like image 336
Duane Gran Avatar asked Dec 27 '12 19:12

Duane Gran


2 Answers

First of all: Warning!!

As I commented earlier, I would be very careful about using the code provided in your original post since you said it's data from a POST request, which means a client could inject any kind of data in it and call functions you might not have wanted on your object (or just cause a failure in your script by sending you non-existant function names).

I would actually read the conclusion first..! :) Then come back to Alt. 1 & 2.


Alternative 1:

With that being said, an alternative solution to your problem would be to give objects the responsability to fetch their own data. With granulated enough objects, you should not end up with bloated code, and you can define in each class which parameters to look for and which functions to call (and localize changes when you make a change to a class):

class BookInformation{
  private $publisher;
  private $name;
  private $price;

  public static createFromRequest($req){
    $publisher = Publisher::createFromRequest($req);
    $book = new BookInformation($publisher, $req['book_name'], $req['book_price']);
    $book->setABC($req['abc']);
    //...
    return $book;
  }

  public __construct($publisher, $name, $price){
    //...
  }
}

class Publisher{
  private $name;
  private $address;

  public static createFromRequest($req){
    return new Publisher($req['publisher_name'], $req['publisher_address']);
  }

  public __construct($name, $address){
    //...
  }
}

Like I said before, one big advantage with this method is that if you need to add new attributes to any of those classes, you don't have to edit the controllers at all and can just edit your "initialization from request method". Future changes will be localized to the modified class.

Of course, don't forget to validate any data sent from a user request (but that's just common sense).


Alternative 2:

Note that the first alternative is very similar to the Factory pattern (based on GoF's Abstract Factory), and that you could also implement a solution using that pattern:

class BookFactory{
  public createBookInformation($req){
    $publisher = $this->createPublisher($req);
    $book = new BookInformation($publisher, $req['book_name'], $req['book_price']);
    $book->setABC($req['abc']);
    //...
    return $book;
  }

  public createPublisher($req){
    return new Publisher($req['publisher_name'], $req['publisher_address']);
  }

  //createAnythingRelatedToBooks($req)...
}

That way, you have all the initializing procedures in a very cohesive class which only responsability is to initiliaze a certain family of objects based on a request object (and that is a VERY good thing). However, if you add attributes to one of those classes, you have to edit the appropriate Factory method too.


Conclusion

Please note that these two alternatives are actually not really alternatives... They can be used along with your initial code (especially the Factory one). They really only solve your last problem (the "where to put the code" problem).

However, even if you do sanitize the POST request and only call functions that are annoted (as stated earlier), I wouldn't really suggest it because I have the feeling that more complex business rules would ruin the design pretty fast (but maybe you've got it all covered already (?)). That is, I don't think you can easily plug in business rules in the initializing process since it's all automatic (it can't do any validation of the values since it could be any kind of value) and I feel like you'd end up "undo-ing" stuff after initialization (which I personally hate.. Lots of room for errors)!

For example, take the same two classes in Alternative 1 (BookInformation and Publisher).

Let's say a Book can only have a Publisher if that Publisher is already registered in a database and that their address has been confirmed (new publishers need to be created using another interface and then have their address confirmed before they can be linked to a book).

Else, regardless of the request data, publisher should be set to XYZ. I have the feeling (I might be wrong) that to support those kind of rules, you'd have to actually construct the object (automatically) and then destroy/reassign the publisher attribute if it does not match certain rules. Now, if you have a pool of those Publisher objects in memory, you also need to remember to delete the wrongly created Publisher in that pool. And that's just one rule!

One thing you could do with your code to "fix" that issue would be to have a validation method for each setter (validXYZ()), but that's starting to look like a design that would fall apart pretty quickly if validations are dependent on other objects/data...

I don't really have anything else to discourage you from using that code, but if you do, please keep us updated on how it works out after a year or two (once there has been some maintenance/new features added, etc...).

like image 144
mbinette Avatar answered Nov 19 '22 03:11

mbinette


I looked for something in the Symfony framework that does this already but all I could find were examples of binding the request to a form.

I would use Forms for this. Even if the HTTP request is not performed from an HTMl form, you can just bind the Request to a form instance: it will take care of all the data injection and validation.

And plus, in case you'll ever need HTML forms, you'll have them ready ^^.

like image 45
moonwave99 Avatar answered Nov 19 '22 03:11

moonwave99