Over a million developers have joined DZone.
{{announcement.body}}
{{announcement.title}}

DZone's Guide to

# Terrible Code Example - Methods From Hell

·
Free Resource

Comment (0)

Save
{{ articles[0].views | formatCount}} Views
I have been wanting to write this series of articles for a long time. Over the years I have come across very bad code examples and I have always wanted to share them with you guys, kind of in the way of “Watch and DON’T learn”

Everything you are about to see in this series is based on my personal experience and completely real, so before we begin I have only one request:

Don’t try this at home!

Ok, lets get started.

First, lets take a look at the code. The code is of a real function I found, that determines the color of an object (All variables were changed to keep confidentiality), brace your selves:

`Public Sub GetSomethingColor(ByVal IntVar1 As Integer, ByVal BoolVar2 As Boolean, ByVal BoolVar3 As Boolean, ByVal BoolVar4 As Boolean, ByVal IntVar5 As Integer, ByVal IntVar6 As Integer, ByRef ClassVar7 As ClassType1, ByVal IntVar8 As Integer , ByVal IntVar9 As Integer, Optional ByVal BoolVar10 As Boolean = False) Try    '' some comment     If BoolVar10 = True Then        ClassVar7.PortStatus = SomeColor.Mixed        If ClassVar7.ConnectedIntVar1_2 = -1 Then            ClassVar7.ConnectedIntVar1_2 = IntVar8        End If        If IntVar6 = 1 Then            ClassVar7.Port.BackgroundImage = My.Resources.MixedLedOn()        Else            ClassVar7.Port.BackgroundImage = My.Resources.MixedLedOff()        End If    Else        If BoolVar2 AndAlso IntVar9 > 0 AndAlso (IntVar9 <> IntVar8) Then            ClassVar7.PortStatus = SomeColor.Mixed            ClassVar7.ConnectedIntVar1_2 = IntVar9            If IntVar6 = 1 Then                ClassVar7.Port.BackgroundImage = My.Resources.MixedLedOn()            Else                ClassVar7.Port.BackgroundImage = My.Resources.MixedLedOff()            End If        Else            ClassVar7.ConnectedIntVar1_2 = -1            If BoolVar2 = False Then                If BoolVar3 Then                    ClassVar7.PortStatus = SomeColor.PatchCordNo_DesignYes_DisconnectWONo                    If IntVar6 = 1 Then                        ClassVar7.Port.BackgroundImage = My.Resources._6LedOn()                    Else                        ClassVar7.Port.BackgroundImage = My.Resources._6LedOff()                    End If                    ClassVar7.ConnectedIntVar1 = IntVar8                Else                    Select Case IntVar5                        Case -1 ' some comment                             ClassVar7.PortStatus = SomeColor.SomethingColor                            If IntVar6 = 1 Then                                ClassVar7.Port.BackgroundImage = My.Resources._1LedOn()                            Else                                ClassVar7.Port.BackgroundImage = My.Resources._1LedOff()                            End If                            ClassVar7.ConnectedIntVar1 = 0                        Case IntVar5.Invalid                            ClassVar7.PortStatus = SomeColor.SomethingColor2                            If IntVar6 = 1 Then                                ClassVar7.Port.BackgroundImage = My.Resources._2LedOn()                            Else                                ClassVar7.Port.BackgroundImage = My.Resources._2LedOff()                            End If                            ClassVar7.ConnectedIntVar1 = IntVar9                        Case IntVar5.Valid                            ClassVar7.PortStatus = SomeColor.SomethingColor3                            If IntVar6 = 1 Then                                ClassVar7.Port.BackgroundImage = My.Resources._3LedOn()                            Else                                ClassVar7.Port.BackgroundImage = My.Resources._3LedOff()                            End If                            ClassVar7.ConnectedIntVar1 = IntVar9                        Case IntVar5.Expired                            ClassVar7.PortStatus = SomeColor.SomethingColor4                            If IntVar6 = 1 Then                                ClassVar7.Port.BackgroundImage = My.Resources._4LedOn()                            Else                                ClassVar7.Port.BackgroundImage = My.Resources._4LedOff()                            End If                            ClassVar7.ConnectedIntVar1 = IntVar9                    End Select                End If            Else                If BoolVar3 = False Then ' some comment                     Select Case IntVar5                        Case -1 ' some comment                             ClassVar7.PortStatus = SomeColor.SomethingColor5                            If IntVar6 = 1 Then                                ClassVar7.Port.BackgroundImage = My.Resources._5LedOn()                            Else                                ClassVar7.Port.BackgroundImage = My.Resources._5LedOff()                            End If                            ClassVar7.ConnectedIntVar1 = IntVar8                        Case IntVar5.Invalid                            If BoolVar4 = True Then                                'ClassVar7.PortStatus = SomeColor.SomethingColor13                                 'If IntVar6 = True Then                                 ' ClassVar7.Port.BackgroundImage = My.Resources._6LedOn                                 'Else                                 ' ClassVar7.Port.BackgroundImage = My.Resources._6LedOff                                 'End If                                 'ClassVar7.ConnectedIntVar1 = IntVar9                             Else ' Disconnect Work-Order in phase 1                                 ClassVar7.PortStatus = SomeColor.SomethingColor6                                If IntVar6 = 1 Then                                    ClassVar7.Port.BackgroundImage = My.Resources._7LedOn()                                Else                                    ClassVar7.Port.BackgroundImage = My.Resources._7LedOff()                                End If                                ClassVar7.ConnectedIntVar1 = IntVar9                            End If                        Case IntVar5.Valid                            ' some comment                            ' some comment                             ClassVar7.PortStatus = SomeColor.SomethingColor7                            If IntVar6 = 1 Then                                ClassVar7.Port.BackgroundImage = My.Resources._8LedOn()                            Else                                ClassVar7.Port.BackgroundImage = My.Resources._8LedOff()                            End If                            ClassVar7.ConnectedIntVar1 = IntVar9                        Case IntVar5.Expired                            ' some comment                             ' some comment                             ClassVar7.PortStatus = SomeColor.SomethingColor8                            If IntVar6 = 1 Then                                ClassVar7.Port.BackgroundImage = My.Resources._13LedOn()                            Else                                ClassVar7.Port.BackgroundImage = My.Resources._13LedOff()                            End If                            ClassVar7.ConnectedIntVar1 = IntVar9                    End Select                Else ' some comment                     Select Case IntVar5                        Case -1 ' some comment                             ClassVar7.PortStatus = SomeColor.SomethingColor9                            If IntVar6 = 1 Then                                ClassVar7.Port.BackgroundImage = My.Resources._9LedOn()                            Else                                ClassVar7.Port.BackgroundImage = My.Resources._9LedOff()                            End If                            ClassVar7.ConnectedIntVar1 = IntVar8                        Case IntVar5.Invalid                            ClassVar7.PortStatus = SomeColor.SomethingColor10                            If IntVar6 = 1 Then                                ClassVar7.Port.BackgroundImage = My.Resources._10LedOn()                            Else                                ClassVar7.Port.BackgroundImage = My.Resources._10LedOff()                            End If                            ClassVar7.ConnectedIntVar1 = IntVar9                        Case IntVar5.Valid                            ClassVar7.PortStatus = SomeColor.SomethingColor11                            If IntVar6 = 1 Then                                ClassVar7.Port.BackgroundImage = My.Resources._11LedOn()                            Else                                ClassVar7.Port.BackgroundImage = My.Resources._11LedOff()                            End If                            ClassVar7.ConnectedIntVar1 = IntVar9                        Case IntVar5.Expired                            ClassVar7.PortStatus = SomeColor.SomethingColor12                            If IntVar6 = 1 Then                                ClassVar7.Port.BackgroundImage = My.Resources._12LedOn()                            Else                                ClassVar7.Port.BackgroundImage = My.Resources._12LedOff()                            End If                            ClassVar7.ConnectedIntVar1 = IntVar9                    End Select                End If            End If        End If     End If     ClassVar7.IntVar1 = IntVar1 Catch     MsgBox(Err.Description, MsgBoxStyle.Critical, "Error in Coloring") End Try End Sub`

