How can I change my code to display information about failure of creation of a value object, when I want to process an array of such objects?

Question

Consider class Crate that is a value object. Crate represents a valid 3-dimensional box.

In constructor I validate the given parameters, and I throw an exception, if supplied dimension parameters are invalid:

class Crate
{
 protected $length, $width, $height;
 function __construct(float $length, float $width, float $height)
 {
 if ($length <= 0 || $width <=0 || $height <=0)
 throw new RuntimeException("Invalid Dimension");
 $this->length = $length;
 $this->width = $width;
 $this->height = $height;
 }
 function getDimensions()
 {
 echo $this->length . 'x' . $this->width . 'x' . $this->height;
 }
}

In my project I have a need to display dimensions of several crate box configurations. Namely, there is a showCrates method that accepts an array of Crate and then uses that array to display each Crate's dimensions, and other info.

function showCrates(array $crates)
{
 foreach($crates as $key => $crate)
 echo 'Crate #' . $key . ':' . $crate->getDimensions() . PHP_EOL;
}

showCrates works great when all parameters given to all Crate objects are valid. However, when a Crate object throws an Exception on an invalid parameter, code execution stops.

I want to be able to keep the execution going and still show the valid crates, but for invalid crates to have a message saying "Crate at index i was invalid".

Sample output that I expect is:

 Crate #1: 2x5x9
 Crate #2: 1x3x4
 Crate #3 is invalid, because supplied dimensions were: 0x0x0
 Crate #4: 5x6x3

Question

I am looking for a way that will let me display the above output without modifying Crate object itself.

Potential Solution that I am rejecting:

One way to solve my question is to allow invalid Crate objects and have an error boolean flag inside them stating whether a Crate is valid or not. Then I can keep showCrates method signature - to accept an array of Crate object - but modify it to check first, whether a Crate is valid, and to display desired output accordingly.

However, ideally I would like to NOT modify my Crate object unless there is a very strong argument to do so. I have constructed Crate object to be an immutable value object that can only exist if parameters supplied to it are valid, and to otherwise throws an Exception. As to why, I believe that this way the Crate is more easily testable and there are no extra checks to see whether or not Crate is valid.

Sample Calling Code

class CrateRequestHandler
{
 function handle(ServerRequestInterface $request)
 {
 // mocked up data from Request
 // this is sample data for showcase purposes only
 // it usually supplied by user or from database
 // and it is impossible to tell ahead of time 
 // if it will be correct for Crate purposes
 $crates = array();
 $crates[0] = new Crate(2, 5, 9);
 $crates[1] = new Crate(1, 3, 4);
 $crates[2] = new Crate(0, 0, 0);
 $crates[3] = new Crate(5, 6, 3);
 // send to View
 $this->showCrates($crates);
 }
 function showCrates(array $crates)
 {
 foreach($crates as $key => $crate)
 echo 'Crate #' . $key . ':' . $crate->getDimensions() . PHP_EOL;
 }
}

• Ewan: showCrates doesnt actually call your constructor atm?
• Dennis: Correct. showCrates only accepts the previously created $crates, which is an array of Crate object created elsewhere. I have added sample calling code.

Answer 1

To me, I'll create another value object called CrateCreation to model the creation of your crates and let showCrates accept the list of CrateCreation objects instead of the list of the actual Crate objects. The CrateCreation object holds these pieces of information:

• The original input values.
• The actual Crate object if the creation is success.

Sample class:

<?php
class CrateCreation
{
 private $length, $width, $height;
 private $crate;
 function __construct(float $length, float $width, float $height)
 {
 $this->length = $length;
 $this->width = $width;
 $this->height = $height;
 try {
 $this->crate = new Crate($length, $width, $height);
 }
 catch(RuntimeException $e)
 {
 // do some loging here
 }
 }
 // getters here
 // the same `getDimensions` method here
}

Then the showCrates function should be modified to:

<?php
function showCrates(array $crateCreations)
{
 foreach($crateCreations as $key => $crateCreation)
 {
 $crate = $crateCreation->getCrate();
 if (!$crate)
 {
 echo 'Crate #' . $key . ' is invalid, because supplied dimensions were: ' . $crateCreation->getDimensions() . PHP_EOL;
 continue;
 }
 echo 'Crate #' . $key . ':' . $crate->getDimensions() . PHP_EOL;
 }
}

Answer 2

You are correct in asserting the Crate object should always be valid. This is a business class that should be enforcing its invariants — which it currently does. The problem lies with taking user input. You must allow user input to violate business rules and constraints. You will need at least 2 other classes:

