Collapsible does not share common class structure with HorizontalCollapsible

kotcrab / vis-ui

libGDX UI toolkit

Apache License 2.0

720 stars 129 forks source link

Collapsible does not share common class structure with HorizontalCollapsible #253

Open czyzby opened 7 years ago

czyzby commented 7 years ago

These two widgets share similar overall API, but do not implement a common interface or extend an abstract class. Joining these two with a common interface could shorten both of their implementations (no code duplication) and simplify tools like LML.

kotcrab commented 7 years ago

I agree. It's kind of messy now because there is CollapsibleWidget (since 0.3.1) and HorizontalCollapsibleWidget (since 1.2.5). Not sure what to do, I could either:

break API; rename CollapsibleWidget to VerticalCollapsibleWidget and add CollapsibleWidget interface. If we do renaming we can also consider dropping Widget suffix, not sure if it servers any purpose here
do not rename and add Collapsible interface. I would rather avoid I prefix for interfaces

What do you think would be better?

czyzby commented 7 years ago

There's always abstract class AbstractCollapsibleWidget, where you could move the common code. You wouldn't break the API (although this might make sense for naming consistence) and would only slightly affect both of their class hierarchies. This shouldn't be an issue for most users.

But yeah, if you're not afraid to break the API - or keep the classes deprecated for the next release - I'd probably go with Collapsible interface, AbstractCollapsible providing basic method implementations (because we all love Java 6 and lack of default methods) and the actual widgets named HorizontalCollapsible/VerticalCollapsible for code conciseness.