Model Virtual Fields #89

Merged
merged 6 commits into from Mar 23, 2013

Conversation

Projects
None yet
2 participants
@nathggns
Member

nathggns commented Mar 23, 2013

You should be able to apply virtual fields to models. These can either be closures, or scalar values, or a combination.

Will look something like this.

<?php
$model->virtual('scalar', 'blah');
$model->virtual('closure', function($field) {
    // $model === $this

    return 'blah';
});
$model->virtual('combination', [
    'scalar' => 'blah',
    'closure' => function($field) {
        return 'blah';
    }
]);

@ghost ghost assigned nathggns Mar 23, 2013

@nathggns

This comment has been minimized.

Show comment Hide comment
@nathggns

nathggns Mar 23, 2013

Member

These can be created from outside the model, or from the init function.

Member

nathggns commented Mar 23, 2013

These can be created from outside the model, or from the init function.

@ClaudioAlbertin

This comment has been minimized.

Show comment Hide comment
@ClaudioAlbertin

ClaudioAlbertin Mar 23, 2013

Contributor

Why is there a need to declare it as a scalar or a closure? Just execute the function (that doesn't need the field as an argument, btw.) if it is one, or return the value if it isn't.
How would combinations work?

Contributor

ClaudioAlbertin commented Mar 23, 2013

Why is there a need to declare it as a scalar or a closure? Just execute the function (that doesn't need the field as an argument, btw.) if it is one, or return the value if it isn't.
How would combinations work?

@nathggns

This comment has been minimized.

Show comment Hide comment
@nathggns

nathggns Mar 23, 2013

Member

Oh no, the first argument is the field name, I was just using scalar and closure as examples. Well, if the scalar value is iterable, then you loop through it and look for closures to execute.

Member

nathggns commented Mar 23, 2013

Oh no, the first argument is the field name, I was just using scalar and closure as examples. Well, if the scalar value is iterable, then you loop through it and look for closures to execute.

@ClaudioAlbertin

This comment has been minimized.

Show comment Hide comment
@ClaudioAlbertin

ClaudioAlbertin Mar 23, 2013

Contributor

In my opinion, it should simply work like this:

$model->first_name = 'Nathaniel';
$model->last_name = 'Higgins';

$model->virtual('full_name', function() {
  return $this->first_name + ' ' + $this->last_name;
});

$model->virtual('login_token', 'w89etuvoeiurwoqn3485npv34q95tgjdu');

echo $model->full_name; // Nathaniel Higgins
echo $model->login_token; // w89etuvoeiurwoqn3485npv34q95tgjdu
Contributor

ClaudioAlbertin commented Mar 23, 2013

In my opinion, it should simply work like this:

$model->first_name = 'Nathaniel';
$model->last_name = 'Higgins';

$model->virtual('full_name', function() {
  return $this->first_name + ' ' + $this->last_name;
});

$model->virtual('login_token', 'w89etuvoeiurwoqn3485npv34q95tgjdu');

echo $model->full_name; // Nathaniel Higgins
echo $model->login_token; // w89etuvoeiurwoqn3485npv34q95tgjdu
@nathggns

This comment has been minimized.

Show comment Hide comment
@nathggns

nathggns Mar 23, 2013

Member

That's exactly what it does.

Member

nathggns commented Mar 23, 2013

That's exactly what it does.

@ClaudioAlbertin

This comment has been minimized.

Show comment Hide comment
@ClaudioAlbertin

ClaudioAlbertin Mar 23, 2013

Contributor

No. Why does a combination even need to exist? And why does the closure have to get the field name as an argument? What field name even?

Contributor

ClaudioAlbertin commented Mar 23, 2013

No. Why does a combination even need to exist? And why does the closure have to get the field name as an argument? What field name even?

@nathggns

This comment has been minimized.

Show comment Hide comment
@nathggns

nathggns Mar 23, 2013

Member

I think you're massively misunderstanding what I'm saying.

The first argument is completely irrelevant, it's just what it's assigned to on the model. If its a string or int or whatever, it'll just assign that right away. If it's a closure, it will call it (at __get time, not when you call virtual) and use that value. If it's an array, it will loop through it, calling closures and reassiging the values, and then use that.

Member

nathggns commented Mar 23, 2013

I think you're massively misunderstanding what I'm saying.

The first argument is completely irrelevant, it's just what it's assigned to on the model. If its a string or int or whatever, it'll just assign that right away. If it's a closure, it will call it (at __get time, not when you call virtual) and use that value. If it's an array, it will loop through it, calling closures and reassiging the values, and then use that.

@ClaudioAlbertin

This comment has been minimized.

Show comment Hide comment
@ClaudioAlbertin

ClaudioAlbertin Mar 23, 2013

Contributor

I understand what you're saying, but what you're suggesting doesn't make sense in multiple points.
What is $field supposed to be? You're not supposed to override a field that exists in the database, as writing to that field wouldn't work anymore, thus it can't be the previous value of the field.
This way, using a combination doesn't make sense. You could only have one scalar in the combination anyway, and its value would have to be passed from closure to closure. This behaviour isn't of much advantage, and could be easily achieved without a combination.

Contributor

ClaudioAlbertin commented Mar 23, 2013

I understand what you're saying, but what you're suggesting doesn't make sense in multiple points.
What is $field supposed to be? You're not supposed to override a field that exists in the database, as writing to that field wouldn't work anymore, thus it can't be the previous value of the field.
This way, using a combination doesn't make sense. You could only have one scalar in the combination anyway, and its value would have to be passed from closure to closure. This behaviour isn't of much advantage, and could be easily achieved without a combination.

@nathggns

This comment has been minimized.

Show comment Hide comment
@nathggns

nathggns Mar 23, 2013

Member

No, you're still misunderstanding. Look at the failing tests I commited - 8a567da

Member

nathggns commented Mar 23, 2013

No, you're still misunderstanding. Look at the failing tests I commited - 8a567da

@nathggns

This comment has been minimized.

Show comment Hide comment
@nathggns

nathggns Mar 23, 2013

Member

I think this is ready for merge now.

Member

nathggns commented Mar 23, 2013

I think this is ready for merge now.

ClaudioAlbertin added a commit that referenced this pull request Mar 23, 2013

@ClaudioAlbertin ClaudioAlbertin merged commit 8ea4c23 into development Mar 23, 2013

1 check passed

default The Travis build passed
Details

@nathggns nathggns deleted the issue-89 branch Apr 4, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment