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

Added auto-casting feature #67

Closed
wants to merge 3 commits into from
Closed

Added auto-casting feature #67

wants to merge 3 commits into from

Conversation

damianopetrungaro
Copy link

Specifying a 'mapTo' key in an entry of the array returned by getCols method in TableInterface, the value will be casted using the settype php function.

This is incredibly useful when using type hinting in an entity (an example here)

Example here from EmployeeTable.php in tests

@pmjones
Copy link
Contributor

pmjones commented Jun 24, 2017

It's very difficult to see the substantial changes when they are mixed in with the style changes. For example, Atlas attempts to follow PSR-2, so that means the brace placement should be left alone. If you could, for Atlas PRs, please attempt to follow the existing style.

Once the style changes back, the diff will be much smaller, and I will be able to examine the auto-casting portion of the PR.

@damianopetrungaro
Copy link
Author

All the commits follow PSR2 standard.
Anyway i understand that may be difficult to see the substantial changes.

The substantial changes are in those commits:
1 2 3

If you prefer i will go back and push only the 'useful' commits.
😄

Adding a 'mapTo' key in the getCols() method of a TableInterface implementor, it will be casted as the value of the mapTo key (if omitted string will be used as default).
…assertSame) when the result is not mapped by Atlas
@damianopetrungaro
Copy link
Author

Cleaned up!

@pmjones
Copy link
Contributor

pmjones commented Jun 24, 2017

All right, this is a lot easier to review, thanks!

I can see now that there is a significant problem. The "Table" classes are auto-generated by the CLI tooling; if you change a table at the database and use the CLI tooling to regenerate the class (which is normal and expected) then all the mapTo keys will be lost.

If you want casting of values coming out of the database, and casting of them when they go back into the database, you will probably want to add some logic to a custom TableEvents class. In particular, you'll want to work with modifySelectedRow() (to cast the values coming out of the database), beforeInsert() or modifyInsert(), and beforeUpdate() or modifyUpdate() (to cast the values going back to the database).

It occurs to me that there's an opportunity here to build the auto-caster as a plugin or class of its own, that can be either injected into or used statically within the TableEvents class.

Does that begin to help?

@damianopetrungaro
Copy link
Author

damianopetrungaro commented Jun 24, 2017

The Table Event is perfect for solve this kinda problem imo, i missed it.

Thanks for clarifying!

@pmjones
Copy link
Contributor

pmjones commented Jun 24, 2017

i missed it.

Easy to miss since I have failed to document it (along with so much else). Thanks for the PR, and for your patience!

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

Successfully merging this pull request may close these issues.

None yet

2 participants