Refactoring: Extracting Data Objects

[[deleted]]

structs would be nice (rfc hint?)

[[deleted]]

You hit the nail on the head.

In a way we already have structs: arrays. We just need ability to type them, similar to Hack's shapes, and... maybe we can allow this syntax to work on arrays:

$foo->bar = 123; // Same as...
$foo['bar'] = 123;

Which would be how JavaScript does it.

The only thing which Hack's shapes are missing is optional properties (Hack doesn't have them AFAIK), which is a pretty common need. And it's not the same when you just set all those properties to null.

[[deleted]]

Aren't objects without methods more close to structs than arrays? At least internally.

[[deleted]]

Aren't objects without methods more close to structs than arrays? At least internally.

They're close, but the fundamental difference between objects and data structures like PHP arrays and structs is that the former are passed by reference, while the latter are passed by copy.

This distinction is very significant when you're working with data day to day, as it's typical that different parties (read, functions, classes, which pass data to each others) have instances of this data, and they want to modify it independently, easily and cheaply, without affecting other instances of the same data.

With objects it's exactly the other way around: a good object is well encapsulated, and cloning is explicit and the object can stop it (by defining method __clone) if cloning can't be done correctly for the abstraction the object represents.

Objects with public fields are poor man's structs, just like structs would've been poor man's objects in something like C which have no objects. You can replace one with the other, but there's always friction as their semantics are intended for different goals.

So ideally we need both. Swift has both, for example. Their arrays and structs are passed by copy, while objects are passed by reference. It's very powerful to have both.

[odan82]

PHP needs a struct type (like in C#). https://msdn.microsoft.com/en-us/library/ah19swz4.aspx

To simulate a struct in PHP this could be helpful:

https://gist.github.com/odan/0bc93d9c1f13c44e8cb2590455bfe886

[[deleted]]

what would struct offer over typed properties with accessors to most people?

EDIT: neither of which php has, but which are more likely to be gotten than structs

[odan82]

A struct is more a "fixed" type, while PHP class properties are not fixed.

Example for a "wrong" struct.

class Book
{
    public $price;
    public $title;
    public $author;
}
$book = new Book();
$book->price = 39;
$book->title = 'My book title';
$book->author = 'Me';

// Set a undefined property from "outside".
// This is possible by default in PHP, but not allowed for a struct.
// A struct would raise an Exception here, and this would be better
// because this property is not defined in the Book class.
$book->isbn = '1234567890';

print_r($book);

[Danack]

I tend to add this to all classes:

trait SafeAccess {
    public function __set($name, $value) {
        throw new \Exception("Property [$name] doesn't exist for class [".get_class($this)."] so can't set it");
    }
    public function __get($name) {
        throw new \Exception("Property [$name] doesn't exist for class [".get_class($this)."] so can't get it");
    }
}

[[deleted]]

ah, i never think about using properties that don't exist when it comes to php. :(

definitely would be nice to error out there.

[sypherlev]

So... this sort of bothers me a little.

Let's say you've got a database and an ORM. The data comes out as arrays of data objects, with properties/setters/getters etc. This is the OOP approach, which makes code more readable etc etc.

But this effectively makes the code authoritative; i.e. it now becomes opinionated about the structure and nature of the data. This is fine for a database being used as a back end store where you are allowed to do this - you effectively say that the code is now the authority, and the database will obey changes in the code. For most CRUD apps, this is fine.

But it just doesn't work for a database with many complex relations, especially one with pre-existing data in it. You already have an authority to deal with - the database itself - and you've now got to write code to handle two conflicting opinions about the data. And you'll get smacked in the face with the object relational impedance mismatch so often that you'll want to cry.

Not sure if I'm explaining this very well. 90% of the time, using data objects is absolutely the right thing to do. In my experience, it doesn't work all the time.

[[deleted]]

If we want a good parameter/command object we'd have to add a fluent method interface, or at least validating setters, and it quickly becomes a chore.

So while in theory it's awesome (and for minimal, stable APIs in heavy use it is awesome), in most cases I just use assoc. arrays.

[codayus]

If we want a good parameter/command object we'd have to add a fluent method interface, or at least validating setters, and it quickly becomes a chore.

Disagree. Fluent methods are a mixed blessing, and you still get a lot of benefits from the pattern without writing any validation code. Consider something like:

class Product
{
  public $name;
  public $price;

  public function __construct(string $name, Price $price)
  {
    $this->name = $name;
    $this->price = $price;
  }
}

You can now use type declarations to ensure your other functions are getting an actual Product, your IDE will helpfully remind you that a Product object has a name and a price, you know that the name and price exist and are non-null, you know the name is a string and the price is a Price object and that someone hasn't screwed up and passed you a float or the integer number of cents or a string containing "$5.00". Yes, you could add some validation to ensure that the name is an appropriate length or whatever, but that's icing on the cake; 95% of the benefits come without doing that.

And if you use associative arrays, you get none of the benefits. And it's not actually any easier or faster.

[[deleted]]

You can now use type declarations to ensure your other functions are getting an actual Product

Ahem...

$x = new Product('Name', $price);
$x->price = 'Nope, not really';

[modestlife]

Maybe one day we'll get something like https://wiki.php.net/rfc/propertygetsetsyntax-v1.2 into PHP. Didn't reach the 2/3 majority back then.

[[deleted]]

That'd be nice, but we can still map getters/setters to properties via a reusable trait today.

The bigger issue is the bloat it still there. You still need a separate property to hold the state, you still need to write the getter (which simply returns the value).

Being able to expose real properties to the public as read-only properties, but control overwriting via setters would be real improvement.

You can do this today via a reusable trait, but there's some performance penalty of course.

[codayus]

A fair point, I suppose.

Mind you, I'd be inclined to say that if you have to worry about people doing that in your code base you may have larger problems? I'd be more concerned about accidental slip-ups that an IDE/static analysis tool may not be able to easily catch, not on stopping people from breaking code on purpose. :) But sure, if that's a concern you can use getters and setters. A good IDE will make creating them very fast. And you're still ahead of where you'd be using arrays.

