I like it. Very much.

I like it, because as always Taylor, I didn't need to read a single word you wrote. The code sells itself.

Assuming this behaves as expected when casting the object to JSON, I think it's great!

@pstephan1187 you can implement Arrayable on your value objects and they will be cast to JSON correctly.

For a moment I was like hmm, this seems ok... Then it clicked. THIS IS AMAZING.

Does everything get casted to a string? or would it be possible to cast to say, integers, booleans, etc?

Edit: I guess it'd be part of the fromModelAttributes method?

Wouw awesome! (y)

Alex, things are cast to whatever you want. But I'm not sure I understand the question.

On Wed, May 25, 2016 at 3:13 PM -0700, "Alex Bowers" notifications@github.com wrote:

Does everything get casted to a string? or would it be possible to cast to say, integers, booleans, etc?

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

It is my understanding, that since for example booleans aren't a thing in mysql, but instead it is an integer, there is currently the value of either 0 or 1 coming back. Whereas, it'd be cleaner to see true or false as their true values.

One thing you can currently do in Eloquent is use

$casts = [
    'billing' => 'boolean',
];

If you want, for example to have an address and one of the fields is billing (boolean), how would you ensure it is a boolean, whilst also casting into an 'address' with $address->billing returning as a boolean, not an integer.

You state that

Value objects MUST implement two methods

But your code example doesn't show an implements contract. Will one be enforced for this?

I don't see a large need for interface because your code will error anyways if you don't define the methods. So adding an interface is just changing the error message, not really doing anything useful and it's just another file to load off of disk. 

On Wed, May 25, 2016 at 3:21 PM -0700, "Alex Bowers" notifications@github.com wrote:

You state that

Value objects MUST implement two methods

But your code example doesn't show an implements contract. Will one be enforced for this?

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

Will one be available to use, for those of us that like to use them to ensure that methods are setup properly, and to make the code headers read well?

I really like this and would be happy as is, but I agree with @alexbowers here that I would really like an interface.

I know it might not be very zonda, but I feel like since we have the tools to make the contract explicit, might as well.

Plus it opens up interoperability via illuminate/contracts so I don't necessarily need to depend on all of Laravel to provide decent value objects, but I can show that they're compatible.

The approach taken by Mongolid, an ODM for MongoDB, designed by @Zizaco works "out of the box" (without the need for a new interface, using fiil and toArray) as this:

class User extends Eloquent
{
    public function address()
    {
        return $this->embedsOne(Address::class, 'address'); // class name, field name
    }
}

The attribute 'address' holds the object serialized to an array, eg lineOne and lineTwo, so you can invoke the field directly and retrieve the array.
The magic is that calling $user->address() will do something like it:

$address = new Address;
$address->fill($user->address);
return $address;

Also, calling $user->embed('address', $address); will cast that $address to array and save to the attribute.

There's a lot more logic involved but the main concept is this.

It might be a way to go about it.

For reference: http://leroy-merlin-br.github.io/mongolid/relationships/

An interface adds no value though. It just changes the error message. Does anyone have a use case for the interface other than changing the error message. I don't really want to get distracted by something so minor so maybe lets focus on actual implementation.

On Wed, May 25, 2016 at 3:41 PM -0700, "Troy Pavlek" notifications@github.com wrote:

I really like this and would be happy as is, but I agree with @alexbowers here that I would really like an interface.

I know it might not be very zonda, but I feel like since we have the tools to make the contract explicit, might as well.

Plus it opens up interoperability via illuminate/contracts so I don't necessarily need to depend on all of Laravel to provide decent value objects, but I can show that they're compatible.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

Although I love the idea of having a embeddables feature, like Doctrine 2 has, I don't like this implementation. It's too intrusive, but I guess this is due to Eloquent's fundamental of being kinda magic-ish. I don't like having this behaviour, as actual code, in my value objects (VOs). There's a difference between the configuration and not intrusive way that Doctrine 2 uses and creating actual public methods on a value object that can be called from anywhere. Since it's just an array that's passed in, you'd have to additional validation just to check if the array has all the correct values. Unlike when you use a normal or named constructor (factory method) in which you get implicit validation and you create a stronger boundary.

