How to structure code - best practices?

[NotNull]

This is *very* off-topic and should be posted in that forum, but I think it is better suited here.
(If not: please move the topic)

I am gathering opinions on the following matter from seasoned developers. Or at least people with more programming experience than I have (which is about half this planet ...)

How would you structure this code (and why?)
Is there a third option?
Are there guidelines somewhere?

Thanks in advance!


Option 1

Code: Select all

Action 1
if result 1 {
	action 2
	if result 2 {
		action 3
		If result 3 {
			report success
		} else { 
			report exception 3
		}
	} else { 
		report exception 2
	}
} else {
	report exception 1
}

Option 2

Code: Select all

Action 1
if not result 1 {
	report exception 1
	return
}	

Action 2
if not result 2 {
	report exception 2
	return
}

Action 3
if not result 3 {
	report exception 3
	return
}

report success

[void]

Consistency is important.

I use the following for readability (this might not work for you if you don't have the vertical screen space):
I only report/return once in a function so I can call cleanup functions -this might not apply to your programming language.

Code: Select all

Action 1

if result 1 
{
	action 2
	
	if result 2 
	{
		action 3
		
		If result 3 
		{
			reportcode = success
		} 
		else 
		{
			reportcode = exception 3
		}
	} 
	else 
	{ 
		reportcode = exception 2
	}
} 
else 
{
	reportcode = exception 1
}

report reportcode

Microsoft guidelines:
https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/introduction
https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/inside-a-program/coding-conventions

[NotNull]

*Very* helpful! Thank you!!

Consistency is important.

It was even the main reason for asking this question. My code is - like every beginners code, I guess - now a mix of different styles and naming conventions, which requires mental switching all the time. As codebase keeps growing, this will be an issue further on.

I expected that Microsoft would have documentation and guidelines like this, but as usual, I couldn't find anything on their site (I am not compatible with Microsoft's way of thinking ...). I am glad you were able to. Very useful information.


For now, I think I will go forward with the following:
- Notepad++ Codefolding support will dictate the if { .. } bracing style. If possible, use void's "Option 3"
- Each function gets a "bouncer" at the door that sends unwanted types away.
- all other types have to follow the program (literally ) and sit out the ride until the end.
- Meaning: one exit instead of several.


Again: Thank you for taking the time to help me out!

[planeW]

I tend to prefer option 2, with little nesting and returning early. By reducing the amount of nesting I find it easier to read what's going on and to spot errors. For example in option 1 there's a big gap between calling Action 1 and handling its error/non-success, so I find it more difficult to reason about them and detect any inconsistencies or issues (like forgetting to handle the error case completly).

[aviasd]

This is my method ( Developed by personal taste):

Code: Select all

try {
;Each of the actions could possibly throw an exception from it's code, on failure which is caught inside this try
	Action 1
	Action 2
	result = Action 3
	return result
}
catch  (MyException ex) {
//Handle different cases - you may create a custom execption class if you plan on detecting known, possibly planned exceptions yourself ( or catch all exceptions)
	notfiy(ex.message)
	return false
}
finally {
//Cleanup
}

[NotNull]

Thanks, @aviasd. Interesting.

How would that try/catch/finally mechanism work if it isn't an error but an unwanted result?
like:

Code: Select all

Action1: add 1+1
if result = 2
{
  action 2 etc. ....
}
else
{ 
  return
}

I did restructure the code the 'void-way' and have to say that it makes it much better to follow the 'flow'. And to catch shortcomings.
Even caught a single bracket (without counterpart) that could have caused issues later on.

Extending the code was easier too. Where I first exit on 'if not result 2', I needed some of those results later on after all. That was straightforward now.

I use code folding a lot, so I can see the structure without the details I already figured out. For me it worked best to use inline commenting (which I understand is a no-go) so I see it like below.

All in all Im quite happy with this new way of structuring the code.

2021-03-20 22_09_25-Window.png (17.73 KiB) Viewed 28768 times

[aviasd]

Thanks, @aviasd. Interesting.

How would that try/catch/finally mechanism work if it isn't an error but an unwanted result?
like:

Code: Select all

Action1: add 1+1
if result = 2
{
  action 2 etc. ....
}
else
{ 
  return
}

The idea of exceptions is generally to stop the execution of the next statements if something went wrong and go to the catch error block where you can notify about the error and exit the function.
If you plan to keep running the next statement even though an unwanted value came back, you should use the logic you used above.
It's a matter of personal preference, in my opinion, I find it more readable.
the example above (where the result is not a fault by an unwanted one ) could be written like this with exceptions ( this is only one option )

Code: Select all

try {
	result = Action1(1+1)
	if (result<>2)
	{
		throw "result is not 2!!"
	}  
	next_result = Action2(result)
	...
	return true
catch e {
	notify("Error has occurred:" + e) 
	#outputs "Error has occurred:result is not 2!!"
	return false 
	}
}

Bit off-topic, but if you're programming in Autohotkey, Vscode has some very nice features for that - there are a lot but debugging is probably the biggest ( via autohotkey plus plus plugin )

[NotNull]

Thank you!