What's a Code Smell?

Question

What's a Code Smell?

Viewing 15 posts - 16 through 30 (of 35 total)

You must be logged in to reply to this topic. Login to reply

Gary Varga SSC Guru Points: 82166 More actions · Answer 1

I have always found the term very apt. If you consider opening a fridge and get a rather pungent smell then food may be off or you might just have some of the best cheese about.

Code smells are indicative of poor coding practices. Not evidence. It is always described, in my experience, as a suggestion to check the code and consider it carefully.

Of course there are some smells that only say foul!!!

Gaz

-- Stop your grinnin' and drop your linen...they're everywhere!!!

Phil Factor SSC-Insane Points: 20244 More actions · Answer 2

@TomThomson

Is this the SQL Code Smells ebook you're referring to? I wasn't sure, since there were 120 SQL Code Smells itemised, not 27. If it is mine, I'll get the changes in the next edition.

Best wishes,
Phil Factor

Jeff Moden SSC Guru Points: 1003895 More actions · Answer 3

Phil Factor (11/10/2015)
@TomThomson
Is this the SQL Code Smells ebook you're referring to? I wasn't sure, since there were 120 SQL Code Smells itemised, not 27. If it is mine, I'll get the changes in the next edition.

Next edition?

Before you do that and with the understanding that your are, in fact, limited by space requirements, we should get together and talk about some of the "smells" that are there.

The reason why I suggest that is because some seem to be a bit incomplete and, to me, some are a really "good smell" rather than a "bad code smell". Yes, I'm one of those that read the front matter and I agree that many of these are considered to be, shall we say, guidelines but a lot of people that may not know better may take them more as "best practices" and you more than anyone, ol' friend, should know what I think of many so called "best practices". I believe one of my earlier quotes on the subject was something to the nature of "Just because a million people agree on something, doesn't make it right." 🙂 And, yes, I also agree that many of the listed code smells deserve and article of their own.

