Clean Coding in Xojo: More tips from the community

General
1

Continuing this Post.

tI’d like to add the following, which improved the readabillity of my own code by a lot.

Wrap the code as little as possible:

// Bad
If THIS = THAT Then
  If THIS Is Not THAT Then
    ...
  End If
End If

// Good
If THIS <> THAT Then Return
If THIS Is THAT Then Return
...
5 Likes
2

Back in the early 80s there seemed to be a craze for SESE - Single Entry, Single Exit. So the Return statement was frowned upon and you were supposed to always exit the method at its bottom. As with all crazes, I just ignored that then and still do; I always test for as many error conditions as possible as soon as possible and then return a code, or log it or whatever. That way the code looks simpler and I don’t have unreadable if statements nested 15 deep.

3 Likes
3

is a much nicer way of doing ‘GOTO endlabel’
And it is much cleaner.
trying to work out which end if / next statement refers to which starting line , many rows above, can be a pain.
I usually try to put a comment at the end of each end if

end if // end of IF THING = TRUE

2 Likes
4

Ha yes, I remember having to do that too - with Pascal under RSX-11M.

5

I totally agree with the returning early philosophy and it’s part of the ‘aligning the happy path to the left’ style. Meaning structuring your code so that the most straightforward and expected flow is visually prominent and easy to follow.

The only problem with your Xojo code is that if you want to put a breakpoint in later to catch a return condition you have to change your code. My goal is to never have to change code for breakpoints.

Yes, it’s kind of pedantic, but people make mistakes when rewriting even simple code. So why rewrite if you don’t have to? I usually write them like this:

// Better for breakpoints
If THIS <> THAT Then 
   Return
end if

If THIS Is THAT Then 
   Return
end if
9 Likes
6

This Go Happy Path: the Unindented Line of Sight | maelvls dev blog is for Go (which I’ve spent a lot of time in recently), but I’ve started adapting it for my new Xojo code and I find that I like it - a lot. It really helps with readability. Your needs and tastes may vary.

My coworker and I are both experience Xojo users (close to 20 years for each of us) and without any planning on our part our Xojo code is becoming more Go-like with the Left Happy Path philosophy.

3 Likes
7

Is it wrong to suggest…

if THIS = THAT then
  //do stuff
else
  //other stuff
end if
return

To my mind it is confusing to explicitly test for an implied condition.

That’s always been the big problem with adhering to style guides, they don’t always suit your style.

Wasn’t there some technical reason for SESE that got trumped by Fail Early?

Seems to me fail early is similar to, “Happy path is left aligned.” Without sounding quite so twee.

8

Some aspects of programming are subjective :wink:
Here’s an interesting and related read: Return Early Pattern. A rule that will make your code more… | by Leonel Menaia Dev | The Startup | Medium

9

There is no right or wrong, just style and preference, as long as your code does what it is intended.

As for your specific example: I have plenty of methods that do quite a bit of checking before they start their actual work. It’s a lot cleaner to have multiple check-return blocks than nesting them as you’ve suggested.

Also, if your function returns a boolean to indicate success or failure, you can avoid a variable declaration and a lot of tracing by breaking out the checks into their own blocks.

1 Like
10

One problem I have with this article is when it puts all the code in one block like this:

Sub DoEverything()
  PerformTaskA
  PerformTaskB
  PerformTaskC
End Sub
 
Sub PerformTaskA()
  ' Task A code
End Sub
 
Sub PerformTaskB()
  ' Task B code
End Sub
 
Sub PerformTaskC()
  ' Task C code
End Sub

It gives the impression you can put all of that code in one single method, which could confuse the hell out of people new to Xojo, especially if they’re coming from C#, which allows you to have multiple functions in one script, like the code above.

11

No, I’m explicitly talking about return early error/condition checking. There’s nothing wrong with if-then-else statements (though Go tends to avoid else conditions).

12

generally cascading multiple if then is not good.
and write a lot of code between if then is not good, instead use a method and call this.

sometimes I use a boolean flag.

ok=false
if this = that then ok = true
if this = that then ok = true
if ok then

or other way

ok=true
if this <> that then ok = false
if this <> that then ok = false
if this <> that then ok = false
if ok then

instead of cancel a method inside we can also call a method if the condition is good

If THIS <> THAT Then Return

If THIS = THAT Then CallMe
1 Like
13

Using flags, sometimes is a great way, sometimes is a bad design.

Your sample approach, for example:

Var ok As Boolean = true
if a <> 1 then ok = false
if b <> 2 then ok = false
if c <> 3 then ok = false
if ok then doIt()

Can be written as an one faster oneliner as:

// If not clear, explain what it does
If a = 1 And b = 2 And c = 3 Then doIt()
14

Bad design:

Sub DoEverything()
  PerformLocalTaskA
  PerformLocalTaskB
  PerformLocalTaskC
End Sub
 
  Sub PerformLocalTaskA()
    ' Task A code
  End Sub
   
  Sub PerformLocalTaskB()
    ' Task B code
  End Sub
   
  Sub PerformLocalTaskC()
    ' Task C code
  End Sub

Good design impossible in Xojo:

Sub DoEverything( radius As Double )

  Sub PerformLocalTaskA( value As Double, radius As Double )
    ' Task A code
  End Sub
   
  Sub PerformLocalTaskB()
    ' Task B code
  End Sub
   
  Sub PerformLocalTaskC()
    ' Task C code
  End Sub

  // Explain if necessary... blah, blah, using 1/4 of circumference
  PerformLocalTaskA(2 * 3.14 / 4, radius)

  // Explain if necessary
  PerformLocalTaskB

  // Explain if necessary
  PerformLocalTaskC

  // Let's do Task A one more time to finalize, using 3/4 of the circumference
  PerformLocalTaskA(3.14 / 2 * 3, radius)

End Sub
15

Why would that be better?
Now you have 3 methods that cannot be used by the rest of the app.
Is it that they can share local variables, maybe?
If so, why pass radius into Task A?

2 Likes
16

In reality I was just using the proposed design to show the real thing, the local reuse of blocks of code. Easier to see in a cleaner code like:

Sub PrepareTheSpecialCircle(colorA As Color, colorB As Color, radius As Double)

  Sub ComputeSpecialArc( value As Double, radius As Double, theColor As Color )
    // the code
  End Sub

  ComputeSpecialArc(2 * 3.14 / 4, radius, colorA)  // Draw 1/4
  ComputeSpecialArc(3.14 / 2 * 3, radius, colorB)  // Draw the rest

End Sub

Scope

To break the isolation Xojo could introduce some keyword/syntax to “import” some variable from the outer scope (as Python’s “nonlocal”) as:

Sub PrepareTheSpecialCircle(colorA As Color, colorB As Color, radius As Double)

  Sub ComputeSpecialArc( value As Double, theColor As Color )

    // proposed keyword "outer" says "use this variable that must exist outside this scope"
    Outer radius

    // the rest of code, and it now can see the variable (parameter) radius

  End Sub

  ComputeSpecialArc(2 * 3.14 / 4, colorA)  // Draw 1/4
  ComputeSpecialArc(3.14 / 2 * 3, colorB)  // Draw the rest

End Sub
1 Like
17

Another small thing that I try to pay meticulous attention to is that I keep the scope of all objects as small as possible. This helps me to keep the autocomplete as compact as possible.

1 Like
18

I was being somewhat pendatic. The examples are variations on…

if THIS = THAT then return
if THIS <> THAT then return

The second comparison is redundant as it is implied by the result of the first. I realise this is an example but I have come across code like that in the wild. The redundancy sticks out like a sore thumb to me.

// avoid redundancy
  if THIS <> THAT then 
    return
  else
    //this equals that
  end if
  return
//avoid redundancy
  if THIS = THAT then 
    return
  end if
  //this does not equal that
  return

Due to my career path it is a very long time since anyone was reviewing my code. It’s interesting to see old concepts being re-discovered.

Fail late was explained to me as simply testing for error conditions first. As far as I can tell it produces the same result as “Left Happy Path,” but if anyone had mentioned ‘Happy Path’ in the teams I was working in 30 years ago the reaction would have been a small amount of sick :face_vomiting:

// fail late
const ERROR = -1
  c = a + b
  c  = c * n
  result = c

  if b = 0 then 
   result = ERROR
  end if 
return result
//fail early
const ERROR = -1
  if b = 0 then
    return ERROR
  end if
  c = a + b
  c  = c * n
return c
19

Isn’t Boolean False by Default at Declaration Time ?

20

Thats the point.

PerformLocalTaskA, PerformLocalTaskB and PerformLocalTaskC are only relevant to DoEverything. It is a more readable / manageable version of putting all of the code into DoEverything. This allows you to structure your code better without making the methods available elsewhere (even within the same class).