1. A "view model" or "parameter" object that allows you to reconstruct the Crate objects without enforcing business rules.

2. A "validator" object that inspects the request data and collects error messages for each object in the collection, for each property being validated.

You must run these data validations against this collection of objects that might be violating business rules. When validations fail, return a collection of error messages back to the client.

Only upon successful validation should new Crate objects be initialized.

This just boils down to basic user input validation.

• Dennis: Thanks. For #1, are you talking about a FakeCrate object, similar to Crate, but where no internal validation is applied? And for #2 I imagine an object CrateValidator to validate FakeCrate objects, where CrateValidator has validation code extracted from the original Crate? I understand you will be relying on results of CrateValidator to know where the error is and not on results of Exceptions thrown by Crate. If I understand this correctly, I will be duplicating validation code from inside Crate object in the CrateValidator. Is there a way to avoid such code duplication?
• Dennis: to clarify, I expected PHP's native exception handling to do warn me when an object is invalid, and use that to my advantage without having a separate validator object.
• Greg Burghardt: @Dennis: For #1 I am talking about creating a Data Transfer Object - a big dumb bag of properties with no logic. For #2 if you need to reuse validation logic, try adding static methods to the Crate class. As a very related aside, you should be checking each dimension independently and throwing exceptions unique for each dimension.
• Dennis: Thanks. Is there a reason for throwing individual exceptions for each dimension? Also, in my quest to simplify my question, I have made some modifications. Even if I can validate user input, there are issues with pulling data from database using that input. My actual Crate object contains weights of individual products that go into the crate, each of which may not be defined, or configuration may not be valid overall, even if individual user input is correct. That is because not all information is available yet in the database, and I need to be able to handle lack of information in my code.
• Dennis: I understand external validation may be a way to go. I will be trying to figure out how to keep messages assigned to proper objects, when the validation is done externally and not within the objects themselves.

Answer 3

I don't often add a second answer, but I have another approach that might make more sense and require less code, and I still think my other answer has value as well.

---

In PHP you can return an array of values and split them out into multiple variables in the caller.

You could add a static method to the Crate class that accepts the arguments to the Crate constructor, and returns an array with a Boolean, a Crate object and an Exception object thrown during initialization.

class Crate
{
 public static function create(float $length, float $width, float $height) {
 $isValid = false;
 $crate = null;
 $error = null;
 try {
 $crate = new Crate($length, $width, $height);
 $isValid = true;
 } catch (Exception $err) {
 $error = $err;
 $crate = null;
 }
 return array($isValid, $crate, $error);
 }
}

And an example of using it:

list($isValid, $crate, $error) = Crate::create(10, 4, 8);
if ($isValid) {
 // Do something with $crate
}
else {
 // Log the $error
}

The advantage here is this creation logic is kept in the class that specializes in Crates: the Crate class, and doesn't bloat your code base with more classes.

Answer 4

So what you are asking is for it to be impossible to compile code which produces an invalid crate at run time.

This is not as impossible as it sounds, but you are going to have to use a strongly typed language. I'm not sure you can do it in PHP

for example, lets say for a second we allow a 0 size crate. you could change the construction parameters to unsigned integer. Now you can't pass in negative values.

You can imagine further than you can create more types that express your requirements and then use them rather than the underlying value types.

Obviously at some stage you will have to parse the input type to your special type and at that point you can either throw an exception, or define the parsing so that exceptions are impossible. 0 is read as 1 for example.

An alternative is to use null to mean an invalid crate. Now I can have a CrateFactory.CreateCrate(x,y,z) which returns null if the parameters are invalid and a show crates function which tests for null and outputs the required string.

I'm not sure thats much different from a bool IsValid flag though

• Dennis: well, no, not that.... I allow situations where you can compile code that produces an invalid Crate. I allow it explicitly too because I want to be notified at run time, as part of normal output, whether a crate is valid or invalid. The input for Crate classes is given by the user, or by an automated process. Sometimes the given information is correct and sometimes the information is missing or incorrect and there is no way to know when all information supplied to Crate will be correct. It would be impossible or at least very impractical to know that ahead of time at compile time.
• Ewan: well im confused. you want not to be able to create an invalid crate. but also to have invalid crates in functions?
• Dennis: 1st is correct. I do not want invalid Crates to be created. 2nd, I am looking for a way to display information about failed crate creation at a later time. i.e. first all of the crates are created (or failed being created). I perhaps somehow store that information. Then later, when I get to the view layer, I can display information for valid crates, and display information that crate i failed creation due to this or that reason. So #2 in your comment is not a requirement, as stated. I want to keep the Crate object as-is.. everything else can be modified as needed to fit my purpose
• Ewan: well my suggestion of using nulls will do that, although you might as well have a seperate list of failed crates or a wrapper with a bool or any one of a million ways

