DEV Community

10 CODING MISTAKES THAT MAKE YOUR CODE SMELL

hussein cheayto on August 18, 2019

I've created this list by walking through several different programs, coding books (like "The Pragmatic Programmer" by Andy Hunt & Dave Thomas ...
Collapse
 
moopet profile image
Ben Sinclair

I don't think that the corrected comment in point 2 is any better than the one it replaces.

Public void DisplayTutorial() //Displays window

That's redundant, and if it's not then it's a smell that the method should be called DisplayTutorialWindow or something instead.

I'm not sure any of your "related articles" are related in any way other than they're written by you? Are they just spam?

Collapse
 
hussein_cheayto profile image
hussein cheayto

They are related to programming and how to succeed in life.
Check them out and see πŸ˜‰

Collapse
 
moopet profile image
Ben Sinclair

Methods that are never called should be discarded. Keeping dead code around is wasteful. Don’t be afraid to delete the function. Remember, your source code control system still remembers it.

Related Article: Get Rich While Sleeping

Hmm.

Thread Thread
 
hussein_cheayto profile image
hussein cheayto

The title is tricky.
If you read it, you will find that in the article, all I talk about is programming.

Thread Thread
 
moopet profile image
Ben Sinclair

The title is tricky.
That's not exactly true. It is a link to a page about getting rich quick, though that page doesn't offer any real advice.

in the article, all I talk about is programming.
That's not true at all. You have three sections, each of which exists to provide a link to another unrelated get-rich-quick page, and only one of which is called "programming".

And it doesn't talk about programming. It says you can make money in your sleep if you publish an app. You can do the same if you publish a book, by the way, this isn't related to programming.

This linked page has nothing whatsoever to do with your preceding paragraph, and calling it a "related article" is misleading to say the least.

Thread Thread
 
hussein_cheayto profile image
hussein cheayto

hmmmmmm, I appreciate your comment, I will take it into consideration in my next articles.
If you didn't like it, you can simple close the tab and skip it :)

Collapse
 
techsuman2019 profile image
Tech Suman

"No argument is best" What's your rationale behind this? If a function isn't receiving any argument, then either of three things can be said about it. One, it's not receiving data from other parts of the program and is directly interacting with the user to get it's inputs. Two, it's interacting with global variables. Three, it's a routine task, that doesn't need any data. If it's return value is session invariant, or is using some other library like datetime, then it makes sense, otherwise, function with no argument seems a little funky to me. The first two possibilities are a big no no.

Collapse
 
alexanderholman profile image
Alexander Holman

There are a few more than 3 ways to peel a potato! e.g. where the data owns the method, and the methods mutate the instance directly without any input, see the example below:

class Account {
    private LocalDate expires; // e.g. 2019-08-19
    public void renew()
    {
        this.expires = this.expires.plusYears(1); // now 2020-08-19
    }
}
Collapse
 
eljayadobe profile image
Eljay-Adobe

I agree with this. Having a function with no arguments means the function only has a side-effect, or operates on variables external to the function. It is far better to pass in all parameters to the function than having a function that is entangled with external variables (including member object variables, which are "global" to the function, even if the function is a member function of the same object).

Collapse
 
srini85 profile image
Srini Vasudevan

Thanks for sharing. Good article. One thing to add about comments is, it is better to avoid all together unless it's helping with auto generated documentation. Methods/functions should be self explanatory, and as you have pointed out, they should do one thing. In that case, name it accordingly and you wont need comments. E.g rather than calling a function Increment(), if it increases a number by one, better name could be IncrementByOne(). Trivial example but shows the intention.

In my years of development, I've always found comments always confuse developers more than they help as the time they stay in the source code grows. They can rot easily as code changes but devs can forget to remove or update comments. Best to avoid them all together :)

Collapse
 
hussein_cheayto profile image
hussein cheayto

Absolutely correct!!
Why need comments if variables and function names explain it all ;)

Collapse
 
woubuc profile image
Wouter

Formatting tip: if you put 3 backticks, it will create a code block for your code snippets.

Like so:

```
your code here
```

You can even set a language for syntax highlighting:

```javascript
your code here
```
Collapse
 
hussein_cheayto profile image
hussein cheayto

The list goes on and on... Thanks for pointing that out

Collapse
 
zakwillis profile image
zakwillis

Can agree with these. Sometimes they can't be avoided.

Collapse
 
hussein_cheayto profile image
hussein cheayto

True, but try to think on how to reduce them.

Collapse
 
zakwillis profile image
zakwillis

I would say this. In simple applications, following something like what Uncle Bob adheres to, it is easy to do what you suggest. However, some code is just complicated. Ideally, maybe 80-90% of code follows what you suggest, but sometimes it is too much to expect a developer to break everything up into functional units. It can be done but perhaps it isn't worth spending another week on an application that has taken three weeks.

Right now, I am building a publisher application and everything has been broken into simple objects with one method and using dependency injection but do I care enough to make the fact that it will only call four main functions follow the open closed principle;
NewReportPublication
SaveReports
PublishReports
NotifyEndPoint

Or can I get by with an enum and switch?

Anyway, open for discussion and good post.

Thread Thread
 
hussein_cheayto profile image
hussein cheayto

Theoretically, it is easy, practically, it needs a lot of practice and logic.

Planning is important. I believe that spending 2 or 3 days brainstorming and planning will save you much time later on.

Goodluck with your project, once done, share a link so that I can check it :)

You're a developer and you know that time is crucial.

Have you ever worked on a project where the previous developer had a smelly code? Like bad names, comments....

Thread Thread
 
zakwillis profile image
zakwillis

I don't know. I don't like to pretend my sh!t doesn't stink. We all make mistakes when working and time pressure.

What appears readable and perfect to one person is not to another. Some patterns seem like nonsense to me, whereas others love it. I don't know if you are a C# developer, but as an example - should Entity Framework's linq expressions get exposed to the Business Layer? Or should it be abstracted away? I avoid using Entity Framework unless for clients but the answer is, of course - we should put a layer in-between. Many though would see this as overkill.

Anyway, you aren't talking to a new developer who struggles with these things, just I think we need to be a bit more balanced. But again, thanks for your article and it is useful for many.

Thread Thread
 
hussein_cheayto profile image
hussein cheayto

I guess there's a misundertanding here.
The point in my question was: always think that there's another one that will read your code (it might be yourself after a couple of days, months or years).

No matter how experienced we are, we still make some mistakes, that's for sure. That's why we keep learning ;)

Thanks and glad you like my article Zak

Thread Thread
 
zakwillis profile image
zakwillis

All good. And keep it up.

Collapse
 
aeharake profile image
Ahmad El Harake • Edited

I knew you were a French educated Lebanese because of "substract" instead of "subtract". Great article keep it up. However, I should point out that since you mentioned "consistency" as one essential point, this would contradict with the idea 4. As sometimes for the sake of dependency, we keep functions with x arguments calling an overloaded one with x-1 arguments and etc.

Collapse
 
hussein_cheayto profile image
hussein cheayto

You have the eyes of a legendary developer πŸ‘€
Thanks, I have fixed the function name.
It's an open discussion, there's no true or false question. Appreciate your comment :)
Glad that you like my article.

Collapse
 
alloush78 profile image
Ali Al-Kaissi

On the point Hussein, if I may add to this is the code repetition which is seeing the same block of code again and again in same/different places rather than using Functions/Classes to minimize code as much as possible.

Cheers!

Collapse
 
hussein_cheayto profile image
hussein cheayto

Thanks for pointing that out Ali. Yes, you're right.

Collapse
 
firozansari profile image
Firoz Ansari

Some good pointers! I stick with a great quote by Cory House when it's come to providing comments to the code:

Code is like humor. When you have to explain it, it’s bad.

Collapse
 
hussein_cheayto profile image
hussein cheayto

Hahahahaah I like that

Collapse
 
axegon profile image
Alex
  1. Bad commit messages. I can't express how much I hate commit messages which only say "fixed". It's fine if it's a small private repo, but if you are working on a large project with others involved... No, just no!
Collapse
 
thorstenhirsch profile image
Thorsten Hirsch

All-caps headlines smell bad, too. :-)

Collapse
 
ogurinkaben profile image
Ogurinka Benjamin

Really insightful

Collapse
 
hussein_cheayto profile image
hussein cheayto

Thanks

Collapse
 
pgalinski profile image
pgalinski

I hoped You will dive dipper into topic.

Collapse
 
hussein_cheayto profile image
hussein cheayto

That would be a good idea for my coming article.

Collapse
 
theunisv profile image
Theunisv

Had a great time reading this article. All I can say is
NamingConventions... namingConventions... Naming_Conventions...

Keep it up mate.

Collapse
 
hussein_cheayto profile image
hussein cheayto

Thanks for your support Theunisv!!
Glad you like it :)

Collapse
 
pris_stratton profile image
pris stratton • Edited

If a function is capable of altering a global variable that might cause an odd smell, like milk that's gone off.