3

I wrote the following lines of code:

$this->validate($group);

$this->em->persist($group);
$this->em->flush();

Method "validate" will throw an exception if $group is not valid. The issue is, it seems kind of "fragile". If another developer changed this code, maybe he would accidently move the validate method and the entity manager would save the object into the database without validating it.

Do you think the following lines of code are better or am I just overthinking it?

$validGroup = $this->validate($group);

$this->em->persist($validGroup);
$this->em->flush();

Are there any patterns for validation?

Dave Schweisguth
  • 36,475
  • 10
  • 98
  • 121
Lukas Lukac
  • 7,766
  • 10
  • 65
  • 75

3 Answers3

2

Template Pattern suits specific to this problem.

In Template pattern, an abstract class exposes defined way(s)/template(s) to execute its methods. Its subclasses can override the method implementation as per need but the invocation is to be in the same way as defined by an abstract class. This pattern comes under behavior pattern category.

abstract class MyTemplate{
   void myPersist(){
      validate();
      persist();
      flush();
}
Rahul
  • 15,979
  • 4
  • 42
  • 63
1

I would validate inside persist, so that it's impossible to persist an unvalidated object. If em is third-party code and you don't want to change it, wrap em in your own object that validates before writing to the database and use that everywhere.

There are a couple of options for designing em or your wrapper so that you only need to write it once and can use a single instance everywhere in your program. You can either

  • give the object being persisted the responsibility of validating itself, e.g. require all persistable objects to have an is_valid method. em or your wrapper would ask the object to validate itself and throw an exception if the object is invalid. This is probably the best solution if you're in a position to add behavior to persistable objects, since it puts the the idea of validity in the same place as the data.

  • have em or your wrapper detect the type of the object, look up (in a registry or by naming convention) a validator object that knows how to validate objects of that type, ask the validator to validate the object being persisted, and, again, throw an exception if the object is invalid. This is more complicated, but might be helpful if validation is complex enough to merit a separate object or necessary if you can't add behavior to persistable objects.

Dave Schweisguth
  • 36,475
  • 10
  • 98
  • 121
  • Wouldn't this lead to a situation where I would need an em wrapper for every type of object I want to persist and validate? – Lukas Lukac Mar 13 '16 at 12:17
  • I expanded my answer to discuss two validation strategies that don't have that issue. – Dave Schweisguth Mar 13 '16 at 15:14
  • the second option seems better I will mark it as an answer even tho I am curious more about how to solve it without adding any complexity. Let's say I have a method that is not so common and simply have: methodThatHaveToBeCalledFirst() and methodThatHaveToBeCalledSecond() ... what do u do (without adding complexity) to avoid this? (and both return void) – Lukas Lukac Mar 16 '16 at 19:19
  • What I said is about as simple as I can think of. If you want to enforce that coding sequence, it needs to be in one place somewhere: your persistence manager, your persisted objects, or both. So that much more complexity just comes with the requirement. – Dave Schweisguth Mar 16 '16 at 21:31
0

This question touches a very important and significant problem in interface design. The problem is "undeclared ordering" i.e. the order of method calls is implicit and undeclared. It is a source of a lot of problems and confusion.

There are two ways to refactor it (at least). Both are already provided above, I am just summarizing them.

  1. Use the result of one method call as an input to another call (as OP already did). This ensures order. It is applicable and useful when the result of the first method call is used multiple times.
  2. Call the first method inside the second method (or vice-versa). This will also solve the problem since the user of the interface has to call a single method.
Tushar
  • 790
  • 7
  • 12