64

Which is generally accepted practice between these two cases:

function insertIntoDatabase(Account account, Otherthing thing) {
    database.insertMethod(account.getId(), thing.getId(), thing.getSomeValue());
}

or

function insertIntoDatabase(long accountId, long thingId, double someValue) {
    database.insertMethod(accountId, thingId, someValue);
}

In other words is it generally better to pass entire objects around or just the fields you need?

AJJ
  • 2,938
  • 4
  • 14
  • 14
  • 5
    It would entirely depend on what the function is for and how it relates (or doesn't relate) to the object in question. – MetaFight May 24 '16 at 19:19
  • That's the problem. I can't tell when I'd use one or the other. I feel like I could always change the code to accommodate either approach. – AJJ May 24 '16 at 19:20
  • 2
    In API terms (and not looking at the implementations at all), the former is abstract and domain oriented (which is good), whereas the latter is not (which is bad). – Erik Eidt May 24 '16 at 20:39
  • 1
    The first approach would be more 3-tier OO. But it should be even more so by eliminating the word database from the method. It should be "Store" or "Persist" and do either Account or Thing (not both). As a client of this layer you should not be aware of the storage medium. When retrieving an Account you would need to pass in the id though or a combination of property values (not field values) to identify the desired object. Or/and implenent an enumeration method that passes all accounts. – Martin Maat May 24 '16 at 22:20
  • 1
    Typically, both would be wrong (or, rather, less than optimal). *How* an object should be serialized into the database should be a property (a member function) of the object, because it typically directly depends on the member variables of the object. In case you change members of the object, you will also need to change the serialization method. That works better if it is part of the object – tofro Mar 14 '17 at 17:24
  • There is actually a refactoring that takes you from the many fields to the object. It's called [introduce parameter object](https://refactoring.com/catalog/introduceParameterObject.html). How badly needed it is depends on the number of fields and if your language supports named parameters. – candied_orange Mar 16 '17 at 01:04

10 Answers10

41

Neither is generally better than the other. It's a judgment call you have to make on a case-by-case basis.

But in practice, when you're in a position that you can actually make this decision, it's because you get to decide which layer in the overall program architecture should be breaking the object up into primitives, so you should be thinking about the whole call stack, not just this one method you're currently in. Presumably the breaking up has to be done somewhere, and it wouldn't make sense (or it'd be needlessly error-prone) to do it more than once. The question is where that one place should be.

The easiest way to make this decision is to think about what code should or should not have to be altered if the object gets changed. Let's expand your example slightly:

function addWidgetButtonClicked(clickEvent) {
    // get form data
    // get user's account
    insertIntoDatabase(account, data);
}
function insertIntoDatabase(Account account, Otherthing data) {
    // open database connection
    // check data doesn't already exist
    database.insertMethod(account.getId(), data.getId(), data.getSomeValue());
}

vs

function addWidgetButtonClicked(clickEvent) {
    // get form data
    // get user's account
    insertIntoDatabase(account.getId(), data.getId(), data.getSomeValue());
}
function insertIntoDatabase(long accountId, long dataId, double someValue) {
    // open database connection
    // check data doesn't already exist
    database.insertMethod(accountId, dataId, someValue);
}

In the first version, the UI code is blindly passing the data object and it's up to the database code to extract the useful fields from it. In the second version, the UI code is breaking up the data object into its useful fields, and the database code receives them directly without knowing where they came from. The key implication is that, if the structure of the data object were to change in some way, the first version would require only the database code to change, while the second version would require only the UI code to change. Which of those two is correct depends largely on what kind of data the data object contains, but it's usually very obvious. For example, if data is a user-provided string like "20/05/1999", it should be up to the UI code to convert that to a proper Date type before passing it on.

Ixrec
  • 27,621
  • 15
  • 80
  • 87
26

This isn't an exhaustive list, but consider some of the following factors when deciding whether an object should be passed to a method as an argument:

Is the object immutable? Is the function 'pure'?

Side-effects are an important consideration for maintainability of your code. When you see code with a lot of mutable stateful objects being passed around all over the place, that code is often less intuitive (in the same way that global state variables can often be less intuitive), and debugging often becomes more difficult and time-consuming.

As a rule of thumb, aim to ensure, as far as reasonably possible, that any objects you pass to a method are clearly immutable.

Avoid (again, as far as is reasonably possible) any design whereby the state of an argument is expected to be changed as a result of a function call - one of the strongest arguments for this approach is the Principle of Least Astonishment; i.e. somebody reading your code and seeing an argument passed into a function is 'less likely' to expect its state to change after the function has returned.

How many arguments does the method already have?

Methods with excessively long argument lists (even if most of those arguments have 'default' values) start to look like a code smell. Sometimes such functions are necessary, however, and you might consider creating a class whose sole purpose is to act like a Parameter Object.

This approach can involve a small amount of additional boilerplate code mapping from your 'source' object to your parameter object, but that's quite a low cost both in terms of performance and complexity however, and there are a number of benefits in terms of decoupling and object immutability.

Does the passed object belong exclusively to a "layer" within your application (for example, a ViewModel, or an ORM Entity?)

Think about Separation of Concerns (SoC). Sometimes asking yourself whether the object "belongs" to the same layer or module in which your method exists (e.g. a hand-rolled API wrapper library, or your core Business Logic Layer, etc.) can inform whether that object really should be passed to that method.

SoC is a good foundation for writing clean, loosely-coupled, modular code. for example, an ORM entity object (mapping between your code and your Database schema) ideally shouldn't be passed around in your business layer, or worse in your presentation/UI layer.

In the case of passing data between 'layers', having plain-data parameters passed into a method is usually preferable over passing in an object from the 'wrong' layer. Although it's probably a good idea to have separate models which exist at the 'right' layer that you can map onto instead.

Is the function itself just too big and/or complex?

When a function needs a lot of data items, it might be worth considering whether that function is taking on too many responsibilities; look for potential opportunities to refactor using smaller objects and shorter, simpler functions.

Should the function be a command/query object?

In some cases the relationship between the data and the function may be close; in those cases consider whether a Command Object or a Query Object would be appropriate.

Does adding an object parameter to a method force the containing class to adopt new dependencies?

Sometimes the strongest argument for "Plain old data" arguments is simply that the receiving class is already neatly self-contained, and adding an object parameter to one of its methods would pollute the class (or if the class is already polluted, then it will make the existing entropy worse)

Do you really need to pass around a complete object or do you only need a small part of that object's interface?

Consider the Interface Segregation Principle with respect to your functions - i.e. when passing in an object, it should only depend upon parts of that argument's interface which it (the function) actually needs.

Ben Cottrell
  • 11,656
  • 4
  • 30
  • 41
8

So when you create a function, you're implicitly declaring some contract with code that is calling it. "This function takes this info, and turns it into this other thing (possibly with side effects)".

So, should your contract logically be with the objects (however they're implemented), or with the fields that just so happen to be part of these other objects. You're adding coupling either way, but as the programmer, it's up to you to decide where it belongs.

In general, if it's unclear, then favor the smallest data necessary for the function to work. That often means passing in just the fields, since the function doesn't need the other stuff found in the objects. But sometimes taking the whole object is more correct since it results in less impact when things inevitably change in the future.

Telastyn
  • 108,850
  • 29
  • 239
  • 365
  • Why don't you change method name to *insertAccountIntoDatabase* or are you going to pass any other type?. At certain number of arguments to use obj does easy to read the code. In your case I would rather think if method name does clear what I'm going to insert instead of how im going to do it. – Laiv May 24 '16 at 19:43
3

It depends.

To elaborate, the parameters your method accepts should semantically match what you're trying to do. Consider an EmailInviter and these three possible implementations of an invite method:

void invite(String emailAddressString) {
  invite(EmailAddress.parse(emailAddressString));
}
void invite(EmailAddress emailAddress) {
  ...
}
void invite(User user) {
  invite(user.getEmailAddress());
}

Passing in a String where you should pass in an EmailAddress is flawed because not all strings are email addresses. The EmailAddress class better semantically matches the method's behavior. However passing in a User is also flawed because why on earth should an EmailInviter be limited to inviting users? What about businesses? What if you're reading email addresses from a file or a command line and they're not associated with users? Mailing lists? The list goes on.

There are a few warning signs you can use for guidance here. If you're using a simple value type like String or int but not all strings or ints are valid or there's something "special" about them, you should be using a more meaningful type. If you're using an object and the only thing you do is call a getter, then you should be passing the object in the getter in directly instead. These guidelines are neither hard nor fast, but few guidelines are.

Jack
  • 4,449
  • 2
  • 26
  • 30
2

Both the approaches have their own pros and cons. What is better in a scenario depends a lot on the use-case at hand.


Pro Multiple params, Con Object reference:

  • Caller not bound to a specific class, it can pass values from different sources altogether
  • Object state is safe from being modified unexpectedly inside the method execution.

Pro Object reference:

  • Clear interfacing that the method is bound to Object reference type, making it difficult to accidentally pass unrelated / invalid values
  • Renaming a field/getter requires changes at all invocations of the method and not just in its implementation
  • If a new property is added and needs to be passed, no changes required in method signature
  • Method can mutate object state
  • Passing too many variables of similar primitive types makes it confusing for the caller regarding the order (Builder pattern problem)

So, what needs to be used and when depends a lot on the use-cases

  1. Pass individual parameters : In general, if the method has nothing to do with the object type it is better to pass individual parameter list so that it is applicable to a wider audience.
  2. Introduce new model object : if the list of parameter grows to be large(more than 3), it's better to introduce a new model object belonging to the called API (builder pattern preferred)
  3. Pass Object Reference : If the method is related to the domain objects, then its better from maintainability and readability point of view to pass the object references.
Rahul
  • 121
  • 5
0

Clean Code recommends having as few arguments as possible, which means Object would usually be the better approach and I think it makes some sense. because

insertIntoDatabase(new Account(id) , new Otherthing(id, "Value"));

is a more readable call than

insertIntoDatabase(myAccount.getId(), myOtherthing.getId(), myOtherthing.getValue() );
someguy
  • 9
  • 1
  • 1
    can't agree there. The two aren't synonymous. Creating 2 new object instances just to pass them to a method is not good. I'd use insertIntoDatabase(myAccount, myOtherthing) instead of either of your options. – jwenting Mar 14 '17 at 13:11
  • When you make the call, you have two different ids and one value. That's three arguments. Stashing them into separate objects doesn't help. If it helped, you'd create an "insertIntoDatabase" object, containing two ids and a value. At that point trying to reduce argument count becomes absurd. – gnasher729 Jan 03 '20 at 17:37
  • To be comparable, your second example should read `insertIntoDatabase(id, id, "Value")`. – Magnus Jun 28 '20 at 07:58
0

Pass around the object, not its constituent state. This supports the object-oriented principles of encapsulation and data hiding. Exposing an object's innards in various method interfaces where it is not necessary violates core OOP principles.

What happens if you change the fields in Otherthing? Maybe you change a type, add a field, or remove a field. Now all methods like the one you mention in your question need to be updated. If you pass around the object, there are no interface changes.

The only time you should write a method accepting fields on an object is when writing a method to retrieve the object:

public User getUser(String primaryKey) {
  return ...;
}

At the point of making that call, the calling code does not yet have a reference to the object because the point of calling that method is to get the object.

  • 1
    "What happens if you change the fields in `Otherthing`?" (1) That would be a violation of the open/closed principle. (2) even if you pass in the whole object, code within that then accesses the members of that object (and if it doesn't, why pass the object in?) would still break... – David Arno May 25 '16 at 09:20
  • @DavidArno the point of my answer is not that _nothing_ would break, but _less_ would break. Don't forget the first paragraph, either: regardless of what breaks, an object's internal state should be abstracted using the object's interface. Passing around its internal state is a violation of OOP principles. –  May 25 '16 at 10:05
  • @DavidArno Fact of life: Principles get broken. – gnasher729 Jan 03 '20 at 17:39
0

From a maintainability perspective, arguments should be clearly distinguishable from each other preferably at the compiler level.

// this has exactly one way to call it
insertIntoDatabase(Account ..., Otherthing ...)

// the parameter order can be confused in practice
insertIntoDatabase(long ..., long ...)

The first design leads to early detection of bugs. The second design can lead to subtle runtime issues that do not show up in testing. Therefore the first design is to be preferred.

Joeri Sebrechts
  • 12,922
  • 3
  • 29
  • 39
0

Of the two, my preference is the first method:

function insertIntoDatabase(Account account, Otherthing thing) { database.insertMethod(account.getId(), thing.getId(), thing.getSomeValue()); }

The reason being that changes made to either object down the road, as long as the changes preserve those getters so the change is transparent outside of the object, then you have less code to change and test and less chance of disrupting the app.

This is just my thought process, mostly based on how I like to work and structure things of this nature and which prove to be quite manageable and maintainable in the long run.

I am not going to get into naming conventions but would point out that although this method has the word "database" in it, that storage mechanism can change down the road. From the code shown, there is nothing tying the function to the database storage platform being used -- or even if it is a database. We just assumed because it is in the name. Again, assuming those getters are always preserved, changing how/where these objects are stored will be easy.

I would re-think the function and the two objects though because you have a function that has a dependency on two object structures, and specifically the getters being employed. It also looks like this function is tying those two objects into one cumulative thing which gets persisted. My gut is telling me that a third object might make sense. I'd need to know more about these objects and how they relate in actuality and the anticipated roadmap. But my gut is leaning in that direction.

As the code stands now, the question begs "Where would or should this function be?" Is it part of Account, or OtherThing? Where does it go?

I guess there is a third object "database" already, and I am leaning towards putting this function into that object, and then it becomes that objects job to be able to handle an Account and an OtherThing, transform, and then also persist the result.

If you were to go as far as make that 3rd object conform to an object-relational mapping (ORM) pattern, all the better. That would make it very obvious to anyone working with the code to understand "Ah, this is where Account and OtherThing get smashed together and persisted".

But it could also make sense to introduce a forth object, which handles the job of combining and transforming an Account and an OtherThing, but not handle the mechanics of persisting. I'd do that if you anticipate a lot more interactions with or between these two objects, because at that grows I would want the persistence bits factored out into an object that manages the persistence only.

I would shoot for keeping the design such that any one of the Account, OtherThing, or the third ORM object can be changed without having to also change the other three. Unless there is a good reason not to, I'd want Account and OtherThing to be independent and not have to know the inner workings and structures of each other.

Of course, if I knew the full context that this is going to be, I might change my ideas completely. Again, this is just how I think when I see things like this, and how a lean.

Thomas Carlisle
  • 1,293
  • 9
  • 11
0

On one side you have an Account and an Otherthing object. On the other side, you have the ability to insert a value into a database, given the id of an account and the id of an Otherthing. That's the two given things.

You can write a method taking Account and Otherthing as arguments. On the pro side, the caller doesn't need to know any details about Account and Otherthing. On the negative side, the callee needs to know about methods of Account and Otherthing. And also, there is no way to insert anything else into a database than then value of an Otherthing object and no way to use this method if you have the id of an account object, but not the object itself.

Or you can write a method taking two ids and a value as arguments. On the negative side, the caller needs to know the details of Account and Otherthing. And there may be a situation where you actually need more details of an Account or Otherthing than just the id to insert into the database, in which case this solution is totally useless. On the other hand, hopefully no knowledge of Account and Otherthing is needed in the callee, and there is more flexibility.

Your judgement call: Is more flexibility needed? This is often not a matter of one call, but would be consistent through all your software: Either you use ids of Account's most of the time, or you use the objects. Mixing it gets you the worst of both worlds.

In C++, you can have a method taking two ids plus value, and an inline method taking Account and Otherthing, so you have both ways with zero overhead.

gnasher729
  • 42,090
  • 4
  • 59
  • 119