Simplify responsibility and implementation of FirstOf

yegor256 / cactoos

Object-Oriented Java primitives, as an alternative to Google Guava and Apache Commons

https://www.cactoos.org

MIT License

736 stars 163 forks source link

Simplify responsibility and implementation of FirstOf #1137

Closed victornoel closed 5 years ago

victornoel commented 5 years ago

I noticed that FirstOf was responsible of two different things:

to take one element in an Iterable
to filter an Iterable

This is its current constructor:

    /**
     * Constructor.
     * @param cond Condition for getting the element
     * @param src Source iterable
     * @param fbck Fallback used if no value matches
     */
    public FirstOf(final Func<T, Boolean> cond, final Iterable<T> src, final Scalar<T> fbck) {

I think it would be much more maintainable if:

FirstOf was only responsible of taking the first element of an Iterable
It was implemented simply by delegating to ItemAt.

There is already Filtered to filter an Iterable, so one will be able to write:

new FirstOf<>(new Filtered<>(cond, iterable));

0crat commented 5 years ago

@llorllale/z please, pay attention to this issue

llorllale commented 5 years ago

@victornoel I wonder if we should just delete FirstOf

victornoel commented 5 years ago

@llorllale ah yes, it could also work :)

llorllale commented 5 years ago

@victornoel created poll in telegram chat: https://t.me/elegantobjects/5348

llorllale commented 5 years ago

@victornoel the thugs over at https://t.me/elegantobjects voted against removing FirstOf

I also don't think they'll appreciate the change you're requesting.

victornoel commented 5 years ago

@llorllale then at least we could rewrite FirstOf with ItemAt and Filtered to avoid code duplication and complexity?

llorllale commented 5 years ago

@0crat in

0crat commented 5 years ago

@0crat in (here)

@llorllale Job #1137 is now in scope, role is DEV

0crat commented 5 years ago

Bug was reported, see §29: +15 point(s) just awarded to @victornoel/z

0crat commented 5 years ago

The job #1137 assigned to @scrobot/z, here is why; the budget is 30 minutes, see §4; please, read §8 and §9; if the task is not clear, read this and this; there will be no monetary reward for this job

scrobot commented 5 years ago

@0crat wait #1160

0crat commented 5 years ago

@0crat wait #1160 (here)

@scrobot The impediment for #1137 was registered successfully by @scrobot/z

0crat commented 5 years ago

The architect of the project has changed; @llorllale/z is not at this role anymore; @paulodamaso/z is the architect now

victornoel commented 5 years ago

@scrobot I didn't know this was fixed, you should have told, I would have closed this sooner

0crat commented 5 years ago

The job #1137 is now out of scope

0crat commented 5 years ago

Order was finished: +30 point(s) just awarded to @scrobot/z