Some of my favorite disagreements as to "code smells" (and I find some of the recommendations to replace them a bit more smelly than the original smell) are that of xp_CmdShell usage, the use of "dated tables" (the backbone of properly created partitioned views), use of EAV tables (they're actually properly used in many so-called "normalized tables" as are their near cousins, NVPs, and should almost never be replaced by XML), avoiding temporary tables on smaller data sets (the recompiles can be a blessing in disguise as can the skeletal reuse of temp tables), not using the FLOAT datatype for certain types of calculations, persisting a "Whole Date" value in a column, actual valid uses of SQL_Variant, writing a scalar function to trim whitespace is a code smell to me and so is always pre-initializing a variable, etc, etc, etc.

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

TomThomson SSC Guru Points: 104773 More actions · Answer 4

Phil Factor (11/10/2015)
@TomThomson
Is this the SQL Code Smells ebook you're referring to? I wasn't sure, since there were 120 SQL Code Smells itemised, not 27. If it is mine, I'll get the changes in the next edition.

Yes. The number 27 was the number of items I'd read when I posted the comment, not the number in the whole book.

Tom

Phil Factor SSC-Insane Points: 20244 More actions · Answer 5

@jeff Moden

It was a difficult booklet to write because I was merely trying to record those things that were considered by people cleverer and more experienced than I to be a code smell. It is really supposed to be ego-less. I'm guilty of using almost all these code smells, but I like to justify the fact that I'm doing them. The problem comes from using these techniques without understanding the constraints and reasons. I too use EAV, heaps and the like, but can argue and demonstrate why they are the best solution. I'm sure that a best-Practice booklet would be a good idea as well, but this is on a different subject; 'Code Smells' which are those practices that are perfectly OK to use if you are prepared to justify their use if challenged.

Best wishes,
Phil Factor

Jeff Moden SSC Guru Points: 1003895 More actions · Answer 6

Phil Factor (11/12/2015)
@Jeff Moden
It was a difficult booklet to write because I was merely trying to record those things that were considered by people cleverer and more experienced than I to be a code smell. It is really supposed to be ego-less. I'm guilty of using almost all these code smells, but I like to justify the fact that I'm doing them. The problem comes from using these techniques without understanding the constraints and reasons. I too use EAV, heaps and the like, but can argue and demonstrate why they are the best solution. I'm sure that a best-Practice booklet would be a good idea as well, but this is on a different subject; 'Code Smells' which are those practices that are perfectly OK to use if you are prepared to justify their use if challenged.

Absolutely no malice born towards you, ol' friend. I know exactly what you went through writing that because it's a whole lot like writing "coding standards" at work. To be honest, you did an outstanding job (just think of the number of points you touched on). Mostly, I'm volunteering to be a sounding board and, perhaps, a contributor to this very worthwhile eBook.

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

Robert Sterbal SSChampion Points: 11048 More actions · Answer 7

This topic would do well on a wiki. Has it already been posted somewhere or should I add it to my wiki?

412-977-3526 call/text

Gary Varga SSC Guru Points: 82166 More actions · Answer 8

robert.sterbal 56890 (6/29/2016)
This topic would do well on a wiki. Has it already been posted somewhere or should I add it to my wiki?

Go ahead Robert, please. And also post back here for all our benefits. Thanks!!!

Gaz

-- Stop your grinnin' and drop your linen...they're everywhere!!!

Robert Sterbal SSChampion Points: 11048 More actions · Answer 9

I reproduced the list here:

https://sqlserver.miraheze.org/wiki/Code_Smells

I hope to set up the individual pages, and put them in appropriate categories, it there is interest.

Anyone can add to or edit the pages.

Thanks!

412-977-3526 call/text

TomThomson SSC Guru Points: 104773 More actions · Answer 10

robert.sterbal 56890 (6/30/2016)
I reproduced the list here:
I hope to set up the individual pages, and put them in appropriate categories, it there is interest.
Anyone can add to or edit the pages.
Thanks!

Although overall it's good document, it has some faults.

Item 8 in the list is the sort of careless nonsense that is going to hopelessly mislead schema designers into getting it absolutely wrong if they believe it. Some of the headlines for other items are misleading. It would have been a good idea to go through the whole thing carefully before propagating it yet further.

Tom

Robert Sterbal SSChampion Points: 11048 More actions · Answer 11

The nice thing about a wiki is that changes can be made as needed and the history can be checked

412-977-3526 call/text

TomThomson SSC Guru Points: 104773 More actions · Answer 12

robert.sterbal 56890 (6/30/2016)
The nice thing about a wiki is that changes can be made as needed and the history can be checked

Yes, when it becomes a wiki problems can be fixed. Of course that risks edit wars unless editing permission is restricted, and restricting editing permission too much can risk faults not getting fixed, but a wiki is a good way to go.

Tom

Jeff Moden SSC Guru Points: 1003895 More actions · Answer 13

robert.sterbal 56890 (6/30/2016)
I reproduced the list here:
https://sqlserver.miraheze.org/wiki/Code_Smells
I hope to set up the individual pages, and put them in appropriate categories, it there is interest.
Anyone can add to or edit the pages.
Thanks!

Since it's copyrighted material, did you check with anyone first? Yeah, I see the credit you gave but that doesn't justify copying.

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

Robert Sterbal SSChampion Points: 11048 More actions · Answer 14

Actually the material is licensed under a Creative Commons Attribution-NonCommercial 4.0 International License https://creativecommons.org/licenses/by-nc/4.0/

412-977-3526 call/text

David.Poole SSC Guru Points: 75906 More actions · Answer 15

TomThomson (6/30/2016)
Item 8 in the list is the sort of careless nonsense that is going to hopelessly mislead schema designers into getting it absolutely wrong if they believe it. .

I wouldn't have put it quite like that but agree with the general thrust.

Many numeric entities have a natural range or some form of asymptote. Latitude/Longitude, minimum age, month of pregnancy.

Many string entities have a defined pattern. I used CHECK constraints to prevent people filling mandatory fields up with white space to circumvent a business rule.

ISO gender only allows the values 0,1,2 & 9

I am very conscious that MySQL check constraints are metadata only. They don't actually do anything. Given that it is one of the most popular traditional databases it is likely that a substantial portion of the development community have not come across a working check constraint or realise what it can do to help them.

I suspect the "don't put business logic in the data tier" crowd will make loud noises. Although I think they are right in the majority of cases I am in favour of implementing a check constraint for a data rule that can be expressed simply and clearly with a light weight constraint. If you are in an agile team the argument that it is "hidden" just doesn't stack up.

LinkedIn Profile