|
While reading theDailyWTF the other day a contributor mentioned a pattern that he thought was a WTF and that was requiring object in constructors over id values.
Sorry, my example was too specific for people
ie
<br />
public List<Bar> GetLineItemsForOrder(Order order);<br />
public static void DoSomeAction(Foo foo);<br />
vs
<br />
public List<LineItem> GetLineItemsByOrderId(Int64 orderId);<br />
public static void DoSomeActionById(Int64 fooId);<br />
Hopefully, my clarification will help focus people to addressing the specific question. But then, I have been here long enough to know; asking a question on this forum really never ends well.
To be honest I nearly universally prefer the former (requiring fully hydrated objects) In the 99% of cases where I write code, where absolute performance isn't critical, the first method is so much more reliable and easier for Junior developers to work with. (Less mistakes, more compiler errors, etc)
So, what say you? Remember, if you disagree you are wrong, also remember I do both depending on my mood so not only am I Intransigent but I am a hypocrite.
modified 27-Mar-14 9:45am.
|
|
|
|
|
But shouldn't the Order instance already have its line items attached?
How about Order.GetLineItems()?
And... the first one should be in a higher layer, the second one in a lower layer.
After your edit:
Now there isn't enough context.
Does DoSomeAction really need the whole Foo? Probably not, so why pass it?
DoSomeActionById may be more reusable.
I think I'd invoke "Do The Simplest Thing That Could Possibly Work" and choose the latter.
You'll never get very far if all you do is follow instructions.
modified 27-Mar-14 10:00am.
|
|
|
|
|
|
|
I'd go a step farther and suggest you should probably be able to iterate through order directly without goofy "get/set" artifacts.
|
|
|
|
|
It depends whether line items are part of the data model that are naturally under orders, or whether this is going to invoke a query which requires significant logic. In the latter case I'd write a service class with a method like GetLineItems(Order o) because data classes shouldn't have complex logic, and (addressing the other answer here) properties shouldn't involve significant processing.
|
|
|
|
|
If you have a fully constructed Order object, then I'd probably prefer to hand that over - as in your first example.
The method then has the option of what it wants to use to find the appropriate items - which may be a List contained within the Order object already rather than a DB access requiring the ID
And given that these are public methods, why expose what type an Id is if you don't have to? Makes it a lot harder to change later to a GUID for example.
Those who fail to learn history are doomed to repeat it. --- George Santayana (December 16, 1863 – September 26, 1952)
Those who fail to clear history are doomed to explain it. --- OriginalGriff (February 24, 1959 – ∞)
|
|
|
|
|
But Shirley, the items are part of the order. so:
List items = order.getItems();
items = new Order(id).getItems();
|
|
|
|
|
Depends which class the GetLineItemsForOrder method is part of.
If it's part of the Customer or Supplier class, then you would be fetching only those relevant to that class instance, and for a specific order.
If it was part of the Order class, then I'd expect it to be parameterless and use this
And I'd hope that it did call the Order class method to get the items!
Those who fail to learn history are doomed to repeat it. --- George Santayana (December 16, 1863 – September 26, 1952)
Those who fail to clear history are doomed to explain it. --- OriginalGriff (February 24, 1959 – ∞)
|
|
|
|
|
Sometimes, for brevity sake, when posting online I do not take the time to design fully complete examples that cover every edge case and condition. I do this in order to ask a simple question in as few lines as possible.
|
|
|
|
|
OriginalGriff wrote: why expose what type an Id is if you don't have to?
Excellent point.
You'll never get very far if all you do is follow instructions.
|
|
|
|
|
The second reveals too much about the underlying implementation.
|
|
|
|
|
Both method can be valid depending on the usage of the parameter inside the method. If it's hard to get the whole Foo object then will work only using id else I'll lean to use DoSomeAction(Foo foo) method.
|
|
|
|
|
Ennis Ray Lynch, Jr. wrote: where absolute performance isn't critical
Passing a pointer is not very expensive; you're not copying the object, just passing an integer that holds it's address in memory.
Ennis Ray Lynch, Jr. wrote: the first method is so much more reliable and easier for Junior developers to work with.
One does not pass a Form if one is merely changing the caption.
There is no discussion, as far as I can see.
Bastard Programmer from Hell 
If you can't read my code, try converting it here[^]
|
|
|
|
|
I would always write the first unless there is a very good reason not to. Sometimes it may not be an object type, it may be an interface, if you only want to deal with a particular facet of your model – sometimes model classes need to implement several interfaces if they cover an object that is used in a lot of places, but that's okay (although it should lead you to question whether your data model is the best).
The only time I'd write code with IDs is in low level database or external API access classes. Anywhere else I'll define an interface for the actions I want to perform on an object, and if necessary a GetXxx(int id) method which looks up enough of an object to populate the interface.
If we're talking about object mapping frameworks then you should be able to construct an object over a query and have it load things on demand at which point you might as well just use the domain objects directly; only the relevant parts will need to be populated.
|
|
|
|
|
You're passing an object by value? That's pretty disgusting.
|
|
|
|
|
0. You're wrong.
1. It depends.
The massive danger (especially to those, whom you call "junior developers" and the rest of us call "noobs") is that they then think they require a complete object, so when displaying (say) a list of Foo in a grid, rather than just bringing back from the Db the few columns required, gathered in a nice piece of minimalistic Sql using only the required joins, they end up bringing back the whole kit and caboodle, probably via some "it's OK I don't have to look at the 437 SQL statements it generates so I can pretend this is still OK" ORM.
And it all looks great on their test database with three customers and half a dozen orders, but once you put it in a real environment the client-pc needs 6Gb Ram just to hold the customer collection, and crashes when you try to open the order history because WCF can't handle the traffic.
So, if you already have an object, pass it to the method. If you don't, pass its Id.
Of course, if the method in question might make changes to the object passed, we're in a different territory - but again it all comes down to 'it depends'
|
|
|
|
|
Hi All,
I feel the need to whinge, late nights, poor food, and the classic "Have you got it ready yet?, he's needs it, you are the block in production". The "you are the block in production" makes me laugh, the block in production is due to the appalling state of the software (it appear to work as the error handling was turned off), the lack of documentation (having to pick through the C command parser), the unwillingness to give out how they have altered the test rig... Now I'm getting the blame! I don't count myself as a code ninja, but the skills I have (Serial Comms) I think I am reasonably good at (counting the reviews of an article I published here). I honestly need to sit down and talk to the Boss man he's going to go ballistic. 
|
|
|
|
|
glennPattonWork wrote: the Boss man he's going to go ballistic
![Badger | [badger,badger,badger,badger...]](https://codeproject.global.ssl.fastly.net/script/Forums/Images/badger.gif) <-- boss
Try and record it if it happens, could be quite humorous! 
|
|
|
|
|
Trust Me it wont!, for me
|
|
|
|
|
Having been the recieving end on occasion, I can sympathize. Usually afterwards though, reflecting on some of the crap that was said during the tirade, one usually finds some amusing bits. Good luck in any case.
|
|
|
|
|
Well just had a start of the "it can't be done" conversation with the Boss, his reply was well the <redacted! for="" my="" own="" good=""> needs to see something quickly or he won't believe we have been working on it! To which my reply was I have good solid comms to the DUT.
|
|
|
|
|
glennPattonWork wrote: he won't believe we have been working on it!
see? a classic line already! Next will be the "Well, maybe I'll get someone in here who can!" idle threat. Are there others on your team, could you approch as a united front?
|
|
|
|
|
No Team, just myself (and others that live in my head), I did say I didn't want to do it...it was a heap a mass of VB code that seem to work after caressing the ini file, praying to the correct deity, sacrifice. I have had to do other things at the same time... no support when it has been asked for...it's there <<expletive deleted="">> fault. 
|
|
|
|
|
:shakes head:
Sounds like a lose - lose situation, hope it improves.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
