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

One Tuple class to rule them all #3

Open
bmccutchon opened this issue Dec 28, 2014 · 8 comments · May be fixed by #4
Open

One Tuple class to rule them all #3

bmccutchon opened this issue Dec 28, 2014 · 8 comments · May be fixed by #4

Comments

@bmccutchon
Copy link

I'd like to be able to write methods like this:

public static <T> Tuple<T> average(Tuple<T>... tuples) {

...rather than writing multiple methods to handle the different types of Tuples.

This could be accomplished by declaring a new Tuple class that Tuple3d, Tuple2f, etc. will extend, like so:

public class Tuple<T extends Tuple<T>> {

And in the extending classes:

public class Tuple3f extends Tuple<Tuple3f> {

If you like the idea, I might fork the project and work on it myself.

@JohnnyO
Copy link

JohnnyO commented Jan 1, 2015

The problem here is that there aren't any arithmetic operations that can be invoked dynamically on the primitive number classes. So, for example, you can't implement Number add(Number n1, Number n2) without a lot of casting, ugliness and slowdown.

See here for some more details.
http://stackoverflow.com/questions/2721390/how-to-add-two-java-lang-numbers

@bmccutchon
Copy link
Author

I'm aware of that, but it doesn't matter here: you just define abstract methods, like this:

public abstract class Tuple<T extends Tuple<T>> {
    public abstract void add(T tuple);
    public abstract void sub(T tuple);
    ...
}

Note that this will only allow you to add tuples of the same number and type, so what you mentioned will never be a problem. Also, the implementations of these methods would just be the existing add() and sub() methods.

@JohnnyO
Copy link

JohnnyO commented Jan 2, 2015

What about all the methods, like scale(float factor), set(float [] values), get(float []values), clamp(float value), etc? . You couldn't really abstract these away.

If Java had true OO support for the Number class, I think this would make sense, but until it does, I think you are going to run into some major headaches, and not be able to build as clean a solution as you'd like.

@bmccutchon
Copy link
Author

That's exactly what I have in mind -- they'd all be abstract, unless an implementation can be provided at the top level. For example, it would be possible to write the static average() method mentioned above using the add() method. Otherwise, the existing implementations can be used. This abstract class is really more like an interface, maybe with a few concrete methods.

@bmccutchon
Copy link
Author

I started trying to implement this and now I see how it could be a problem. While there wouldn't be any issue specifying something like this:

public abstract class Tuple<T extends Tuple<T, N>, N extends Number> {
    public abstract void add(T t);
    public abstract void scale(N s);
    ...
}

The problem would come when trying to use the scale method in a generic context, like so:

@SafeVarargs
public static <T extends Tuple<T, N>, N extends Number> T average(T... tuples) {
    T avg = sum(tuples);
    // The following fails to compile:
    // "The method scale(N) in the type Tuple<T,N> is not applicable for the arguments (double)"
    avg.scale(1.0 / ((double) tuples.length));
    return avg;
}

The sum method, however, can be written, since it relies only on the add method, which does not accept a number. The average method could be written as four methods, if their signatures look like this:

public static <T extends Tuple<T, Double>> T average(T... tuples) {

However, at this point, it would be better for the sake of performance* (and another reason I will describe below) to specify four classes corresponding to each of the four primitive types that tuples can hold. These would extend Tuple. Tuple would specify all methods that accept Tuples only, like the add method, because these can be specified without reference to the primitives. The four subclasses would specify methods like scale(float) and get(float[]) that reference primitives.

  • Tuple
    • Tupled
      • Tuple2d, Tuple3d, Tuple4d
    • Tuplef
      • ...
    • Tuplei
    • Tupleb

This would allow a method like average to be written as 4 methods instead of 11.

The other reason for doing it this way that I mentioned above is that scale(double) is not a valid implementation of scale(Double), but using generics to specify scale(N) would require that scale(Double) be implemented.

* For a reference on autoboxing performance, check out this answer by corsiKa on Stack Overflow.

@ghost
Copy link

ghost commented Jul 9, 2015

I don't really see the benefit of this request for enhancement and I see some pitfalls (abuse of auto-boxing, code hardly readable with tons of parametrized classes, ...). Personally, I would reject this proposal. @hharrison Please make a decision, it's up to you.

@bmccutchon
Copy link
Author

So, in the process of implementing this in my fork, I decided to ditch all autoboxing and some of the parametrization, only declaring the abstract methods that could be declared without "abuse of autoboxing." I think this mostly resolves the issues brought up by @gouessej. The generics are far more readable than the ones in my previous comment, and there is no autoboxing. However, I will admit that this limits the usefulness of this feature and leaves me wishing that Java had better support for generics. Still, it might as well be added, since it's been written and it improves functionality. Should I submit a pull request?

@ghost
Copy link

ghost commented Jul 10, 2015

Does it break the public API? If it doesn't, make a pull request and maybe it will be accepted.

@bmccutchon bmccutchon linked a pull request Jul 13, 2015 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants