search

fuel / orm

Fuel PHP Framework - Fuel v1.x ORM
http://fuelphp.com/docs/packages/orm/intro.html
152 stars 96 forks source link

Add a method to get the primary key's as an array #383

Closed xploSEoF closed 8 years ago

xploSEoF commented 9 years ago

This creates an array containing the relevant information for identification of the object. This could be used in many locations, specifically in my case, logging.

emlynwest commented 9 years ago

Functionality looks fine but please fix the coding style. Indent with tabs and {} on new lines please. Also possibly the function would be better named get_pk_values() or something akin to that to better describe what it does.

WanWizard commented 9 years ago

I'd say get_pk() would be enough, no need to add "array" or "values" to it? I'd only do that if you have or intend to have more methods starting with get_pk?

Also, please update the documenation with the new method.

emlynwest commented 9 years ago

Only issue is that there's already a "get pk" method that returns the property names.

WanWizard commented 9 years ago

Then "values" seems better than "array", agreed.

xploSEoF commented 9 years ago

Or assoc, since values implies it is literally just the values...

emlynwest commented 9 years ago

I'd be happy with either assoc or values, both work for me.

xploSEoF commented 9 years ago

Where would you like documentation to be added? I can't find any of the functions on \Orm\Model in any of the documentation, with the exception of the very basic ones on the CRUD page.

WanWizard commented 9 years ago

You got a point, it does miss a method reference, so nowhere to add it...

xploSEoF commented 8 years ago

Was this not merged then, or has GitHub not identified the merge?

WanWizard commented 8 years ago

No, it wasn't. And it was a PR against a now outdated branch, so it couldn't be merged anymore.

WanWizard commented 8 years ago

Also, I wonder if that code (in particular the value reference) still works in PHP 7, as it always loops over a copy now.

@stevewest can you comment?

xploSEoF commented 8 years ago

Given my understanding over the PHP7 changes, this should continue to work.

Foreach loops in PHP < 7 loops over a copy, but with references when necessary. PHP >= 7 loops through the array itself.

WanWizard commented 8 years ago

Ok, then creating a new PR for 1.9/develop should not be a problem.

Regarding the method name, @stevewest stated get_pk() already existed, but I can''t find it?