Improve SelectionState interface
LeoColman opened this issue · 5 comments
Context
I believe this interface can be improved to accommodate more use cases.
In my case, I had to implement the interface, but I don't want the selection to be changeable by the user:
val selection = object : SelectionState {
override fun isDateSelected(date: LocalDate): Boolean {
return date in currentSelected.dates
}
override fun onDateSelected(date: LocalDate) {
}
}
You see that I'll have to leave an empty method here, or override it to Unit?
The problem
The SelectionState interface isn't flexible on what you must implement
Possible solutions
Interface Segregation
(On a quick note, this is one of the solid principles, the Interface Segregation Principle)
We can extract the interface in two interfaces, but I"m not completely sure how we could adapt the rest of the code to fit this. I suppose that this is still an early project (0.x as per the documentation), it's ok to break it or add a deprecated version of the method until the 1.0.0 release.
They're both marked as stable, so this point might not hold true :P
// Take a look at this fun interface
// https://kotlinlang.org/docs/fun-interfaces.html
fun interface DateSelector : Consumer<LocalDate> {
// Empty body, method `accept` inherited from `Consumer`
}
fun interface DateSelectionPredicate : Predicate<LocalDate> {
// Empty body, method `test` inherited from `Predicate`
}
interface DateSelectionHandler : DateSelector, DateSelectionPredicate {
override fun test(t: LocalDate): Boolean {
return false
}
override fun accept(t: LocalDate) {
}
}
And then we adjust the code to ask only for what it needs from the user.
Adding default values
Moving the defaults from EmptySelectionState
to SelectionState
will allow you to override exactly what you want.
The Adapter pattern will be created this way
@Stable
public interface SelectionState {
public fun isDateSelected(date: LocalDate): Boolean = false
public fun onDateSelected(date: LocalDate) = Unit
}
I really like solution 1, although it needs more refinement.
Solution 2 makes this issue a low-hanging fruit. I'm open to discuss and implement both of these.
Hi @LeoColman Thank you very much for a contribution with such detailed analysis! Will try to analyze it myself today and get back to you with my feedback.
With the approach 1, I'm just wondering what it will add from the user perspective. From the context of Calendar
we will have to pass the interface handling both input and output anyway, as you don't have information what the consumer will need. So in the end you even if you don't need both applying and changing the selection, you will have to adapt it in some way on the call site to make it work, but maybe I'm missing something 😄 As for the approach 2, I think it's a great little addition and will simplify things, without breaking anything.
@LeoColman If you want to go with the 2nd approach or provide some more information on approach no1, please feel free to do so :)
The second approach is easier.
The first one is more complex, but more flexible in the end.
I'll try to code something out and submit a PR.