10

Our teams is having the following discussion:

Let's say we have the following two methods:

public Response Withdraw(int clubId, int terminalId,int cardId, string invoice, decimal amount);

public Response Withdraw(Club club, Terminal terminal,Card card, string invoice, decimal amount);

whats sent over-the-wire are just the ids.

one side says that the first method is correct, because we only have the ids of terminal and club, and it should be clear that we have nothing else, this is my approach.

the other side says that the second method is correct because it's more flexible.

We are familiar with object parameter idea, the other side also thinks that the object parameter should have the objects as properties.

Which is the correct approach?

Maybe there is a third even better approach?

|improve this question
  • What?........... – James Mar 3 '13 at 12:33
  • 1
    Context? Webservice? WCF? – CodesInChaos Mar 3 '13 at 12:42
  • 1
    @James - sorry I wrote this question kinda fast, can you tell me what is not understood so I can edit it? – Mithir Mar 3 '13 at 12:43
  • @CodesInChaos the methods are BL methods actually – Mithir Mar 3 '13 at 12:44
10

The answer is context dependent.

If the client is expected to have all those objects already available, I'd use the object parameters. Otherwise, their code will look more convoluted than it needs to be. (E.g. they will have calls like club.getId(), for example.)

If the client will only have the ids available easily, then maybe the second approach is better, as you might not want the client to have to assemble/load all those objects if you really only need the ids.

An option is to provide both methods, so the client can choose which one to use (given that this does not clutter your API)

In general the object parameters are more extensible, since if in the future you need another piece of data to do the work, you do not need to introduce another method that takes that extra info.

Lastly, your method signatures should not be dictated by the specifics of what the method does (in your case, what exactly goes over the wire). The API should abstractly make sense so if implementation changes, you are not screwed.

|improve this answer
  • 3
    +1 I'd only add one more point: for methods called remotely ("over the wire"?), passing objects can entail deeply serializing an extensive object tree. IDs are excellent substitutes for objects when you're concerned about the size of a remote-call payload. – Ross Patterson Mar 3 '13 at 13:54
  • 1
    In this case it looks like you're coupled to a set of abstractions so passing Id's isn't going to buy you more flexibility and will probably increase the likelihood you'll reverse parameters. The question is how tightly is the code in the method coupled with the abstractions I'm passing in? For example a method like "validateCreditCard(string card, string cvi)" probably should stay with primitives to avoid being tightly coupled with some kind of CreditCard object. – ipaul Mar 3 '13 at 17:12
  • I would meet in the middle and use an interface in the parameter list. Then your club can be a club, but a also a sauna in the future. – Pieter B Sep 12 '18 at 8:20
13

The first approach is indicative of Primitive Obsession. Because you are passing ints and strings around, it's very easy for the programmer to make a mistake (eg passing a clubId to the terminalId parameter). This will result in difficult to find bugs.

In the second example, it's impossible to pass a club when a terminal is expected - this would give you a compile time error.

Even so, I would still look at string invoice. Is an invoice really a string? What does amount mean? This is more likely a monetary value.

You mentioned in your question "whats sent over-the-wire are just the ids.". This is correct, but don't let this requirement muddy your domain.

The best explanation I've seen in favour of this approach was in rule 3 of Object Calisthenics:

An int on its own is just a scalar, so it has no meaning. When a method takes an int as a parameter, the method name needs to do all of the work of expressing the intent. If the same method takes an Hour as a parameter, it’s much easier to see what’s going on. Small objects like this can make programs more maintainable, since it isn’t possible to pass a Year to a method that takes an Hour parameter. With a primitive variable the compiler can’t help you write semantically correct programs. With an object, even a small one, you are giving both the compiler and the programmer additional info about what the value is and why it is being used.

|improve this answer
  • So the second approach is preffered? even if there is a club object with just the id property filled? – Mithir Mar 3 '13 at 12:46
  • This appears to be a random blog. There's no evidence to say what's preferred. Who cares really what's preferred? Do what works for you – James Mar 3 '13 at 12:48
  • @James There is no definitive answer to this question, especially since the OP has not given us very much context. Anyone who categorically claims that one approach is preferable to the other is doing the OP a disservice. It's not that black and white. – MattDavey Mar 3 '13 at 12:49
  • 1
    @James I merely pointed out that the first approach makes it very easy to introduce difficult to find bugs. I'm not saying primitive types are bad, but the purpose of primitive types is to build up meaningful domain types. Using them outside of this context is the very definition of the primitive obsession code smell. – MattDavey Mar 3 '13 at 12:55
  • 5
    @James: What MattDavey said is a well-established fact. He is not saying that native types are bad, what he is saying that this: someMethod(int, int, int, string, decimal) is a lot harder to understand and use by a client than someMethod(Club, Terminal, Card, String, decimal) – c_maker Mar 3 '13 at 12:56
2

There's no right answer to this one. Either option could be right for the job. Well nearly any way, the invoice argument raised a furrow on my brow, I don't have a clue what that is from reading the code.

If you send an id, then both systems need to be tightly coupled at to what that represents. ClubID is key in the clubs table. More to the point both caller and callee need to agree what the Clubs table is called and which database it's in. If you don't want to or can't impose that constraint, then you'd pass the object using some common description, native, serialized, xml, name = value whatever, an ini file:)

That as you identified will cost you "over the wire". Avoiding that by just sending the identifier costs you elsewhere. So which one hurts you the least, now (or may be later...) is the indicator of good vs bad.

|improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.