With the current implementation there's no way to use one of the constructors on the VO that might have the needed validation to keep it consistent and valid.

In my opinion this type of feature doesn't fit on Eloquent. The idea behind this concept of a VO, which is literally just a good ol' object, is to package certain behaviour up in neat little chunk of code that will always have the same, it'll always be deterministic and thus immutable, but because of Eloquent's inherent openness it doesn't seem like a good fit.

A better (seeming) solution could simply be to make a method on the model that instantiates the VO.
Literally something like:

public function address() {
    return new Address($this->lineOne, $this->lineTwo);
}

Edit 1:
A value object is only for reading.

I have to agree, having something similar to the current way that relationships works would be pretty neat.

To me, casting to an object seems odd (i think). it feels more like what the relationships setup, in terms of the properties created and such.

@alexbowers except for that this doesn't need any framework support. A value object is simply just, an object. It shouldn't need outside support. If it does, if it's not boxed off (or so to speak), then there's something wrong.

Taking the address example, this could be solved by creating getAddressAttribute() and setAddressAttribute() mutators on the user model.

It appears then this is attempting to refactor that multi-value getter and setter out to its own VO to simplify the model? (Is that the intended problem this PR is trying to solve?)

I like it, as I have done the mutator methods many times in the past and it always felt convoluded and dirty.

Update: thinking on this some more, really the only place this makes sense is in one of two situations:

• you have objects serialized in JSON fields in your database.
• your database tables aren't normalized.

I'm thinking that this concept is best used only after thinking long and hard if it is the right solution.

@mikebronner could you give me a concrete example of what these methods could look like? To me, it all just seems like this can be handled by a simple method on the model itself, just code, not anything the framework has to do.

Eloquent doesn't need to able to change the properties on the VO, because they're immutable. If they're mutable, there's no need to use a value object.

@mitchellvanw using the address example above:

namespace App;

use Illuminate\Foundation\Auth\User as Authenticatable;

class User extends Authenticatable
{
    protected $appends = [
        'address',
    ];
    /**
     * The attributes that are mass assignable.
     *
     * @var array
     */
    protected $fillable = [
        'name', 'email', 'password',
    ];

    /**
     * The attributes that should be hidden for arrays.
     *
     * @var array
     */
    protected $hidden = [
        'password', 'remember_token',
    ];

    public function getAddressAttribute()
    {
        return new Address([
            'street' => $this->streetAddress,
            'city' => $this->city,
            'state' => $this->state,
        ]);
    }

    public function setAddressAttribute(Address $address)
    {
        $this->street = $address->streetAddress;
        $this->city = $address->city;
        $this->state = $address->state;
    }
}

And the Address class would be a simple VO, or perhaps a non-eloquent model using jenssegers/model.

I would love to see this in Laravel. I must say I like the Doctrine embeddables a bit better. Here you have to define both the constructor and two other methods to make it work. Also: When you add up city/zip for your address there are suddenly four fields that need to be handled and it will get messy with more method parameters and more boilerplate code to be written. The address example is what Symfony uses as well, so it's tempting to compare…

Also: PHPMD will squawk at the unused $model parameter in fromModelAttributes() ;)

All in all though, I would love to see this feature in Laravel.

There an other ways to inject the constructor properties. For example we could allow defining which attributes should be injected in the cast definition.
Address:line_one,line_two
etc...

On Wed, May 25, 2016 at 5:02 PM -0700, "Torkil Johnsen" notifications@github.com wrote:

I would love to see this in Laravel. I must say I like the Doctrine embeddables a bit better. Here you have to define both the constructor and two other methods to make it work. Also: When you add up city/zip for your address there are suddenly four fields that need to be handled and it will get messy with more method parameters and more boilerplate code to be written. The address example is what Symfony uses as well, so it's tempting to compare…

Also: PHPMD will squawk at the unused $model parameter in fromModelAttributes() ;)

All in all though, I would love to see this feature in Laravel.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

Eh nevermind that won't be that great.

On Wed, May 25, 2016 at 5:14 PM -0700, "Taylor Otwell" taylorotwell@gmail.com wrote:

There an other ways to inject the constructor properties. For example we could allow defining which attributes should be injected in the cast definition.
Address:line_one,line_two
etc...

