Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FuncOf returning null on apply #551

Closed
neonailol opened this issue Jan 11, 2018 · 35 comments
Closed

FuncOf returning null on apply #551

neonailol opened this issue Jan 11, 2018 · 35 comments
Labels

Comments

@neonailol
Copy link
Contributor

neonailol commented Jan 11, 2018

According to readme this library advertised and null-free

* No `null` ([why?](http://www.yegor256.com/2014/05/13/why-null-is-bad.html))

But in FuncOf there is 2 constructors, that returns null on calling apply of retuned Func
Is this really nececary? Maybe they do not belong in there and should be moved to smth like FuncNlsOf or changed to better abstractions or simple removed

public FuncOf(final Proc<X> proc) {
this(proc, null);
}

public FuncOf(final Runnable runnable) {
this((Proc<X>) input -> runnable.run());
}

Also tests say that null is expected behaviour there

@Test
public void convertsProcWithNoResultIntoFunc() throws Exception {
final AtomicBoolean done = new AtomicBoolean(false);
MatcherAssert.assertThat(
new FuncOf<String, Boolean>(
input -> {
done.set(true);
}
).apply("hello you"),
Matchers.nullValue()
);
}

@Test
public void convertsRunnableIntoFunc() throws Exception {
final AtomicBoolean done = new AtomicBoolean(false);
MatcherAssert.assertThat(
new FuncOf<String, Boolean>(
() -> done.set(true)
).apply("hello, world"),
Matchers.nullValue()
);
}

Also i'm seeing same behaviour in constructors of CallableOf and BiFuncOf

@0crat
Copy link
Collaborator

0crat commented Jan 11, 2018

@yegor256 please, pay attention to this issue

@llorllale
Copy link
Contributor

@yegor256 I agree with this. Let us know if you differ

@llorllale
Copy link
Contributor

@0crat in

@0crat 0crat added the scope label Apr 20, 2018
@0crat
Copy link
Collaborator

0crat commented Apr 20, 2018

@0crat in (here)

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

@llorllale llorllale added the bug label Apr 20, 2018
@0crat
Copy link
Collaborator

0crat commented Apr 20, 2018

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

@0crat
Copy link
Collaborator

0crat commented May 5, 2018

The job #551 assigned to @SharplEr/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

@SharplEr
Copy link
Contributor

SharplEr commented May 5, 2018

@llorllale By the way, in case if all our objects are immutable then Proc<> have no sense. Proc<> can be used only for produce some side effects which are prohibited. I think it's okay to remove Proc<>. What do you think?

@llorllale
Copy link
Contributor

@SharplEr what we are against is null. Be careful when removing that ctor - it's used in a number of places.

@yegor256
Copy link
Owner

yegor256 commented May 6, 2018

@llorllale how can we fix that? I can't see any other solution...

@SharplEr
Copy link
Contributor

SharplEr commented May 6, 2018

@llorllale Okay. In case if I remove this ctor, it will affect about 15 other classes (tests included). So a big part of functionality will be lost. Do I need to save this functionality somehow?

@SharplEr
Copy link
Contributor

SharplEr commented May 6, 2018

@yegor256 I see one way, create new class:

FuncOptional<X, Y> implements Func<X,Optional<Y>>

Which can return Optional.empty() instead of null.

But unfortunately such code will not build:

public CallableOf(final Runnable runnable) {
    this(new FuncOptional<>(runnable));
}

Now I'm thinking about can I avoid this or not.

@llorllale
Copy link
Contributor

@yegor256

how can we fix that? I can't see any other solution...

I believe we should drop this functionality all together because it goes against our principles and also simply because Func returns a value while Proc doesn't. To me, they're not conceptually compatible. If the user wants to return null then they are free to use the overload FuncOf<Proc<X> proc, Y value).

@llorllale
Copy link
Contributor

llorllale commented May 6, 2018

@SharplEr although not stated as one of our principles, I know for a fact that @yegor256 doesn't like Optional.

I am personally not against Optional, but in this case the ready alternative for users is the FuncOf<Proc<T> proc, Y value) ctor.