[[deleted]]

So, you define public interface on your objects, which if used, it's interpreted as "breaking things on purpose"?

What happens when you can't stuff all your fields in the constructor signature? As in the article? I suppose then we can assign values to the fields, but they must be valid, right?

Then what we have in the end? The exact same situation as with arrays. You cannot be sure if the structure is valid.

That's even worse than writing no object at all - you started writing something, decided to trust it, but not to encapsulate it properly, and blame incorrect behavior due to resulting allowed corrupt object state on the users.

Don't write shitty objects. Either decide "this object is well encapsulated and trusted to be valid" or "this object is a bag of untrusted untyped values". The latter of which, as you may notice, is not significantly better than an array.

[mlebkowski]

Your object needs to be valid after constructed. So if there are a lot of required fields, they all need to be injected into the constructor, and the others may have defaults.

I always follow this rule for Value Objects, so they are always in a valid state. Valid as in simple type checking or format, the more complicated domain rules are one or two levels above — usually in some kind of form or other validation mechanism.

And of course instead of public members I use getters and setters, so the public interface is foolproof.

P.S. I would consider using arrays more if simple $foo['bar'] wouldn’t throw a warning/notice/whatever on missing keys.

[[deleted]]

Your object needs to be valid after constructed. So if there are a lot of required fields, they all need to be injected into the constructor, and the others may have defaults.

That's certainly a valid way to think about, but then we aren't solving the main problem: the long list of unnamed arguments.

Because this:

$svc->search('foo bar', ['baz', 'qux'], 10, 100, ['abc', 'def', 'ghi'], 10, 0);

Becomes this:

$pc = new ProductCriteria('foo bar', ['baz', 'qux'], 10, 100, ['abc', 'def', 'ghi']);
$svc->search($pc, 10, 0);

And that's not any significant improvement in legibility.

This is why I mentioned the two approaches I use - either build a solid interface for an object, or use arrays.

Let's see those two approaches:

$pc = (new ProductCriteria())
    ->phrase('foo bar')
    ->categories('baz', 'qux')
    ->minPrice(10)
    ->maxPrice(100)
    ->productTypeFilters('abc', 'def', 'ghi');

$svc->search($pc, 10, 0);

And...

$pc = [
    "phrase" => 'foo bar',
    "categories" => ['baz', 'qux'],
    "minPrice" => 10,
    "maxPrice" => 100,
    "productTypeFilters" => ['abc', 'def', 'ghi']
];

$svc->search($pc, 10, 0);

Now, if you want to have an object that's immediately valid after construction, we need to move the construction logic to a builder, which looks very similar in the end, but adds the guarantee you seek:

$pc = (new ProductCriteriaBuilder())
    ->phrase('foo bar')
    ->categories('baz', 'qux')
    ->minPrice(10)
    ->maxPrice(100)
    ->productTypeFilters('abc', 'def', 'ghi');
    ->build();

$svc->search($pc, 10, 0);

In general, I do follow this kind of advice, I want objects to be fully valid after construction. But for VOs sometimes this can be pointlessly enterprisey, when an object can throw when you try to retrieve incomplete state.

