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

Entity exception for non existed props. #1927

Merged
merged 5 commits into from Apr 11, 2019

Conversation

nowackipawel
Copy link
Contributor

As for now if code calls non existed property of entity causing ErrorException with no info at all.
This PR is changing ErrorException to EntityException with translated information like this:
"Trying to access non existent property fle_amount_fromss of App\Entities\Fee\FeeLevel"

@lonnieezell
Copy link
Member

Honestly, I'm tempted to return null in these occasions. Because it is possible to set values that are not defined properties I kind of think that you should just be able to check for null instead of wrapping something in a try/catch block every time.

I know at least one of the other major frameworks handles it this way.

@jim-parry What are your thoughts?

@nowackipawel
Copy link
Contributor Author

As for now I believe PRed behaviour is better than actual :)

@lonnieezell
Copy link
Member

You're right it's better than current, but that doesn't mean it's the right long-term solution. :)

Still waiting to hear @jim-parry thoughts.

@jim-parry
Copy link
Contributor

I favor the idea of declaring entity properties explicitly, and thus support this PR.

@lonnieezell
Copy link
Member

The part I'm concerned about is when you have a join, and select some fields from the other table. That property was not explicitly set on the object, but was implicitly defined through the query result, though I suppose it would still pass muster as existing at the time someone tried to get a property? It's late and I'm not honestly sure of that answer. Will have to investigate later.

@jim-parry
Copy link
Contributor

Hmmm. If you are planning that an entity be the result of a join, would you not then be able to "predict" that it should have a specific property? I haven't used the Entity class, and don't have a good feel for what would work best :-/

@nowackipawel
Copy link
Contributor Author

nowackipawel commented Apr 11, 2019 via email

@lonnieezell
Copy link
Member

@nowackipawel Now that my brain is a little clearer and less tired - you're right. I'm good with this then.

If you are planning that an entity be the result of a join, would you not then be able to "predict" that it should have a specific property?

sometimes, but not always since the entity will be used in different situations that might require different sets of data joined in, sometimes only for a single call.

@lonnieezell lonnieezell merged commit 2b0bbf4 into codeigniter4:develop Apr 11, 2019
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

3 participants