Converter question / suggestion? #45

Closed
jnash67 opened this Issue May 2, 2014 · 4 comments

Comments

Projects
None yet
2 participants
@jnash67

jnash67 commented May 2, 2014

In interface Converter why is Class getType() not Class getType() ?

I added the following class to my project to get rid of some boilerplate:

import org.csveed.bean.conversion.AbstractConverter;

public abstract class EasierAbstractConverter<K> extends AbstractConverter<K> {

    private final Class<K> clazz;

    public EasierAbstractConverter(Class<K> clazz) {
        this.clazz = clazz;
    }

    @Override
    public Class getType() {
        return clazz;
    }
}
@robert-bor

This comment has been minimized.

Show comment
Hide comment
@robert-bor

robert-bor May 5, 2014

Owner

Eliminating boilerplate is a noble goal, which I heartily support. One way to achieve this is to introduce a constructor in AbstractConverter which sets clazz, as you do in EasierAbstractConverter.

However, don't you just move boilerplate around by introducing the need for an extra no-arg constructor in an implementing class?

Looking forward to your reaction.

Owner

robert-bor commented May 5, 2014

Eliminating boilerplate is a noble goal, which I heartily support. One way to achieve this is to introduce a constructor in AbstractConverter which sets clazz, as you do in EasierAbstractConverter.

However, don't you just move boilerplate around by introducing the need for an extra no-arg constructor in an implementing class?

Looking forward to your reaction.

@jnash67

This comment has been minimized.

Show comment
Hide comment
@jnash67

jnash67 May 5, 2014

It's just a (potentially substantial) reduction in boilerplate. You add some number of characters of boilerplate in the constructor while removing 5 lines of boilerplate when overriding the method (4 lines of method + one blank line -- the exact number of lines, of course, depends on one's formatting and spacing conventions.

I noticed a substantial reduction in a method where I was consecutively setting converters for 4 or 5 fields.

There's also a slight improvement in clarity. I mis-typed my first sentence to you. The question should have read:

Why is Class getType() not Class getType()?

With K in the constructor, There's no confusing that K is what should be returned. When overriding getType with the , after not looking at it for a bit, I had to give a little thought to what class the method was supposed to return.

Jonathan

jnash67 commented May 5, 2014

It's just a (potentially substantial) reduction in boilerplate. You add some number of characters of boilerplate in the constructor while removing 5 lines of boilerplate when overriding the method (4 lines of method + one blank line -- the exact number of lines, of course, depends on one's formatting and spacing conventions.

I noticed a substantial reduction in a method where I was consecutively setting converters for 4 or 5 fields.

There's also a slight improvement in clarity. I mis-typed my first sentence to you. The question should have read:

Why is Class getType() not Class getType()?

With K in the constructor, There's no confusing that K is what should be returned. When overriding getType with the , after not looking at it for a bit, I had to give a little thought to what class the method was supposed to return.

Jonathan

@jnash67

This comment has been minimized.

Show comment
Hide comment
@jnash67

jnash67 May 5, 2014

Sorry, the editor keeps messing up my question. It is:

Why is Class getType() not Class<K> getType()?  

jnash67 commented May 5, 2014

Sorry, the editor keeps messing up my question. It is:

Why is Class getType() not Class<K> getType()?  
@robert-bor

This comment has been minimized.

Show comment
Hide comment
@robert-bor

robert-bor May 5, 2014

Owner

I have decided to adopt your suggestion, it's definitely better. The Converter hierarchy is almost literally copied from the Spring framework, so I guess they would be well served by your suggestion as well.

Thanks!

Owner

robert-bor commented May 5, 2014

I have decided to adopt your suggestion, it's definitely better. The Converter hierarchy is almost literally copied from the Spring framework, so I guess they would be well served by your suggestion as well.

Thanks!

robert-bor added a commit that referenced this issue May 5, 2014

Issue #45 adopted suggestion by jnash67 to set the Class in AbstractC…
…onverter, so that implementing classes do not have to override getType. Instead a super(Class) call to AbstractConverter must be made

@robert-bor robert-bor added this to the v0.4.0 milestone May 6, 2014

@robert-bor robert-bor closed this May 6, 2014

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