How long was that! Lets start butchering it from top to bottom

## Method Variables

I don’t know about you, but I think that 10 method variables are just too much!

Once you are sending that many variables to a method, you are either writing a method that does too much, or you are overlooking the possibility that some of these variables have similar uses and responsibilities, and should be grouped into a structure or a class. That object will be passed to methods instead of passing so many variables.

This is a very good example of coding with no prior design, every time there is a need to add some functionality the result is:

“Sure, lets add to method x another variable, some “if elses”, a “switch case” and that’s it! It works!”

## Method Length

168 lines!!!!!!!!!!!!!!!!!!!!!!

How can you write a 168 lines long method?

After 20 lines you have no recollection of: what the method name is, what it is supposed to do, Why are you reading this method, and what got you in this method in the first place.

Debugging and understanding this code is just great this way...

No doubt, This code had to be broken to at least 10 methods and its a no brainer. Switch Case and If Else expressions makes breaking pieces of code into smaller chunks so much easier.

## Try Catch Phrases

Try Catch phrases have a very important role in coding. Using them correctly is an art known to not many people. On the other hand, abusing them is easy as hell. Sure, Why not wrap the whole code in a big try catch and then if there’s an error, we will have no way of knowing where it happened. What to do? Just throw out a message box that says

“Error in Coloring”

That will help the user know what is wrong… NOT!

And of course, Lets make that message box Critical, so the user will get nervous… (line 166)

Throwing a new exception? Or letting it bubble up? Nope, why bother...

I must say, and I think I speak for 99.9% of the programmers:

I hate commenting my code!

But, I do it anyway cause it is so important, not only for the next guy who is going to handle my code, but also for me one month later, when I have to fix some bugs there, and won’t have a clue as to why I wrote this line or another.

Here, I have counted 10 comment lines on 168 code lines, and I can tell you that the comments written weren’t all that informative...

## IF Else Expressions

I know this is more of a flavor question, but using 5 If Else expressions one inside the other? And some Switch Cases for desert? I don’t like it.

that is a recipe for a major Bug. Everyone knows that sometimes you have no choice, but I can tell you that here, there was a choice...

There is no doubt that writing this code will someday come back at you and bite you in the ass, your only hope is that you might not work there anymore...

Topics:

Comment (0)

Save
{{ articles[0].views | formatCount}} Views

Published at DZone with permission of Amit Raz. See the original article here.

Opinions expressed by DZone contributors are their own.