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

MaxOf and MinOf #460

Closed
yegor256 opened this issue Nov 8, 2017 · 15 comments
Closed

MaxOf and MinOf #460

yegor256 opened this issue Nov 8, 2017 · 15 comments

Comments

@yegor256
Copy link
Owner

yegor256 commented Nov 8, 2017

Let's rename Max to MaxOf, Min to MinOf and make it possible to provide a list of primitives there, just like SumOf works. Also, I think we need to implement them the way SumOf works.

@bedward70
Copy link
Contributor

bedward70 commented Nov 10, 2017

Hi, Yegor!
I think it is impossible to support primitive arrays (like SumOf) in the MaxOf and MinOf classes:

  1. We can't set the Comparable generic type in bodies of the constructors;
  2. If MaxOf and MinOf are final classes, we will not extend the classes with Comparable generic type;
  3. If MaxOf and MinOf are rewritten with the common Comparable type (without generics), the input class type of the values will be lose. So, we will not get a result with origin class type.

In current time:
a) We can write implementations of MaxOf and MinOf for each primitive;
b) We can transform primitive arrays to an Iterable then use MaxOf and MinOf constructors:

        final Iterable<Scalar<Long>> iterable = new Mapped<>(
            input -> () -> input,
            new ListOf<>(
                LongStream.of(10000000000L, 20000000000L,  20000000000L)
                    .boxed()
                    .collect(Collectors.toList()).iterator()
            )
        );
        final Long result = new MinOf<>(iterable).value();

@yegor256
Copy link
Owner Author

@bedward70 please, take a look at how SumOf is implemented. Can't we do the same with MaxOf? Can you give code example that doesn't work with SumOf?

@bedward70
Copy link
Contributor

@yegor256
Sorry for your time, but I will try to explain.

I consider the MaxOf must return generic original type.
For example:

public class App {
    public static void main(String[] args) throws Exception {
       final String max = new MaxOf<>(() -> "A",() -> "B").value();
    }
}

please, take a look at how SumOf is implemented. Can't we do the same with MaxOf?

In current implementation if we add public 'MaxOf(final int... src)' constructor (no generic types in the constructor declaration), we can't set generic members.

If we do MaxOf like SumOf, we will get the Comparable interface only. So, we will have to use casting a result. I think it is terrible.
Example:

public final class MaxOf implements Scalar<Comparable> {

    private final Scalar<Comparable> result;

    public MaxOf(final int... src) {
        this(new Mapped<>(
            input -> () -> input,
            IntStream.of(src).boxed().collect(Collectors.toList())
        ));
    }

    public MaxOf(final Iterable<Scalar<Comparable>> iterable) {
        this.result = new Folded<>(
            new MaxFunc<>(),
            iterable
        );
    }

    @Override
    public Comparable value() throws Exception {
        return this.result.value();
    }
}

public class App {

    public static void main(String[] args) throws Exception {
       final MaxOf maxOf = new MaxOf(1, 2, 3, 15);
        Integer result = (Integer) maxOf.value();
        System.out.println(result);
    }
}

Can you give code example that doesn't work with SumOf?

I can't give any example code.
If java let to create new primitive type, I can't use new primitive type as result from SumOf.
The SumOf class declaration does not generic types. The SumOf is very limited and we perform casting by Number methods.

@yegor256
Copy link
Owner Author

@bedward70 please, see the changes I made in 25dc45d What do you think about these MinOf and MaxOf classes? They are in version 0.24 already.

@bedward70
Copy link
Contributor

bedward70 commented Nov 27, 2017

@yegor256 the MinOf and MaxOf is OK with primitive types and their classes only.
After this commit we lost the opportunity to use Comparable implementations:

public class App {
    public static void main(String[] args) throws Exception {
       final String max = new MaxOf<>(() -> "A",() -> "B").value();
    }
}

So, we also need the old MinOf and MaxOf implementations for all other Comparable implementations. There are a lot of them, here are the famous ones: Calendar, Date, Character, String, Timestamp.
An user will be able to implement Comparable interface for his classes and will be able to use MinOf and MaxOf for his classes.

yegor256 added a commit that referenced this issue Nov 28, 2017
@yegor256
Copy link
Owner Author

@bedward70 I guess we just need two new classes HighestOf and LowestOf in order to work with Comparables... What do you think?

yegor256 added a commit that referenced this issue Nov 28, 2017
@bedward70
Copy link
Contributor

@yegor256 it is great. If we take Max and Min(with tests) before 25dc45d commit and rename to HighestOf and LowestOf, we will able to do it easily.
Should I help?

@yegor256
Copy link
Owner Author

@bedward70 yes, please, go ahead. Thanks!

@yegor256
Copy link
Owner Author

@rultor release, tag is 0.25.3

@rultor
Copy link
Collaborator

rultor commented Nov 28, 2017

@rultor release, tag is 0.25.3

@yegor256 OK, I will release it now. Please check the progress here

@rultor
Copy link
Collaborator

rultor commented Nov 28, 2017

@rultor release, tag is 0.25.3

@yegor256 Done! FYI, the full log is here (took me 10min)

@0crat
Copy link
Collaborator

0crat commented Nov 28, 2017

Oops! There is no order for job gh:yegor256/cactoos#460.

@0crat
Copy link
Collaborator

0crat commented Nov 28, 2017

Oops! Job gh:yegor256/cactoos#460 is not in scope.

bedward70 pushed a commit to bedward70/cactoos that referenced this issue Nov 28, 2017
@yegor256 yegor256 reopened this Dec 1, 2017
@yegor256 yegor256 closed this as completed Dec 1, 2017
@0crat
Copy link
Collaborator

0crat commented Dec 1, 2017

Oops! There is no order for job gh:yegor256/cactoos#460.

@0crat
Copy link
Collaborator

0crat commented Dec 1, 2017

Oops! Job gh:yegor256/cactoos#460 is not in scope.

This was referenced Jan 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants