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

#1445: Reorganise classes around Scalar and Callable #1494

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/main/java/org/cactoos/experimental/Threads.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import org.cactoos.iterable.IterableOf;
import org.cactoos.iterable.Mapped;
import org.cactoos.list.ListOf;
import org.cactoos.scalar.CallableOf;

/**
* Allows to execute the tasks concurrently.
Expand Down Expand Up @@ -110,7 +109,12 @@ private Threads(
super(
() -> new Mapped<>(
Future::get,
new UncheckedFunc<>(fnc).apply(new Mapped<>(CallableOf::new, tasks))
new UncheckedFunc<>(fnc).apply(
new Mapped<>(
task -> () -> task.value(),
tasks
)
)
).iterator()
);
}
Expand Down
14 changes: 11 additions & 3 deletions src/main/java/org/cactoos/experimental/Timed.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.cactoos.iterable.IterableOf;
import org.cactoos.iterable.Mapped;
import org.cactoos.list.ListOf;
import org.cactoos.scalar.CallableOf;

/**
* Allows to execute the tasks concurrently within given timeout.
Expand Down Expand Up @@ -125,7 +124,11 @@ public Timed(
threads
);
try {
return executor.invokeAll(new ListOf<>(todo), timeout, unit);
return executor.invokeAll(
new ListOf<>(todo),
timeout,
unit
);
} finally {
executor.shutdown();
}
Expand All @@ -146,7 +149,12 @@ private Timed(
super(
() -> new Mapped<>(
Future::get,
new UncheckedFunc<>(fnc).apply(new Mapped<>(CallableOf::new, tasks))
new UncheckedFunc<>(fnc).apply(
new Mapped<>(
task -> () -> task.value(),
tasks
)
)
).iterator()
);
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/cactoos/func/BiFuncOf.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import org.cactoos.Func;
import org.cactoos.Proc;
import org.cactoos.Scalar;
import org.cactoos.scalar.CallableOf;
import org.cactoos.scalar.ScalarOf;

/**
* Represents many possible inputs as {@link BiFunc}.
Expand Down Expand Up @@ -115,7 +115,7 @@ public BiFuncOf(final Callable<Z> callable) {
* @since 0.32
*/
public BiFuncOf(final Runnable runnable, final Z result) {
this(new CallableOf<>(runnable, result));
this(new ScalarOf<>(runnable, result));
}

/**
Expand Down
103 changes: 0 additions & 103 deletions src/main/java/org/cactoos/scalar/CallableOf.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,20 @@
*/
package org.cactoos.scalar;

import java.util.concurrent.Callable;
import org.cactoos.Scalar;
import org.cactoos.func.FuncOf;
import org.cactoos.func.Repeated;

/**
* Callable that runs repeatedly for a number of times.
* Scalar that runs repeatedly for a number of times.
*
* @param <X> Type of output
* @param <T> Type of output
* @since 0.49.2
*/
public final class RepeatedCallable<X> implements Callable<X> {

public final class Repeated<T> implements Scalar<T> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabriciofx Not realated to the review. But what is the use case for such class?
It seems that it applies some side effects, which is bad for Scalar to have
For example:
new Repeated<>(new Count<>(1), 5)).value() == 6 (all other values lost)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreoss Sorry, but I didn't get the point. I did some code here to test and works fine (but I can't make Count<T> because, I can't increment something like T:

@Test
void countFiveTimes() throws Exception {
    new Assertion<>(
        "Must repeat count 5 times",
        new Repeated<>(
            new Count(1),
            5
        ),
        new ScalarHasValue<>(5)
    );
}

final class Count implements Scalar<Integer> {
    private AtomicInteger num;

    public Count(final int num) {
        this.num = new AtomicInteger(num);
    }

    @Override
    public Integer value() throws Exception {
        return this.num.incrementAndGet();
    }
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabriciofx I mean, that with Repeat only the last result is returned, and all of other iteratatios are lost. Which implies mutable state.
So Repeated may be seen as an encouragement to put mutable state in a Scalar, as in your Count example.

/**
* Callable.
* Scalar.
*/
private final Callable<X> callable;
private final Scalar<T> scalar;

/**
* Times to repeat.
Expand All @@ -48,23 +46,23 @@ public final class RepeatedCallable<X> implements Callable<X> {
/**
* Ctor.
*
* <p>If {@code max} is equal or less than zero {@link #call()} will return
* <p>If {@code max} is equal or less than zero {@link #value()} will
* return
* an exception.</p>
*
* @param cllbl Callable to repeat.
* @param sclr Scalar to repeat.
* @param count How many times.
*/
public RepeatedCallable(final Callable<X> cllbl, final int count) {
this.callable = cllbl;
public Repeated(final Scalar<T> sclr, final int count) {
this.scalar = sclr;
this.times = count;
}

@Override
public X call() throws Exception {
return new Repeated<>(
new FuncOf<>(this.callable),
public T value() throws Exception {
return new org.cactoos.func.Repeated<>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabriciofx Can this be moved to the primary ctor?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreoss done!

new FuncOf<>(this.scalar),
this.times
).apply(true);
}

}
49 changes: 39 additions & 10 deletions src/main/java/org/cactoos/scalar/ScalarOf.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,32 +23,61 @@
*/
package org.cactoos.scalar;

import org.cactoos.Func;
import org.cactoos.Proc;
import org.cactoos.Scalar;
import org.cactoos.func.FuncOf;

/**
* ScalarOf.
*
* @param <X> Element type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabriciofx This type parameter should be on ctor level.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreoss Can you explain it better? Shoul I add X to ctor, as public <X> ScalarOf(final Runnable runnable, final T result)?

Copy link
Contributor

@andreoss andreoss Nov 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabriciofx Exactly, it should be like that

    public <X> ScalarOf(final Func<? super X, ? extends T> fnc, final X ipt) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreoss done, and thanks for the help! :)

* @param <T> Element type
* @since 0.4
*/
public final class ScalarOf<T> implements Scalar<T> {
public final class ScalarOf<X, T> extends ScalarEnvelope<T> {
/**
* Ctor.
* @param runnable Encapsulated proc
* @param result Result to return
* @since 0.32
*/
public ScalarOf(final Runnable runnable, final T result) {
this(
() -> {
runnable.run();
return result;
}
);
}

/**
* The scalar.
* Ctor.
* @param proc Encapsulated proc
* @param ipt Input
* @param result Result to return
* @since 0.41
*/
private final Scalar<T> origin;
public ScalarOf(final Proc<X> proc, final X ipt, final T result) {
this(new FuncOf<>(proc, result), ipt);
}

/**
* Ctor.
*
* @param origin The scalar
* @param fnc Encapsulated func
* @param ipt Input
* @since 0.41
*/
public ScalarOf(final Scalar<T> origin) {
this.origin = origin;
public ScalarOf(final Func<X, T> fnc, final X ipt) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabriciofx Func & Proc parameters should follow PECS (not sure about `Scalar``).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreoss done!

this(() -> fnc.apply(ipt));
}

@Override
public T value() throws Exception {
return this.origin.value();
/**
* Ctor.
*
* @param scalar The scalar
*/
public ScalarOf(final Scalar<T> scalar) {
super(scalar);
}
}
44 changes: 0 additions & 44 deletions src/main/java/org/cactoos/scalar/ScalarOfCallable.java

This file was deleted.

Loading