On Wed, May 25, 2016 at 5:02 PM -0700, "Torkil Johnsen" notifications@github.com wrote:

I would love to see this in Laravel. I must say I like the Doctrine embeddables a bit better. Here you have to define both the constructor and two other methods to make it work. Also: When you add up city/zip for your address there are suddenly four fields that need to be handled and it will get messy with more method parameters and more boilerplate code to be written. The address example is what Symfony uses as well, so it's tempting to compare…

Also: PHPMD will squawk at the unused $model parameter in fromModelAttributes() ;)

All in all though, I would love to see this feature in Laravel.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@mitchellvanw I'm not sure if your argument that you won't have your constructor validation is valid.

You said this:

With the current implementation there's no way to use one of the constructors on the VO that might have the needed validation to keep it consistent and valid.

In PHP 7 you could just do this:

return new static(
     $attributes['line_one'] ?? null,
     $attributes['line_two'] ?? null,
);

You don't really need to do any additional validation if the attributes exist in the mapping method because you can just send null into the constructor if they don't and let your constructor barf an error if it doesn't like null.

So, I'm not sure it's correct to say you need to check if the attributes exist. All you're doing in this method is creating a new instance of the object. No validation is needed. You can still let your constructor do all that.

@taylorotwell I think regardless, we are talking about value objects here and there are advantages to them being immutable.

@taylorotwell Sometimes it does not worth the trouble of having a database-design in the first normal form. When the decision made, right or wrong, I am going to have composite values in some columns, e.g. using json. Each json value in a column has its own schema, and to detect what is the schema, one solution is to have another column, namely type. It would be very nice to be able to convert those json objects to the appropriate php objects based on the type column:

Example

Suppose you need to make a survey-monkey-like application. There are different types of questions in a single questionnaire, but in your app, the idea of having one or multiple tables per each question type is an overkill. So here is what the questions table looks like:

And in the PHP side, I would like to not have any switch-case at all when I need to, for example validate an answer against the question schema:

abstract class QuestionSchema
{
    public abstract function validationRules();
    public function validate($answer)
    {
        //apply the validation rules against the answer
    }
}

class ShortAnswerQuestionSchema
{
    public function validationRules()
    {
        return ["answer" => "string|max:64"];
    }
}
class ColumnMatchingQuestionSchema
{
     protected colum1;
     protected column2;
    public function validationRules()
    {
        return [
                       "answer" => "array|size:"count($this->column1),
                       "answer.*" => "in:".toCSV($this->column2);
       ];
    }
}

So when I have an $answer to be validated against question 10, I will do this:

Question::where('id', 10)->schema->validate($answer);

No switch-case at all 😄
Lets do that please.

Definitely any class that implements these two methods should be implementing an EloquentCastingInterface @taylorotwell

All discussion of an interface can basically stop because it’s not going to happen, heh. There is no point to making an interface other to calm people’s superstitions that it somehow makes the code better. It doesn’t. Period. So, you can discuss other things here but not that :)

@taylorotwell You wouldn't know this as you don't use an IDE, but using an Interface helps massively when creating new code, as the stub methods are added automatically.

@joshbrw I can understand why that seems like a reasonable requirement, but coding to satisfy the functionality of an IDE is fundamentally wrong. What do you do with all your specialized code if you switch IDEs or your IDE changes behavior. That's a no-win scenario for me.

Going to table this for now as it's apparently fairly divisive and I'm not sure I want to take on the complexity in Eloquent for something that isn't more unanimous.

@taylorotwell I think it's sad that this was closed… For a proper example though, try using Google's Address Data Service format when making a value object. It has roughly these fields:

• Country
• Administrative area
• Locality (City)
• Dependent Locality
• Postal code
• Sorting code
• Address line 1
• Address line 2
• Organization
• Recipient

I might have said earlier that it get's messy with a lot of getters, setters and a huge constructor, but I'm going back on that as I see it as more important to get this feature into the codebase.

Check out what the value object looks like in commerceguys/addressing, in their Address class, which can basically be used as an embeddable in Doctrine.

Ross Tuck also has a nice article on the embeddables feature in Doctrine, which he calls "the promised land".

I think that says it all. Please reconsider! :)