Model Class To Array Does Not Use Get Magic Method Or Allowed Fields Property

[SmolSoftBoi]

Because the Model->classToArray() method use reflection, it does not get values use the __get() magic method and could produce incorrect data, also I believe if the Model->allowedFields property is set then it should attempt to get those properties from the class too.

If this is something that's agreed I can implement this and create a pull request.

[lonnieezell]

I am generally good with this, and will help get the Entity classes actually working correctly, as was discussed on the forums.

I'd like to know a tad more about how you plan to implement things:

• Since not every class will have a magic getter implemented, how would you detect the difference between a null reponse from get() and a null value? I would think we'd want to fallback to reflection to verify the value?
• Using allowedFields is tricky I think, and I'd love to hear your thoughts on it. While I think it's generally desirable, There are also times that I could see a user creating a new Entity, setting values that would not normally be updatable, like created on, id, etc, and wanting to save it. For that matter, the model itself might do that, which should be considered a safe operation that wouldn't need mass assignment protection.

[SmolSoftBoi]

@lonnieezell I'll try to get an example of what I'm currently doing, it's a little messy and could probably use improving, but I think it's a good starting point. Entities can be a very powerful part of CodeIgniter I would imagine.

[SmolSoftBoi]

@lonnieezell How would be best to show you an example of what I'm currently doing? I could upload it and provide you with a link.

[lonnieezell]

@EpicKris You could do that, or provide a GIST, or, if it's short enough, put it here in a code block.

[SmolSoftBoi]

Here is a GIST of my entities and data mappers: https://gist.github.com/EpicKris/619903892899843fe6596561ba2d7b70

[lonnieezell]

Looks a little more involved than I would want to put into core at this moment.

I've written and erased 3 different replies so far. :) The more I think about this, though, the more I think the best solution might be to implement a new BaseEntity class, that can handle the getX() and setX() methods, and could also potentially handle the data-mapping, and additional features. I actually wrote a blog post with a start on something like this a while back, that is close to your Domain class.

Here's my thinking, let me know your thoughts:

• Not everyone is going to use Entities/DataMapping, etc. So we should keep additional complexities the core database layer, since it's already complex enough as it is.
• By leaving it how it is now, people can choose to create their own custom base classes and provide a different style/syntax that matches their preferences, without being tied to what we implement here. I think that keeps with the traditional flexibility that people like about CodeIgniter.
• By implementing a BaseEntity class, we can provide additional tools like automatic timezone-handling, like mentioned in the blog post, in clean way that doesn't dirty up the db layer anymore, especially since it doesn't know anything about field types.
• The __get() and __set() methods could check for the object in a class property, datamap, where you could list the database field name and the class property it should map to. This would only require fields that had changed to be inserted, and would be a super-simple to use implementation of datamapper pattern, but would be very effective, I think.

[SmolSoftBoi]

On the surface, entities and data mappers seem like a simple task, but the more you play with them, the more complex they seem to become. While I agree with your thoughts, I did notice that there's an onSave() hook in the database layer for entities, perhaps a similar hook (like extract()) could be used to get the data in a clean way from the entity (if the method exists). A comprehensive base entity could include all the functionality we've discussed, would just need planning out in an appropriate way.