SharplEr added a commit to SharplEr/cactoos that referenced this issue May 8, 2018
SharplEr added a commit to SharplEr/cactoos that referenced this issue May 8, 2018
SharplEr added a commit to SharplEr/cactoos that referenced this issue May 9, 2018
SharplEr added a commit to SharplEr/cactoos that referenced this issue May 9, 2018
SharplEr added a commit to SharplEr/cactoos that referenced this issue May 9, 2018
SharplEr added a commit to SharplEr/cactoos that referenced this issue May 10, 2018
SharplEr added a commit to SharplEr/cactoos that referenced this issue May 10, 2018
SharplEr added a commit to SharplEr/cactoos that referenced this issue May 10, 2018
@0crat
Copy link
Collaborator

0crat commented May 10, 2018

@SharplEr/z this job was assigned to you 5days ago. It will be taken away from you soon, unless you close it, see §8. Read this and this, please.

SharplEr added a commit to SharplEr/cactoos that referenced this issue May 10, 2018
SharplEr added a commit to SharplEr/cactoos that referenced this issue May 10, 2018
SharplEr added a commit to SharplEr/cactoos that referenced this issue May 10, 2018
@0pdd
Copy link
Collaborator

0pdd commented May 11, 2018

@neonailol the puzzle #861 is still not solved.

@SharplEr
Copy link
Contributor

@neonailol PR was merged, please close the issue.

@SharplEr
Copy link
Contributor

@0crat waiting
Waiting for author reaction.

@SharplEr
Copy link
Contributor

@neonailol ping

@0crat
Copy link
Collaborator

0crat commented May 13, 2018

@0crat waiting Waiting for author reaction. (here)

@SharplEr The impediment for #551 was registered successfully by @SharplEr/z

@0crat
Copy link
Collaborator

0crat commented May 14, 2018

@elenavolokhova/z please review this job completed by @SharplEr/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed

@0crat 0crat removed the scope label May 14, 2018
@0crat
Copy link
Collaborator

0crat commented May 14, 2018

The job #551 is now out of scope

@elenavolokhova
Copy link

@SharplEr According to our Policy:

Some of ticket messages must mention all pull requests where the problem was actually fixed;

There is no direct message with the link to corresponding PR.
Please, confirm that you will follow this rule in future.

@SharplEr
Copy link
Contributor

@elenavolokhova confirm
PR was #843

@elenavolokhova
Copy link

@0crat quality acceptable

@0crat
Copy link
Collaborator

0crat commented May 16, 2018

Order was finished, quality is "acceptable": +30 point(s) just awarded to @SharplEr/z

@0crat
Copy link
Collaborator

0crat commented May 16, 2018

Quality review completed: +8 point(s) just awarded to @elenavolokhova/z

@0pdd
Copy link
Collaborator

0pdd commented May 19, 2018

@neonailol 6 puzzles #883, #884, #885, #886, #887, #888 are still not solved; solved: #861.

@0pdd
Copy link
Collaborator

0pdd commented Sep 20, 2018

@neonailol 5 puzzles #883, #884, #885, #886, #888 are still not solved; solved: #861, #887.

@0pdd
Copy link
Collaborator

0pdd commented Oct 24, 2018

@neonailol 4 puzzles #884, #885, #886, #888 are still not solved; solved: #861, #883, #887.

@0pdd
Copy link
Collaborator

0pdd commented Jan 24, 2019

@neonailol 4 puzzles #1040, #884, #885, #886 are still not solved; solved: #861, #883, #887, #888.

@0pdd
Copy link
Collaborator

0pdd commented Jan 24, 2019

@neonailol 3 puzzles #1040, #884, #886 are still not solved; solved: #861, #883, #885, #887, #888.

@0pdd
Copy link
Collaborator

0pdd commented Feb 7, 2019

@neonailol 3 puzzles #1040, #1062, #884 are still not solved; solved: #861, #883, #885, #886, #887, #888.

@0pdd
Copy link
Collaborator

0pdd commented Feb 26, 2019

@neonailol 2 puzzles #1040, #1062 are still not solved; solved: #861, #883, #884, #885, #886, #887, #888.

@0pdd
Copy link
Collaborator

0pdd commented Apr 19, 2019

@neonailol the puzzle #1040 is still not solved; solved: #1062, #861, #883, #884, #885, #886, #887, #888.

@0pdd
Copy link
Collaborator

0pdd commented Apr 21, 2019

@neonailol all 9 puzzles are solved here: #1040, #1062, #861, #883, #884, #885, #886, #887, #888.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants