Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use magic "behind the curtains" #104

Open
kemo opened this issue Jun 17, 2014 · 5 comments
Open

Don't use magic "behind the curtains" #104

kemo opened this issue Jun 17, 2014 · 5 comments

Comments

@kemo
Copy link
Member

kemo commented Jun 17, 2014

ORM shouldn't rely on magic methods natively but use the methods for setting / getting instead. Examples;
$this->{$name} = $var;
$this->{$key}->values($values[$key], $column);
$this->$column = $values[$column];
$data[$var] = $this->{$var};
$this->{$property} = $value;

This obviously doesn't apply to properties which aren't magic anyway

@kemo
Copy link
Member Author

kemo commented Jun 22, 2014

Any opinions?

@evanpurkhiser
Copy link

I'm a little bit confused by your example. Are you saying end-users should use ORM::get and ORM::set and we remove the magic methods that allow you to act on columns as if they were properties of the object?

// So instead of this
$my_model->some_column = 'value';

// It would be
$my_model->set('some_column', 'value');

Or are you saying internally we should never use magic methods? (In that case I think I would agree)

@kemo
Copy link
Member Author

kemo commented Jun 24, 2014

Or are you saying internally we should never use magic methods?

@evanpurkhiser exactly, sorry if I wasn't clear enough

@evanpurkhiser
Copy link

This sounds good to me then. I can't think of any reasons not to use the get/set methods. 👍

@arteymix
Copy link

Yeah, even magic methods are mapped to get and set, so this would lighten the call stack. It would also be more flexible when dealing with uncommon column name containing spaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants