Java: pass parameter in constructor or method?

Question

Currently I have a class, TransactionData, which is just a little bit more than a POJO. I build the object from an HTTPServletRequest. What I do:

public class TransactionData
{

    // ...

    public TransactionData(HttpServletRequest request) throws IOException
    {
        // do actual work here
    }

}

There many WTF here, the most disturbing one is that the object TransactionData is tightly coupled to HTTPServletRequest. What I thought: create an interface, TransactionDataExtractor, with an extract() method, so that I might implement different classes to build the object.

public interface TransactionDataExtractor
{
    public TransactionData extract();
}

But how do I pass the stuff needed to build the TransactionData to every implementation? The firt thing that came to mind was to use the different constructors, like this:

public class TransactionDataExtractorRequest implements TransactionDataExtractor
{
    private HttpServletRequest httpRequest;

    public TransactionDataExtractorRequest(HttpServletRequest httpRequest)
    {
        this.httpRequest = httpRequest;
    }

    public TransactionData extract()
    {
        // do whatever is required
    }

}

But in this case whenever I need to build a new TransactionData object I have to create a new TransactionDataExtractorRequest. An implicit dependency I don't like at all. The other alternative I could think of was passing an Object parameter to extract() and cast it whenever required, giving up the type safety and introducing a lot of boiler plate ugly code

    public TransactionData extract(Object o)
    {
        HttpServletRequest h;
        if (o instanceof HttpServletRequest)
        {
             h = (HttpServletRequest)o;
        } 
        //...
    }

I don't know if I have made myself clear. I do feel like I'm missing something, I know the solution is very simple but I can't get hold of it. Any thoughts? TIA.

EDIT: the problem might even be that my hunch is completely wrong and I may dismiss it without any regret

What's wrong with having a dependency on HttpServletRequest? Assuming this class is meant to be used as part of some sort of webapp that seems like a completely reasonable dependency to have, to me. — aroth
– aroth, Commented Jul 19, 2011 at 11:02
The TransactionData is part of a complex protocol; part of the transmission is handled by a servlet, but other parts of the infrastructure (the unit tests, for example, or the client) are agnostic to HttpServletRequest. — Manrico Corazzi
– Manrico Corazzi, Commented Jul 19, 2011 at 12:30
Still not entirely convinced this is necessary, but see if my answer below is of any help. — aroth
– aroth, Commented Jul 19, 2011 at 13:07
I start to feel that aroth and Stephen C are indeed right: this kind of coupling really is not to be a concern. I accepted axtavt answer, though, because it encouraged me to try something I don't do often. — Manrico Corazzi
– Manrico Corazzi, Commented Jul 19, 2011 at 14:55

axtavt · Accepted Answer · 2011-07-19 11:34:44Z

4

If your only problem is ensuring type safety when passing the source object to extract(), you can use generics:

public interface TransactionDataExtractor<E> {
    public TransactionData extract(E source); 
} 

public class TransactionDataExtractorRequest 
    implements TransactionDataExtractor<HttpServletRequest> {
    public TransactionData extract(HttpServletRequest source) { ... }
}

answered Jul 19, 2011 at 11:34

axtavt

243k42 gold badges518 silver badges486 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Manrico Corazzi Over a year ago

Thank you very much for your answer; I admit I generally still don't use the generics as much as I would need/like to, maybe it's about time.

Stephen C · Accepted Answer · 2011-07-19 11:06:53Z

3

I do feel like I'm missing something ... Any thoughts?

Well my thought is that you are attempting to solve a problem that isn't really a problem. There's no obvious (to me) reason why the coupling you are trying to get rid of is actually harmful. Certainly, your attempts to remove the coupling are not making the code any easier to understand.

answered Jul 19, 2011 at 11:06

Stephen C

723k95 gold badges849 silver badges1.3k bronze badges

Comments

Bozho · Accepted Answer · 2011-07-19 11:07:37Z

3

In case you rely on request parameters only, you can get request.getParameterMap() and use the Map instead. (if you need headers - getHeaders())

answered Jul 19, 2011 at 11:07

Bozho

599k147 gold badges1.1k silver badges1.2k bronze badges

Comments

Thomas · Accepted Answer · 2011-07-19 11:15:05Z

1

Creating/reusing a TransactionDataExtractorRequest instance isn't a problem, IMHO. You'd need to somewhere distinguish between the parameter types anyway and if you decouple TransactionData and the parameter types by using some sort of factory, what's wrong with that?

answered Jul 19, 2011 at 11:15

Thomas

88.9k13 gold badges126 silver badges162 bronze badges

Comments

aroth · Accepted Answer · 2011-07-19 13:12:29Z

I'm still not convinced that much is gained by removing the dependency on HttpServletRequest, but I would suggest something along the lines of:

public class TransactionData {
    public TransactionData(TransactionDataOptions options) throws IOException {
        // do actual work here
    }
}

//TransactionData wants some state that it currently gets from a HttpServletRequest,
//figure out what that state is, and abstract an interface for accessing it
public interface TransactionDataOptions {
    //getters for things that TransactionData needs
}

//take all the code that pulls state out of the HttpServletRequest, and move it here
public class TransactionDataHttpOptions implements TransactionDataOptions {
    private HttpServletRequest request;

    //getter implementations that pull the required information out of the request

    public TransactionDataHttpOptions(HttpServletRequest request) {
        this.request = request;
    }
}

//now you can also do this, and use TransactionData even without a HttpServletRequest
public class TransactionDataMapOptions implements TransactionDataOptions {
    private Map<String, Object> map;

    //getter implementations that pull the required information out of the map

    public TransactionDataHttpOptions(Map<String, Object> map) {
        this.map = map;
    }
}

If you go this route, then TransactionDataHttpOptions is the only object with a dependency on HttpServletRequest. And since it is basically a wrapper that is intended to work with a HttpServletRequest I think that should be fine.

Collectives™ on Stack Overflow

Java: pass parameter in constructor or method?

5 Answers 5

1 Comment

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

1 Comment

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related