Proposal: Feature - AbstractViewOwner#9
Conversation
…methods are the essential ones.
|
This code is dumb, I totally forgot you can't call super two levels up, so this exact example doesn't work, but I'm leaving it because you still get the gist of what I'm looking at here. |
… default methods that reference the original ViewOwner.
|
Okay, better version now, this makes more sense for my explanation, I'd actually also be happy if this was the final solution, works for me. |
|
I think you make a good point, but most of my ViewOwner implementations don't implement all of the fundamental methods (create/init/reset/respond). The createUI() method, in particular, is almost never implemented since the common case is to put the XML in the MyViewOwner.snp file which will be loaded automatically with the default createUI() method. The initUI() method is regularly not needed if the UI is straight forward. It is mostly needed to configure things that aren't covered by the XML. I've even had a few cases where one of the resetUI()/respondUI() methods isn't implemented, though this is rare. Might happen if the UI is read-only (no respond method needed). Or if the UI only has buttons in it (no reset needed). |
Yeah, this is one of the thoughts I was having as well. This would be one of the advantages to having the Another way I didn't think about until later was, could also make the other methods final, but I think relying on final is one of those things that can get you stuck in a hole and have people unable to use things for a purpose different than what was first thought. There have been a couple other projects running similar init, loop, react type models, such as JMonkeyEngine. In their particular case, they implemented everything in a "Simple" class, and then have you override what is necessary, but that requires you to fully understand the library before you learn it. It takes time for that knowledge to sink in, and it's easy to forget if you spend some time away. Especially when there are lots of other methods in the same class that you have to check out. That's why I was leaning towards the extension class, since you can always downgrade it back to the parent once you are more confident/ once you have implemented what is necessary. It also therefore doesn't break any existing or future workflows.It's what I'm running with right now, since I keep forgetting the method names of the stages. It might be making more work about nothing, but this is one of those things that I found quite helpful on my own, knowing what I am supposed/allowed to change, and what I should leave alone. Especially during the learning stage, being able to stay in the code editor and not switch out all the time is nice. I should say though, not heartbroken if you don't want it. I can just as easily add it as a class in my own project, just thought it might be nice to help people get a handle on this library faster. |
This pull request is an request for discussion.
I would like something similar to this in the project to assist with on-boarding people who are not familiar with the format (including myself). Right now the ViewOwner class is difficult to tell which methods are meant to be overridden, and which ones are not, at a glance. The starting out wiki page helps with this, but the API contract should reflect this as well.
I can see several different ways that it could be done.
ViewOwnerintoAbstractViewOwner, make the major methods abstract there, and then provide default implementations inViewOwnerclass (orDefaultViewOwneror something). The pluses with this way is that all existing code would continue to work, but that most references toViewOwnershould be refactored toAbstractViewOwnerViewOwnerand make the modifiable methods abstract in that class. This is what I have done in the provided code, but I would recommend a different classname, becauseAbstractViewOwnerimplies that it is the parent ofViewOwner, rather than the other way around. This way requires no refactoring at all, but would have your documentation directed towards this new abstract class, rather thanViewOwnerdirectly. It also goes against common convention, but isn't a big deal because it doesn't break anything.1.i.in the previous statement for the end user.I have no hard demands one way or another, but this was something that got me a little stumped at the start, and if it wasn't for the how-to-guide, I would not have been able to guess this from the code. I personally would prefer that the main ViewOwner class was abstract (especially since this library expects you to extend it), but that default implementations were available in a default class or method. That said, the code example I've provided works as is with no changes required, but it doesn't match the Java pattern very well. Give it a different name and it might be totally fine though.
Let me know what you think, these are just some starter ideas I came up with, but so long as there is an easy way to determine the API expectations, I'm all for it.