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

Return type of ValueConverter.valueType() should be Class<? extends V> #59

Closed
TjlHope opened this Issue Jul 29, 2014 · 4 comments

Comments

Projects
None yet
2 participants
@TjlHope

TjlHope commented Jul 29, 2014

This would allow the flexibility of decoupling the generic class definition from the return type implementation.

My specific example is that I have a ValueConverter<Iterable<String>> that I want to read a file and return the lines in a TrieSet (a custom class that implements Set<String>), so although the convert method return is fine, currently the valueType method must be defined as:

    @SuppressWarnings("unchecked")
    @Override public Class<Iterable<String>> valueType() {
        return (Class<Iterable<String>>) (Class<? extends Iterable<String>>) TrieSet.class;
    }

Whereas, with an interface definition of Class<? extends V> valueType();, this simply becomes:

    @Override public Class<? extends Iterable<String>> valueType() {
        return TrieSet.class;
    }

and also allows for overloading the return type to Class<? extends TrieSet> (or simply Class<TrieSet>, though that would be re-creating the problem).

I realise that changing to a ValueConverter<TrieSet> would be another way to solve this simple example, but this is just one of several ValueConverters I have in a hierarchy, sharing and overloading methods as necessary, meaning I need to use the shared superclass/interface for the return type (in this case Iterable<String>).

@pholser

This comment has been minimized.

Show comment
Hide comment
@pholser

pholser Jul 29, 2014

Collaborator

@TjlHope This sounds reasonable. Will look into it -- thanks for your interest!

Collaborator

pholser commented Jul 29, 2014

@TjlHope This sounds reasonable. Will look into it -- thanks for your interest!

@TjlHope

This comment has been minimized.

Show comment
Hide comment
@TjlHope

TjlHope Jul 29, 2014

@pholser Thanks for the quick reply (and the great lib)!
I should have mentioned that as far as I can tell, the change wouldn't break any client code as, to use my example, Class<Iterable<String> is an acceptable @Override of Class<? extends Iterable<String>>

TjlHope commented Jul 29, 2014

@pholser Thanks for the quick reply (and the great lib)!
I should have mentioned that as far as I can tell, the change wouldn't break any client code as, to use my example, Class<Iterable<String> is an acceptable @Override of Class<? extends Iterable<String>>

pholser added a commit that referenced this issue Jul 30, 2014

Merge pull request #60 from pholser/feature/59/relax-value-converter-…
…value-type

For #59, relax return type of ValueConverter.valueType()
@pholser

This comment has been minimized.

Show comment
Hide comment
@pholser

pholser Jul 30, 2014

Collaborator

Latest 4.7 snapshot at Sonatype should have this. I am planning to declare a 4.7 beta 1 soon also.

Collaborator

pholser commented Jul 30, 2014

Latest 4.7 snapshot at Sonatype should have this. I am planning to declare a 4.7 beta 1 soon also.

@TjlHope

This comment has been minimized.

Show comment
Hide comment
@TjlHope

TjlHope Jul 30, 2014

Great, thanks!

TjlHope commented Jul 30, 2014

Great, thanks!

@TjlHope TjlHope closed this Jul 30, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment