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

Propose Moving Model::classToArray() method to Entity class as public method #579

Closed
daylightstudio opened this issue Jul 1, 2017 · 4 comments

Comments

@daylightstudio
Copy link
Contributor

daylightstudio commented Jul 1, 2017

I've found it useful to be able for a model's entity class to return an array of the values being saved (e.g. for a toJson method on the Entity class). Currently the method that extracts that data is in the Model class but protected. I would like to suggest moving that method to the Entity class as public and have the Model class call it instead.

protected $exclude = [];

public function values()
{
	$mirror = new \ReflectionClass($this);
	$props  = $mirror->getProperties(\ReflectionProperty::IS_PUBLIC | \ReflectionProperty::IS_PROTECTED);

	$properties = [];

	$exclude = array_merge(['datamap', 'exclude'], $this->exclude);

	// Loop over each property,
	// saving the name/value in a new array we can return.
	foreach ($props as $prop)
	{
		if (in_array($prop->getName(), $exclude))
		{
			continue;
		}

		// Must make protected values accessible.
		$prop->setAccessible(true);
		$properties[$prop->getName()] = $prop->getValue($this);
	}

	return $properties;
}
@lonnieezell
Copy link
Member

I need to think on this one a little more. Here's my concerns:

  1. This wouldn't support anyone who didn't want to use the Entity class at the basis of a class-based value-object. Which may not be a concern, because we probably want to encourage people to use Entity in these instances. However, feels like we might be reducing user choice here.

  2. (and this is very much a personal thing) I'm not sure I really want to encourage taking the current class and jamming it back into a JSON object. My view is that you should explicitly decide what to return so that you don't screw things up when you have to version an API, or add new db-columns that are intended for internal usage and you don't want it thrown back in the API.

@daylightstudio
Copy link
Contributor Author

daylightstudio commented Jul 3, 2017

What about making classToArray a static public method on the Model class instead?

@lonnieezell
Copy link
Member

What about making classToArray a static public method?

That, I think is doable.

@lonnieezell
Copy link
Member

Fixed in 2d05e64

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

2 participants