Abstract ViewController, Implementations, Documentation.

[Smithor]

I have created an alternate proposal to #9, and I would prefer this one over that one. I had this idea when we were first discussing it, but didn't want to propose it due to the refactoring it would require on your end. But, when I saw that you had decided to create the ViewController class, that provided the perfect opportunity to raise this pull-request.

The idea is to make ViewController an abstract class that makes the core methods that are expected to be overriden more transparent. To keep the ease of use though, it also adds two new implementations, SimpleViewController and DefaultViewController (We could shorten the names to something like SimpleController and DefaultController if that is more preferential. I have no attachment to the names themselves).

SimpleViewController acts as a gateway into the simple use of the library. Right now, it adds the default createUI() implementation, but we could also add a default void implementation for resetUI() as I don't think it is necessarily used in most simple cases. This means that in order to use the library, a user only needs to implement two methods (or 3 right now), and they will be able to use it. It is also more clear, and follows regular Java inheritance expectations (unlike proposal #9, which subverts them by adding the abstract version of the class as a child rather than as the parent).

DefaultViewController is the exact same implementation as what is currently in ViewController, with everything implemented requiring overriding to utilize.

All in all I think this is a much friendlier implementation for newcomers, and makes it much easier to start using the library. I've also tried to add lots of friendly documentation to assist a new user in figuring out the flow path, as its not a normal path to see outside of game design.

[Smithor]

I would appreciate some feedback on this one. This proposal is more proper than the previous one, but I'm happy to change anything to make it work.

[reportmill]

You make a great point and I definitely see the academic value of making ViewController abstract, but I think the practical value is less once you have to introduce DefaultViewController and SimpleViewController.

It's fair to expect anyone using ViewController to read a sentence about it first, which should say (despite my poor docs): "The ViewController class handles UI create, init, reset and respond and can be customized by overriding the methods createUI(), initUI(), resetUI() and respondUI()". If it then has to explain DefaultViewController and/or how to stub those methods if not needed, I think the overall simplicity is diminished.

So I think any changes just get a new SnapKit developer over the initial 5 minute conceptual hurdle at the expense of introducing good, but additional unnecessary structure. And in the age of AI, few people would probably find themselves in this position anyway. They would just say, "AI, create a simple ViewController example to modify a boolean value": https://share.google/aimode/g7QR4xy0TRM8f1j9h

Let me know what you think - I'm so thrilled you are showing interest I shouldn't rock the boat. I've tried to stick to a primary philosophy with SnapKit of "less is more" (it actually contains more than I wanted - but I also want no real external dependencies).

[Smithor]

I would be absolutely willing to drop the DefaultViewController and SimpleViewController in favour of merely having the abstract. The DefaultViewController class was more of a drop-in replacement anyway, I honestly don't like it existing, since it is just all classes made empty, and it should really be the child class' responsibility to assign those methods.

The only one I could see being sticky was the SimpleViewController. For a lot of the content you have prepared already, it is expected to contain the "load from snapxml" statement in the create. It is my opinion that this method should be included among the abstract ViewController messages, which requires something to add the functionality back. When I first set up the abstract environment in my test environment, I kept having a hard time remembering what method was used to load from file, so the SimpleViewController makes that more obvious. The alternative would be having another method in the class like "loadViewFromSNP", which you could then call in your createUI() method.

I have been using the SimpleViewController class in my own project, but I could work with it being just the abstract one. I just think it's important that critical things be vocal to the user, rather than silent.

As for boat rocking, I'm aware that I'm a bit of a boat rocker, so my feelings won't be hurt if you say no to something I say. I like this project, and would like to help it be more successful, but at the end of the day I'm coming into your house and your project that you've been working on by yourself for over 8 years, and I can imagine how it might feel like I'm stepping on toes. I don't intend anything ill-willed by the criticism I'm providing, just trying to help it be more open for other people. Don't worry about losing me, I've already decided I'm using SnapKit for the UI on the current project, which means you've got me for at least 3 work-months (Assuming the prototype doesn't get terminated by my superiors at the end of that process haha) one way or another. You'll probably still see me around even if it's on my own, I do like SnapKit after-all, and I would like it if it was actually a full and legitimate alternative to either Swing or JavaFX. For it to be there though, it needs to be extensible, and that is one thing that it is suffering from a bit right now.

From my understanding, each view has to define its own valid elements for the xml tree, which is okay, but it also isn't clear until you dive into the props package and start chasing implementations up the tree. This is required knowledge if you are wanting to build a new type of view that can be included in a snp file, and it is not intuitive. If a snp file is meant to be the default method of loading a class, then an arbitrary class that someone creates should be loadable from the snp file from the start, and they shouldn't have to understand the entire library to know that they need to override a method from a class 3-4 levels deep in the inheritance hierarchy. There should be something that alerts the developer to the requirement at the top level if it is still required, or be handled (properly, not with a stub but with actual default logic) by one of the ancestor classes.

If a snp file is not the default method, and is instead just a handy way you can also do things, then it isn't as important for clarity at the lower levels, because they won't be necessary for the novice snapkit developer. Still helpful, but not as important.

Sorry that this message kind of overlaps with #15, wrote both at the same time so the thoughts are mixed together. I've been handling the low-hanging fruit so far, abstract this and abstract that, because I'm still learning the library, and I'd rather save my big edits for when I feel more confident that I understand the codebase better, but even these shallow edits are a step towards an attempt to resolve the bigger issues I'm seeing.

Anyway, long story short, I'll stop tip-toeing around and just talk about the bigger stuff rather than just the little ones (and make proposals based on what I actually think, rather than preemptive compromises), and you feel free to shoot down any thing I propose that you don't like haha. I feel like we are actually getting along, I just felt kinda bad walking into your project with a bunch of proposed changes, and so I was trying to soften it as I went. Hard to know on a project like this with few contributors before me to act as an example.