views:

274

answers:

12

I often see two conflicting strategies for method interfaces, loosely summarized as follows:

// Form 1: Pass in an object.
double calculateTaxesOwed(TaxForm f) { ... }

// Form 2: Pass in the fields you'll use.
double calculateTaxesOwed(double taxRate, double income) { ... }

// use of form 1:
TaxForm f = ...
double payment = calculateTaxesOwed(f);

// use of form 2:
TaxForm f = ...
double payment = calculateTaxesOwed(f.getTaxRate(), f.getIncome());

I've seen advocates for the second form, particularly in dynamic languages where it may be harder to evaluate what fields are being used.

However, I much prefer the first form: it's shorter, there is less room for error, and if the definition of the object changes later you won't necessarily need to update method signatures, perhaps just change how you work with the object inside the method.

Is there a compelling general case for either form? Are there clear examples of when you should use the second form over the first? Are there SOLID or other OOP principles I can point to to justify my decision to use one form over the other? Do any of the above answers change if you're using a dynamic language?

+3  A: 

It depends on the intention of your method.

If the method is designed to work specifically with that object and only that object, pass the object. It makes for a nice encapsulation.

But, if the method is more general purpose, you will probably want to pass the parameters individually. That way, the method is more likely to be reused when the information is coming from another source (i.e. different types of objects or other derived data).

Robert Cartaino
+1  A: 

I don't think it really matters. You open yourself to side effects if you pass in the Object as it might be mutated. This might however be what you want. To mitigate this (and to aid testing) you are probably better passing the interface rather than the concrete type. The benefit is that you don't need to change the method signature if you want to access another field of the Object.

Passing all the parameters makes it clearer what the type needs, and might make it easier to test (though if you use the interface this is less of a benefit). But you will have more refactoring.

Judge each situation on its merits and pick the least painful.

Rich Seller
A: 

One advantage of the first form is

  1. Abstraction - programming to an interface rather than implementation. It makes the maintainance of your code easier in the long run becuase you may change the implementation of TaxForm without affecting the client code as long as the interface of TaxForm does not change.
Vincent Ramdhanie
+2  A: 

I strongly recommend the second solution - calculateTaxesOwed() calculates some data, hence needs some numerical input. The method has absolutly nothing to do with the user interface and should in turn not consum a form as input, because you want your business logic separated from your user interface.

The method performing the calculation should (usualy) not even belong to the same modul as the user interface. In this case you get a circular dependency because the user interface requires the business logic and the business logic requires the user interface form - a very strong indication that something is wrong (but could be still solved using interface based programming).

UPDATE

If the tax form is not a user interface form, things change a bit. In this case I suggest to expose the value using a instance method GetOwedTaxes() or instance property OwedTaxes of the TaxForm class but I would not use a static method. If the calculation can be reused elsewhere, one could still create a static helper method consuming the values, not the form, and call this helper method from within the instance method or property.

Daniel Brückner
I'm reading "tax form" here as "Form 1040", not "a user interface screen used for data entry".
Dave Sherohman
(Oh, and for non-US readers, the 1040 is the primary form used for submitting personal tax returns in the US. So I'm assuming the "TaxForm" is a class encapsulating the information and procedures related to a specific tax document.)
Dave Sherohman
This changes things ...
Daniel Brückner
Sorry for the confusion. Yes, by "form" I meant "a set of data and behavior representing a self-calculating income tax submission".
Kyle Kaitan
+6  A: 

In all honesty it depends on the method in question.

If the method makes sense without the object, then the second form is easier to re-use and removes a coupling between the two classes.

If the method relies on the object then fair enough pass the object.

There is probably a good argument for a third form where you pass an interface designed to work with that method. Gives you the clarity of the first form with the flexibility of the second.

Andrew Barrett
+1  A: 

Passing just the arguments can be easier to unit test, as you don't need to mock up entire objects full of data just to test functionality that is essentially just static calculation. If there are just two fields being used, of the object's many, I'd lean towards just passing those fields, all else being equal.

That said, when you end up with six, seven or more fields, it's time to consider passing either the whole object or a subset of the fields in a "payload" class (or struct/dictionary, depending on the language's style). Long method signatures are usually confusing.

The other option is to make it a class method, so you don't have to pass anything. It's less convenient to test, but worth considering when your method is only ever used on a TaxForm object's data.

ojrac
A: 

This is the same as the "Introduce Parameter Object" from Martin Fowler's book on refactoring. Fowler suggests that you perform this refactoring if there are a group of parameters that tend to be passed together.

trilobite
A: 

If you believe in the Law of Demeter, then you would favor passing exactly what is needed:

http://en.wikipedia.org/wiki/Law_of_Demeter

http://www.c2.com/cgi/wiki?LawOfDemeter

Adam Goode
A: 

Separation of UI and Data to be manipulated

In your case, you are missing an intermediate class, say, TaxInfo, representing the entity to be taxed. The reason is that UI (the form) and business logic (how tax rate is calculated) are on two different "change tracks", one changes with presentation technology ("the web", "The web 2.0", "WPF", ...), the other changes with legalese. Define a clear interface between them.


General discussion, using an example:

Consider a function to create a bitmap for a business card. Is the purpose of the function

(1) // Formats a business card title from first name and last name

OR

(2) // Formats a businnes card title from a Person record

The first option is more generic, with a weaker coupling, which is generally preferrable. However, In many cases less robust against change requests - e.g. consider "case 2017: add persons Initial to business card".

Changing the implementation (adding person.Initial) is usually easier and faster than changing the interface.

The choice is ultimately what type of changes you expect: is it more likely that more information from a Personrecord is required, or is it more likely that you want to create business card titles for other data structures than Person?

If that is "undecided", anfd you can't opf for purpose (1) or (2) I'd rather go with (2), for syntactic cleanliness.

peterchen
+1  A: 

I realize that this is largely an artifact of the example used and so it may not apply in many real-world cases, but, if the function is tied so strongly to a specific class, then shouldn't it be:

double payment = f.calculateTaxesOwed;

It seems more appropriate to me that a tax document would carry the responsibility itself for calculating the relevant taxes rather than having that responsibility fall onto a utility function, particularly given that different tax forms tend to use different tax tables or calculation methods.

Dave Sherohman
A: 

If I was made to choose one of the two, I'd always go with the second one - what if you find that you (for whatever reason) need to caculate the taxes owed, but you dont have an instance of TaxForm?

This is a fairly trivial example, however I've seen cases where a method doing a relatively simple task had complex inputs which were difficult to create, making the method far more difficult to use than it should have been. (The author simply hadn't considered that other people might want to use that method!)

Personally, to make the code more readable, I would probbaly have both:

double calculateTaxesOwed(TaxForm f) 
{
    return calculateTaxesOwed(f.getTaxRate(), f.getIncome());
}

double calculateTaxesOwed(double taxRate, double income) { ... }

My rule of thumb is to wherever possible have a method that takes exactly the input it needs - its very easy to write wrapper methods.

Kragen
A: 

Personally, I'll go with #2 since it's much more clear of what it is that the method need. Passing the TaxForm (if it is what I think it is, like a Windows Form) is sort of smelly and make me cringe a little (>_<).

I'd use the first variation only if you are passing a DTO specific to the calculation, like IncomeTaxCalculationInfo object which will contain the TaxRate and Income and whatever else needed to calculate the final result in the method, but never something like a Windows / Web Form.

Jimmy Chandra