Answer 5

OP here. I saw my problem as a need to postpone exception handling messages. Namely, exception handling interrupts the flow of the program to deliver an exception message right at the point of failure. I needed to postpone this reporting until the time of the view layer takes effect.

In other words I needed a way to store the exception messages and information about a lack of a Crate in case a Crate failed to initialize.

I thus created a wrapper object of [Crate|NULL + Exception message|NULL] and called it CrateRecord

Not sure if it is a way to go but it seems to be working.

class CrateRecord
{
 /** @var Crate */
 protected $crate;
 /** @var string */
 protected $error;
 function __construct(Crate $crate = null, string $error)
 {
 $this->crate = $crate;
 $this->error = $error;
 }
 public function getCrate()
 {
 return $this->crate;
 }
 public function getError()
 {
 return $this->error;
 }
}
class CrateRequestHandler
{
 function handle()
 {
 // mocked up data from Request
 // this is sample data for showcase purposes only
 // it usually supplied by user or from database
 // and it is impossible to tell ahead of time
 // if it will be correct for Crate purposes
 $data = array(
 array(2, 5, 9),
 array(1, 3, 4),
 array(0, 0, 0), //invalid
 array(5, 6, 3)
 );
 $crateRecords = array();
 foreach($data as $key => $d)
 {
 try
 {
 $crate = new Crate($d[0], $d[1], $d[2]);
 $error = "";
 }
 catch (Exception $e)
 {
 $crate = null;
 $error = $e->getMessage();
 }
 $crateRecords[$key] = new CrateRecord($crate, $error);
 }
 // send to View
 $this->showCrates($crateRecords);
 }
 /**
 *
 * @param CrateRecord[] $crateRecords 
 */
 function showCrates(array $crateRecords)
 {
 foreach($crateRecords as $key => $crateRecord)
 if ($crateRecord->getCrate())
 echo 'Crate #' . $key . ': ' . $crateRecord->getCrate()->getDimensions() . PHP_EOL;
 else
 echo 'Crate #' . $key . ': ' . $crateRecord->getError() . PHP_EOL;
 }
}
// To call
$handler = new CrateRequestHandler();
$handler->handle();

• Greg Burghardt: The problem with this approach is exception messages are being passed back to the client. Exception messages are for debugging problems by developers. The messages you should be passing back to the client should be user interface friendly/user friendly validation messages. What is a user going to do with an "Invalid Dimension" message? This gives them no indication of how to fix the issue.
• Dennis: I may have a greater issue. My program is to generate a set of crates (with valid dimensions) for a series of products. Some products, even valid product combinations produce crates that are illegal (i.e. dimensions that do not fit in a truck). One purpose of the program is to provide friendly messages back to the user saying why this or that crate is a bad idea (invalid dimension, too large, etc). I was thinking of using Exceptions to provide those messages, or at least be a major part in reporting - that is, for example, I can reinterpret Exceptions to provide friendlier messages.
• Greg Burghardt: Exceptions are less efficient that simple checks and pushing messages onto an array. Throwing an exception unwinds the current stack. Comparatively exceptions are pretty costly. You actually have some complexity here. Not only are you validating the dimensions of a crate, but you are also validating the fact it can fit on a particular truck or trailer.

Answer 6

The problem here is that, despite your description of the problem and the name you have given your method, you aren't actually showing a Crate. You are displaying something else altogether. You are displaying a message indicating the result of attempting to create a Crate.

class CrateRequestHandler
{
 function handle(ServerRequestInterface $request)
 {
 $messages = [];
 // data is undefined
 foreach ($data as $key => $dimensions) {
 try {
 $crate = new Crate(...$dimensions);
 $messages[$key] = 'Crate #' . $key . ':' . $crate->getDimensions();
 } catch (Exception $ex) {
 $messages[$key] = 'Crate #' . $key . ':' . $ex->getMessage();
 }
 }
 $this->showMessages($messages);
 }
 function showMessages(array $messages) : void
 {
 foreach ($messages as $message) {
 echo $message . PHP_EOL;
 }
 }
}

I'm frankly astonished at the answers in this thread and sheer amount of over-engineering I am seeing when a simple try/catch along with a subtle shift in our understanding of the problem is all that is necessary.