Reuse good -- Abstractions better!
I posted something about the word 'Upsert' recently -- and I was kind of surprised by the comments. My real feelings about the concept of coining a new term 'upsert' are simple: Don't do it!Upsert is a bad name. An evil name. And not because it is a silly portmanteau. Coining a term upsert, to embody the concept of "Update or Insert" is bad because it fundamentally misses the point of "Why We Code". Let's get back to basicsJust say you discover that your code is frequently peppered with this little snippet: If (this.State != New) { this.Update(); } else { this.Insert(); }
So -- you decide to extract that code snippet and place it in its own routine. Routines are nice, routines are great. "Aside from the invention of the computer, the routine is arguably the single greatest invention in computer science." Why You Should Use Routines, Routinely
This is what you create: private void Upsert() { If (this.State != New) { this.Update(); } else { this.Insert(); } }
Now -- any instance of those fives lines becomes instead: Upsert();
Great! You've achieved one of the primary goals of routine construction: code reuse. Your code is now shorter, and thus hopefully cheaper to maintain. It's more versatile because now if you change the way "update or insert" works you only need to change it in one place. Life is sweet. The birds sing and the flowers bloom once more. But there's something amiss. Something foul remains. You've failed on the other goal of routine construction -- you've failed to hide away the implementation details. You've failed to "raise the abstraction level!" The user/programmer still has to think about the inner concepts: update and insert. Every time they see that words "upsert" -- the details "Update and Insert" are staring them in the face. You lose sleep. Your dreams are troubled. Dark clouds follow you everywhere. And in quiet moments you hear the ominous echo of Alfred North Whitehead whispering: "Civilization advances by extending the number of important operations we can perform without thinking." --Alfred North Whitehead
Did you read that? What? You missed it -- here it is again: "Civilization advances by extending the number of important operations we can perform without thinking." --Alfred North Whitehead
By choosing the name "Upsert" we've failed in our duty to advance civilization! We want the user (the programmer who uses your routine) to perform these two important operations without thinking about both of them. Let's think of a better name -- a name that alleviates the consumer from having to think about the constituent parts of the routine they're using. How about this: Save. private void Save() { If (this.State != New) { this.Update(); } else { this.Insert(); } }
Isn't that nicer? And civilization has taken another tiny step forward.
Incidentally -- it seems this 'Alfred North Whitehead' was a functional Programmer in the 1800s -- before Church and the Lambda calculus that gave rise to functional programming. Here's a comment about him: "There are no fundamental "things," or "objects" in the world of Whitehead. Whitehead's ontology, or parts-list of the universe, contains only processes." --Richard Lubbock in 'Alfred North Whitehead: Philosopher for the Muddleheaded'
'Don2' on Wed, 04 Jun 2008 09:05:45 GMT, sez: ...And with those words Leon takes off his preaching hat and steps down from the pulpit.
;-)
'http://' on Wed, 04 Jun 2008 09:14:40 GMT, sez: i agree it is hard to keep the basics in mind when you are coding
'Michael Goldobin' on Wed, 04 Jun 2008 10:06:38 GMT, sez: "Save" is no good. It will keep your mind still. Something more obscure, like "Drumwand" will stimulate brain much better.
private void Drumwand() {
If (this.State != New) {
//It exists in db? Update and drum roll
this.Update();
} else {
//It's missing? Wave a wand and it will magically appear.
this.Insert();
}
}
Also this naming convention is "Security Through Obscurity" compliant.
'Nate' on Wed, 04 Jun 2008 13:25:58 GMT, sez: I think that the folks who helped created the 2003 SQL ANSI Standard agreed with you...which is why they called it a "MERGE" statement instead of "UPSERT". I'm guessing "Save" was a bit too ambiguous for them ;-)
'Ben' on Wed, 04 Jun 2008 16:35:28 GMT, sez: This post had me laughing out loud.
Thanks again....
'Eber Irigoyen' on Wed, 04 Jun 2008 18:02:30 GMT, sez: funny... I have a function with that name and almost that exact code deep on my code base for my current project
'John Walker' on Wed, 04 Jun 2008 18:10:16 GMT, sez: point well taken!
'OJ' on Wed, 04 Jun 2008 20:35:34 GMT, sez: I don't mind "Save", but for me it still doesn't capture the moment :)
My preference is "Persist". But I guess that's not perfect either :)
'DOm' on Wed, 04 Jun 2008 20:39:13 GMT, sez: Good post Leon.
'lb' on Wed, 04 Jun 2008 21:38:49 GMT, sez: @OJ --
i almost went with "persist" but didn't want to sound too much like an OR/M nutcase.
'Kyralessa' on Fri, 06 Jun 2008 18:49:42 GMT, sez: I like Persist() better.
And if you decide you don't want to save something after all: Desist()
And if you want something to Save right now instead of batching later on: Insist()
'Wittgenstein' on Sat, 07 Jun 2008 19:19:51 GMT, sez: "...Wittgenstein rejects a variety of ways of thinking about what the meaning of a word is, or how meanings can be identified. He shows how, in each case, the meaning of the word presupposes our ability to use it."
http://en.wikipedia.org/wiki/Philosophical_Investigations#Meaning_and_definition
'Okla' on Sat, 07 Jun 2008 19:24:17 GMT, sez: Why not spell it out? update_or_insert(), after all that's what it does :)
And as McConnell also suggests you can make your routine names as long as they need to be in order to make them understandable.
'lb' on Sun, 08 Jun 2008 08:35:53 GMT, sez: @Okla
personally i think "update_or_insert" is a poor choice of name for all of the reasons given in this article.
the arguments presented above are not specific to the contracted form, they apply equally to the longer form.
Say if we update the method to first check if anything had changed before updating, we wouldn't change the name to:
check_for_changes_then_update_or_insert
it's fine to have a long name for a method -- but i don't think that the name should simply be an echo of the implementation details.
'okla' on Mon, 09 Jun 2008 13:39:58 GMT, sez: @lb
I think if the name is getting too long as well then there is a problem - this probably shows the code may need breaking down further into different routine.
In your example the code that performs the check should go into its own routine and given a name that shows it carries out this test as it may be reusable somewhere else, one example is if you have a delete() fuction and you need to do the same check
The reason for naming a routine this way is to make the code self-documenting and promote good practice as maintainers will think through adding code into main flow of an existing code.
The larger the routine the less reusable and more complex it gets.
'Kevin H' on Mon, 09 Jun 2008 19:47:21 GMT, sez: "Save" is fine, but I like "Store" as well. Or occasionally "Keep".
'Samuta' on Tue, 10 Jun 2008 01:18:38 GMT, sez: I felt compelled to comment because the CAPTCHA says 'meatbag' and I cant pass up an opportunity to type 'meatbag'.
My 2 cents; Save(), Keep(), Store(), Persist() [really like Insist() clever], Drumwand()... so many names. Though I suppose if you want to be really really 'Security Through Obscurity' compliant all your procs should be named with UID's, but that would be very silly.
I do find it equally frustrating and amusing when you're able to functionalize some complex logic quicker and easier than you can name the f'ing thing. If only everything would lend itself to simple names, names like: Save, Load, Init, and other choice four letter words.
Granted that most of the time you can't name it chances are it ought to be broken up into a few smaller functions, but there are times that some complex chunk of business logic is going to take a pile of code and isn't going to be particularly reusable.
One of my more recent run on function names:
getNextNotificationDateByScheduleID
I bet you could all guess what the inputs and outputs are. That's about as long as I'm comfortable going in general.
Anyway, back to why I'm really here;
while(1){meatbag()}
'SteveJ' on Sat, 14 Jun 2008 10:18:36 GMT, sez: Just ran into this one today and immediately thought of this post. What do you use for a get/retrieve but create if doesn't exist? So Getsert() or Selsert() right? How about Finate()? (find_or_create)
Or I'll take a real phrase if one exists.
|