Closed karolherbst closed 10 years ago
Looks good to me.
I am quite unhappy, that https://github.com/karolherbst/Gamekeeper-Framework/commit/5b00d2722ffd04a3316a983b798ecd2031b95fad takse so much loc :/
Yeah, it's kind of bad but you're not going to be adding more fields are you? If so, it's not scaling well and you should probably use a map, which loses some type safety but scales better.
I've addes some generic stuff here, to make it easier in the future
I've added some metaprogramming interface, so that it is much easier to implement a model interface
Initially it's hard to tell if this is going be worth the complexity given the use of the preprocessor, but this does allow much much much easier to extend as seen in 40ecadb909884b70926bcfadaf0ba4e2c1f56fc0 . However interactions in 397d0df0681627df793ff034742a4b28ab631757 with pugixml are causing some grief. It'd be interesting to do a diff ignoring the comments (which I think the majority of new lines are from.) Looking at the lines diff by file I'd guess this cuts down on the Gamekeeper code where the actual concerns for scaling is by a lot, but the macros file is huge (not sure whether this is comments or lines, or even if that matters) and adding source to the model creates new dependencies.
I'm cautious about getting behind this considering the amount of complexity involved with the preprocessor. Regardless, I don't think I have enough information as to how beneficial this actually is without estimates on the future number of fields that would be added, how often and how tedious it'd become to maintain.
though the issues in https://github.com/karolherbst/Gamekeeper-Framework/commit/397d0df0681627df793ff034742a4b28ab631757 doesn't have any relation to the preprocessor stuff.
the issue is, that a store might not tell a homepage or a description or anything else so that this property would be missing in the store config file and pugixml throws an exception
I also do it is worth it, because a change in a model can lead to changes across the different backends. I want try to generalize there a lot more, so that we don't need to add hundreds of lines of codes to just add a simple property. I would like to tackle that now instead later
this is the backend changes. Store files has to be updated