Spaghetti code or not?

[tiredofit]

Is writing spaghetti code always bad?

Of the two approaches below, which is preferable, or should there be a better third approach?

Approach 1:

If Condition1 Then

    Do something1

Else

    Do something2

End If

If Statement1 Then

    Do something1

Else

    Do something2

End If​

Approach2:

If Condition1 Then

   GoTo A

Else

   GoTo B

End If

If Statement1 Then

   GoTo A

Else

   GoTo B

End If

A:

    Do something1

B:

    Do something2

Thanks


​

 

[MARK858]

IMHO approach one is better, 2 many Goto's make the code code difficult to follow and debug.

 

[tiredofit]

IMHO approach one is better, 2 many Goto's make the code code difficult to follow and debug.

Thanks.

I think there are pros and cons to both, hence can't decide.

Approach1

Pro: Easier to read
Con: Involves writing more of the same code

Approach2

Pro: Ability to write the relevant code only once and is re-used
Con: You aleady mentioned it!

 

[MARK858]

Wouldn't fancy having 20 Goto's also I would look at using Case statements.

 

[tlowry]

My opinion: Don’tuse if’s if you can use a “Select Case”

1. They are less mind boggling- lower spaghetti factor
2. Easier to maintain
3. I don’t know if it is faster or slower without testing

Here is some code as an example.
How many if statements would you use to do the following?

[FONT=Calibri][SIZE=3][COLOR=#000000]Option Explicit[/COLOR][/SIZE][/FONT]

[FONT=Calibri][SIZE=3][COLOR=#000000]Sub tester()[/COLOR][/SIZE][/FONT]
[FONT=Calibri][SIZE=3][COLOR=#000000]    example 10[/COLOR][/SIZE][/FONT]
[FONT=Calibri][SIZE=3][COLOR=#000000]End Sub[/COLOR][/SIZE][/FONT]

[FONT=Calibri][SIZE=3][COLOR=#000000]Sub example(iValue As Integer)[/COLOR][/SIZE][/FONT]
[FONT=Calibri][SIZE=3][COLOR=#000000]    Select CaseiValue[/COLOR][/SIZE][/FONT]
[FONT=Calibri][SIZE=3][COLOR=#000000]        Case Is>= 15, 10               'condition 1[/COLOR][/SIZE][/FONT]
[FONT=Calibri][SIZE=3][COLOR=#000000]            MsgBox"tada!"[/COLOR][/SIZE][/FONT]
[FONT=Calibri][SIZE=3][COLOR=#000000]        Case Else[/COLOR][/SIZE][/FONT]
[FONT=Calibri][SIZE=3][COLOR=#000000]            MsgBox"not tada"           'not condition 1[/COLOR][/SIZE][/FONT]
[FONT=Calibri][SIZE=3][COLOR=#000000]    End Select[/COLOR][/SIZE][/FONT]
[FONT=Calibri][SIZE=3][COLOR=#000000]End Sub[/COLOR][/SIZE][/FONT]

 

[Eric W]

If "Something1" is only a line or two, then there's not much duplication of code to worry about. If there's more than a line or two, then you convert it into a subroutine:

If Condition1 Then
   Call Routine1
Else
   Call Routine2
End If

Passing parameters to the routines makes this idea even more flexible. Writing neat, maintainable code takes some effort and some practice, but is well worth it. I'm not a stickler who never uses GoTo, but they should be quite rare. Some languages don't even have that verb. In VBA there are situations that require it (On Error GoTo Somewhere). I'm a professional programmer with decades of experience, and I've seen a lot of different styles, and I can tell you that spending a little extra time up front designing a structured program is more than worth it when it comes to debugging and maintaining the program.

You should familiarize yourself with all the structured elements of the language you're using. In VBA, things like Select Case, Do, While, Loop, Call, functions, Iif, etc.

 

[Fluff]

I would also recommend against using GoTo when possible.
Depending on what you are doing inside the Ifs you could do assign a value to a variable & keep the code at the end of the ifs like

   If condition1 Then
      Sht = "Fluff"
      RngAddr = "10:20"
   ElseIf condition2 Then
      Sht = "Tiredofit"
      RngAddr = "30:35"
   End If
   Sheets(Sht).Range(RngAddr).EntireRow.Hidden = True

 

[tiredofit]

Thanks for all the suggestions.

The problem is the requirements have not been completely defined at the start, hence Iconstantly find myself having to amend the code, so adding GoTo lines seems an easy solution.


On a similar note, I understand worksheets and userforms are a sort of class module and all code residing there should be self contained.

But suppose in Sheet1 and SHeet2 you have this code:

If Me.Range("A1").Value = 1 Then

    Range("B1").Value = 10

Else

    Range("B1").Value = 20

End If

Should the code be written in a standard module and the respective worksheets refer to it, ie:

If Me.Range("A1").Value = 1 Then

    Call Module1.ChangeVal(Me, 1)

Else

    Call Module1.ChangeVal(Me, 2)

End If

and Module1 contains this:

Sub ChangeVal(wks As Worksheet, v As Long)

    Select Case v

        Case 1

            wks.Range("B1").Value = 10
    
        Case 2

            wks.Range("B1").Value = 20
    
    End Select

End Sub

or is it better to repeat the code within each sheet because it's meant to contain "all its own code"?