October 3, 2015 at 11:39 pm
Comments posted to this topic are about the item What's a Code Smell?
October 4, 2015 at 7:00 am
I think that what some people consider to be a "code smell" is sometimes actually an indication that the person that wrote the code has a very good handle on the art of writing code for SQL Server.
For example, there are a lot of people that consider it to be a code smell to use DATEPART abbreviations. If someone consistently uses the 2 character abbreviations (mcs and isowk not withstanding), then I look at it as someone that actually knows what they're doing and hates screen clutter and ragged formatting, as well.
Another supposed code smell is when people simply subtract one date from another to get a duration. Knowing that writing truly portable code that can actually do much worthwhile is a pipedream at best, I'd much rather see someone do such a subtraction rather than go through all the gyrations that others may go through only to come up with the wrong answer or a formula that has a limited range (millisecond conversions seem to be the favorite, there).
As a bit of a sidebar, I would have much preferred for Microsoft to have used the much more appropriate and correct "us" abbreviated for microseconds instead of using the code smell of "mcs" and ISO Week should have been abbreviated "IW", which would have followed the 2 character convention used up to that point and made the pronunciation of the abbreviation the very appropriate "ew" sound. 😛
--Jeff Moden
Change is inevitable... Change for the better is not.
October 5, 2015 at 12:40 am
To me the "code smell" I really don't agree with is the names ( 65 -> 69) . I will use square brackets around a name with spaces and with special characters if I need to make the code more legible. I been in the industry long enough to have run into lengths limits on object/column names and that sucked when you had to have a dictionary to translate an abbreviated name into something meaningful.
If the sproc results are aimed at a report then I will include the unit of measure to avoid confusion. ie TransitSpeed -> [Transit Speed (mph)] or [Transit Speed (m/s)]. PreferredRouteDistance -> [Preferred Route (Km)] .
Extra cost on compile time is zero, time saved on wrong assumptions or bizarre bugs hours/days.
October 5, 2015 at 2:23 am
@[Yet another DBA]
That's true, we all use whatever we think will make the clearest column heading when we are generating reports. These aren't rules, just indications, but I think perhaps we should make a distinction between naming conventions for reports and those for relational tables
Best wishes,
Phil Factor
October 5, 2015 at 3:44 am
@[Phil Factor]
You are absolutely right as long as reports will be assembled by skilled people using their own naming convention rules. However I've learned over the years that this is quite often not the case. Reports are frequently thrown together by people who barely know the difference between an inner and an outer join. Don't expect them to follow some kind of rigorous naming convention for column titles or even know about the appropriate units of the measures presented in their reports. Maybe we're not used to it (yet) in SQL Server but when the frameworks accessing the database allow these 'decorated' names in my opinion the benefits might outweigh the merrits. As usual, it depends ...
October 5, 2015 at 10:59 am
For what it's worth, I'm pretty sure these were compiled into a GitHub repo here: https://github.com/red-gate/SQL-code-smells
And for those using SSDT, Dave Bally wrote an add-in here:
https://github.com/davebally/TSQL-Smells
(Note that the add-in could use some tweaking to get it working w/ other versions of SSDT:
dataidol.com/davebally/2015/03/23/getting-out-of-the-ground-with-tsql-smells/
October 5, 2015 at 12:15 pm
The thing about code smells is that they tend to be working code that someone was praised for delivering on time and to budget.
Identifying the code smell is a thankless task as the one who smelt it dealt it!
It's worked up until now are you saying you're not talented enough to maintain it? The other guy managed just fine!
The fact that a ticking bomb has finally gone boom somehow gets lost in translation
October 5, 2015 at 12:29 pm
I guess "code smells" and "technical debt" are somewhat related.
"Do not seek to follow in the footsteps of the wise. Instead, seek what they sought." - Matsuo Basho
October 5, 2015 at 1:21 pm
As a bit of a sidebar, I would have much preferred for Microsoft to have used the much more appropriate and correct "us" abbreviated for microseconds instead of using the code smell of "mcs" and ISO Week should have been abbreviated "IW", which would have followed the 2 character convention used up to that point and made the pronunciation of the abbreviation the very appropriate "ew" sound. 😛
Nice.
October 6, 2015 at 12:16 am
I hate to tell you (well maybe not 🙂 ) Us 'old' programmers have talked about how code smells, and giving code the 'sniff test' for a lot longer.For me, I only go back to the early 80's but I heard it from programmers that came before me. But I'll give them 'CodeSmell' (we didn't drop the space). Of course, asking how code smells or what the sniff test says is subjective - but I've been doing it and calling it that for more than 30 years and it is still a very valuable tool. (Got to use ALL our senses!) 😉
October 6, 2015 at 1:56 am
So much of what we talk about today has existed in some form for decades.
Fred P Brooks wrote "The mythical man month" in 1975 after 25 years in the industry. In that book he and a colleague were wrestling with development concepts that we would recognise today as agile.
Code smell started off as Developer humour and is now an industry accepted term.
A friend took one look at a canonical data model of dubious provenance and coined 2 alternative names.
1. The comical data model. A model that surely must be a joke
2. The colonical data model that you hoped was a joke but has been signed off at a Senior level
October 6, 2015 at 7:15 am
If I say "who wrote this crap"...it's a code smell.
Of course, I'm often the one who wrote it, but that's beside the point. 😀
October 14, 2015 at 12:56 pm
MadmanPierre (10/6/2015)
I hate to tell you (well maybe not 🙂 ) Us 'old' programmers have talked about how code smells, and giving code the 'sniff test' for a lot longer.For me, I only go back to the early 80's but I heard it from programmers that came before me. But I'll give them 'CodeSmell' (we didn't drop the space). Of course, asking how code smells or what the sniff test says is subjective - but I've been doing it and calling it that for more than 30 years and it is still a very valuable tool. (Got to use ALL our senses!) 😉
I can't recall exactly when I first came across the term, but I think it was closer to 50 years ago than 30 - my best guess is 43 years ago.
Tom
October 14, 2015 at 5:45 pm
Nice little e-book, I think I'll find it useful.
But it's a bit sloppy in places.
For example item 8 is badly worded and the headline is exremely badly worded: what smells isn't what the headline says (using a constraint to limit values in a column), it's using a CHECK constraing to enumerate the permitted values in the column. The wording in the body of the item suggests that a foreign key constraint isn't a constraint, which is just plain wrong. The idea of using a foreign key for limits as opposed to to reference and enumeration, which is what the first sentence appears to say, is just plain nonsense. If I want to limit the values in MyColumn to strings of between 6 and 24 characters I am not going to set up a table with a large number (about 1 followed by 58 0s) of rows in it to enumerate all possible values, I'm going top use a check constraint to state the lower length limit and the type to handle the upper limit, like this:
MyColumn varchar(24) not null check(len(Mycolumn)>5)
and anyone who would even consider using a foreign key for a straightforwards limit constraint like that is just being silly. Of course anyone who would consider using a CHECK constraint to hld an enumeration instead of holding it in a table is also being silly, but the headline and the body should have been worded so that it was clear that the topic was representing enumerations, not stating limits.
Then in article 27 it's a pity that the ambiguous word "duration" was used instead of something non-ambiguous like "interval". The most common use of duration, in my experience, is to refer to a length or quantity of time (or of a sound, or of ....) and one can talk about things like "the average duration of life in the world war I trench warfare"; an interval isn't a quantity or a length (although of course is has a prperty - its duratipn - that is) and it doesn't have an average; but the proposed solution assumes that "duration" always means "interval". This isn't an American vs British version of English thing, the MacMillan dictionary gives identical defintions for "duration" for British English and for American English, with the emphasis on length. So for what "duration" normally means the correct representation is an exact numeric which gives the length in some unit , with the unit clearly documented and preferable indicated in the column name (units can be second, days, hours, weeks, picoseconds, millenia, or whatever is appropriate).
2 faulty items out of 27 is actually pretty good, most technical books about databases have rather more things wrong, if it keeps the sillies down to that level all the way through it will be a very good book.
Tom
Viewing 15 posts - 1 through 15 (of 35 total)
You must be logged in to reply to this topic. Login to reply