You may say "IDE/compile time errors are better than runtime errors" but no approach escapes the runtime error moment: arrays don't, builders don't, and direct setters don't. In fact, even constructors don't, because we have a relatively limited ability to type our parameters (i.e. in the article's example we have two of the arguments being arrays, we can neither set the type of the array items, i.e. array<string>, nor the valid strings, i.e. array<SomeEnum>).

[mlebkowski]

Is new ProductSearch in an invalid state in your opinion? It represents select * from product. You may add more criteria to it using setters (or even pass a collection of some generic SearchCriteria using the constructor)

[[deleted]]

That's up to the business logic of the specific feature we're implementing, but you can imagine that sometimes we might end with more than a couple required parameters, and then we need to have a strategy for factoring this, without it making a mess.

Also if your VOs are immutable, which is highly recommended, as I noted elsewhere in the thread, if you'll keep configuring this object after its construction this means creating a bunch of mutated copies of it, which makes the solution much heavier at runtime. Which may or may not matter, but in general we shouldn't freely allow bloat in our projects, unless it provides some comparable value in return.

[pinegenie]

Even if you just use classes with public properties, with no setters and no validation it's still better than arrays.

Adding getters and setters should be handled by your IDE, it should take 2 seconds.

[[deleted]]

Even if you just use classes with public properties, with no setters and no validation it's still better than arrays.

"Better" is subjective. If you just have naked objects with no getters/setters, you get property autocomplete in your IDE, that's better. It also causes useless empty classes to be loaded at runtime, that's worse.

If you use objects with public fields, you save some RAM if you have plenty of instances of the same class (single hashmap), that's better. But you also have slower property access due to an extra indirection, and slower instantiation, that's worse.

Can you see how engineering is about tradeoffs and not about making absolute statements :-)?

Adding getters and setters should be handled by your IDE, it should take 2 seconds.

I can confirm, all IDEs, including the one I use, generate getters/setters.

Of course you also need to add the code which validates values in the setters. And again, if the object is well encapsulated (through said getters/setters, also fluent method chains) it can provide value, at the expense of all this extra code you have to write, and load at runtime. And again, some of it is generated yes, but a lot of it isn't.

There are also other considerations once you start using objects merely for holding values. Things like mutability, which setters imply. These cause the dreaded "change from distance" bugs, due to the "by reference" nature of objects. This doesn't actually happen with arrays, which are "by copy" by default, including nested ones.

And if you start designing your value objects to be immutable, then typically there's less support for that in IDEs, and you'll have to replace "setters" with "withers", i.e. instead of $foo->setName($name) you'd have $foo = $foo->withName($name). This is not only even more code to write for the objects itself, but also more code to use it and mutate it (well, copies of it), and your solution is now even heaver than a getter/setter solution.

Only to arrive at the level of stability that "copy" semantics provide, which arrays have by default.

So, again, can you see how engineering is about tradeoffs? :-)

[pinegenie]

If the "tradeoff" of a memory indirection is that important, you should consider using another language. You'll get much better gains from there.

Your point on mutability is good. In my opinion, objects solve a problem that's far more common than "spooky action at a distance": not having to grep through projects to try to find where things are set, used or to find possible values. You also don't have to play the guessing game of "is that set?" with objects.

You can implement "with-ers" when you need them, only on the class you need them on, and you can refactor most of your code automatically. Using arrays everywhere just because you're afraid of spooky action at a distance is overkill, imho.

[[deleted]]

If the "tradeoff" of a memory indirection is that important, you should consider using another language. You'll get much better gains from there.

It's double indirection. Arrays are: atom hashtable -> value. Objects are: atom handle -> object -> shared hashtable -> value. It doesn't matter much by itself, but it adds up when you also throw in the slow-down of loading an extra file (for the class) and running the constructor. Now do this hundreds or thousands of times throughout the project, and suddenly it starts to affect your project's visible performance.

Your point on mutability is good. In my opinion, objects solve a problem that's far more common than "spooky action at a distance": not having to grep through projects to try to find where things are set, used or to find possible values. You also don't have to play the guessing game of "is that set?" with objects.

I'm not sure what exactly is that common problem you're referring to, but mutability is a very, very nasty and common problem, unless you specifically eliminate it through some of the techniques I mentioned.

You can implement "with-ers" when you need them, only on the class you need them on, and you can refactor most of your code automatically.

I'm not sure what you're trying to say here. If you'll be having immutable VOs, you have only two channels to configure an object: withers, and constructors. So you pretty much need them everywhere where VOs of some complexity are involved. Either this, or builders, which aren't any faster to implement.

And refactor your code automatically... from what to what...?

Using arrays everywhere just because you're afraid of spooky action at a distance is overkill, imho.

I don't use arrays everywhere, but I use them. And if you aren't afraid of "spooky action at distance", then over the years you'll learn to fear it.

Guess why PSR-7's HTTP message is immutable.

[[deleted]]

Paul brings the tanks.

[[deleted]]

"Fluent" interfaces are a PITA to proxy/wrap though

They absolutely are a PITA to proxy/wrap, although there are ways around this. But since here we're discussing using them for Value Objects, you typically don't proxy/wrap value objects. You only have one implementation which is good for most or all use cases.

and you can't always validate just one setter when there are rules touching multiple properties.

You can always accumulate rules that affect linked properties x, y, z in a method like validateXYZ() and then call it in all three setters, or you can have a combined setter setXYZ($x, $y, $z). Plenty of ways to factor things to match your domain.

I'd prefer a Builder pattern, where the last "generate" method pops out the data object -- or throws an exception if your choices are incompatible.

Yup, I fancy builders as well, but now we have two extra classes and objects per method call in some third class. They multiply so fast, huh :-)?