Void, value, or other?

Question

Hi, all.

I'm working on a spaghetti monster (unfortunately not of the flying variety), and I've got a question about proper design.

I'm in the process of taking a gigantic static Java method that returns an object and splitting it into reusable (and readable) components. Right now, the method reads an XML document, and then appends summary and detailed information from the document to a "dataModule", and the dataModule is then returned from the method.

In breaking my code up into a getSummaryData and getDetailedData method, I noticed I'd done the following:

dataModule = getSummaryData(xmlDocument);
setDetailedData(xmlDocument, dataModule);

(Pass by reference, append detailed data to dataModule within method)

This mostly has to do with the fact that the detailed data requires business logic based on the summary data in order to be parsed properly, and the fact that changing the structure of the dataModule involves lots of changing the front end of the application.

Is this approach any better than:

dataModule = getSummaryData(xmlDocument);
dataModule = setDetailedData(xmlDocument, dataModule);

(Pass by reference, append detailed data to dataModule within method, return dataModule)

I can't share much more of the code without revealing "teh secretz", but is there a strong reason to go with one approach over the other? Or, am I just getting caught up in which shade of lipstick I'm putting on my pig, here?

Thanks,
IVR Avenger

Answer 1

I find your second approach, where you return the same object, more confusing - because it implies to the calling function that a different object might be returned. If you're modifying the object, your first solution looks fine to me.

Answer 2

One principle I'd use to answer your question is that you want as many things as possible to be final, so that you have less trouble reasoning about state. By that principle, you'd want to avoid a meaningless reassignment.

final DataModule dataModule = getSummaryData(xmlDocument);
setDetailedData(xmlDocument, dataModule);

But that's wrong too. Why should the summary and detailed data be separate steps? Will you ever do one without the other? If not, those steps should be private to the DataModule. Really, the data module should probably know how to construct itself from the xml data.

final DataModule dataModule = new DataModule(xmlDocument);

Answer 3

The (arguable) advantage of the second approach is that it permits method chaining.

Say you had, in addition to setDetailedData(), setMoreData(), and that both functions were written to return the object. You could then write:

dataModule = getSummaryData(xmlDocument);
dataModule = dataModule.setDetailedData(xmlDocument).setMoreData();

I don't think the example you've provided benefits much from a method chaining syntax, but there are examples where it can lead to truly beautiful, expressive code. It permits what Martin Fowler calls a Fluent Interface.