Is it ok to use the "return" function like this, what I mean is will the local variables and stuff be cleaned up automatically?

This function is called in main, but is in a separate .c file linked by a header.

#include "project.h"
#include "Loop_Functions.h"
#include "Alarm_Macros.h"
void Loop_01(void)

{
    uint8 Loop;
    uint8 Flag;
    CyDelay(5000);
    Loop = Loop_1_Read();
    Flag = Flag_out_1_Read();
    if(Loop == Open && Flag == Down) {Bell_Control_1_Write(1); Bell_Control_2_Write(1);}
    else return; // Here is what I mean
    Alarm_Write(0);
    CyDelay(120000);
    Bell_Control_1_Write(0); Bell_Control_2_Write(0);
    CyDelay(5000);
    Loop = Loop_1_Read();
    Flag = Flag_out_1_Read();
    if(Loop == Open && Flag == Down) Bell_Control_2_Write(1);
    else return; // And here
    Flag_in_1_Write(Up);
    CyDelay(300000);
    Bell_Control_2_Write(0);
    Flagged_Write(0);
    
}

 

Yes, that is perfectly legal and correct use of return.
I don't like doing that though as it is not obvious that there are multiple possible exits from the function.
https://www.geeksforgeeks.org/return-statement-in-c-cpp-with-examples/

 

Thanks Al, I actually read that link and many more as well, but I kept having a bad feeling about whether the destroyer would be called or not, because the variables will be declared in other functions using the same name.

 

Is it ok to use the "return" function like this, what I mean is will the local variables and stuff be cleaned up automatically?

Yes, what you have done is perfectly legal and well-defined, but it is generally not preferred without a real good reason (and good reasons do exist).

Your code, as presented, is very hard for most humans to read and that makes it very easy for mistakes to go overlooked. At least indent your code in a coherent and generally accepted way, especially when you want others to review it. But you should find that it helps YOU out quite a bit, as well.

Here's your code with absolutely no changes other than indentation and spacing:

#include "project.h"
#include "Loop_Functions.h"
#include "Alarm_Macros.h"

void Loop_01(void)
{
    uint8 Loop;
    uint8 Flag;

    CyDelay(5000);
    Loop = Loop_1_Read();
    Flag = Flag_out_1_Read();

    if (Loop == Open && Flag == Down)
    {
        Bell_Control_1_Write(1);
        Bell_Control_2_Write(1);
    }
    else
        return; // Here is what I mean
   
    Alarm_Write(0);
    CyDelay(120000);
    Bell_Control_1_Write(0);
    Bell_Control_2_Write(0);
    CyDelay(5000);
    Loop = Loop_1_Read();
    Flag = Flag_out_1_Read();

    if (Loop == Open && Flag == Down)
        Bell_Control_2_Write(1);
    else
        return; // And here

    Flag_in_1_Write(Up);
    CyDelay(300000);
    Bell_Control_2_Write(0);
    Flagged_Write(0);
}

Also, unless you are completely confident that you know the associativity and relative precedence of the operators you are using, it is best to make liberal use of parentheses to force the compiler to do what you intended. Plus, it makes it easier on others that are reading your code.

For example, you might consider writing your loop test as

if ( (Loop == Open) && (Flag == Down) )

It takes little time to add the parens, makes your intent explicit, forces the compiler to do what you intended, and just the effort and thought needed to do it will result in you catching the odd typo or logic goof that we all make from time to time.

While this is particularly useful for the less-universal operators, it should also be kept in mind for the more mundane operators. How many times have we all seen code like

 Vout=Vin*R2/R1+R2;

and the person can't figure out what the problem is? Far better to write it as:

 Vout = (Vin * R2) / (R1+R2);

or

 Vout = Vin * ( R2 / (R1+R2) );

 

I agree with everything WBahn said, but one minor tweak I would add due to my own preferences, is to make the return statement the true outcome rather than false. Less compound statements and seems a little clearer.

if (Loop != Open || Flag != Down) 
    return;
Bell_Control_1_Write(1); 
Bell_Control_2_Write(1);
//...

 

I agree with everything WBahn said, but one minor tweak I would add due to my own preferences, is to make the return statement the true outcome rather than false. Less compound statements and seems a little clearer.

if (Loop != Open || Flag != Down)
    return;
Bell_Control_1_Write(1);
Bell_Control_2_Write(1);
//...

In principle I agree. The particulars of the specific logic may or may not be clearer after applying DeMorgan's theorem, plus there is always the possibility of misapplying it (seen that many a time!). If the modified logic is going to be easily grasped as written by the target audience (which may be just yourself), then modifying has several advantages. There are probably some code-size and performance advantages (depending on how good a job the compiler does at optimization), but it also makes code that has embedded returns quite a bit easier to read (most of the time).

 

plus there is always the possibility of misapplying it (seen that many a time!)

Right! Even with this simple example I had to stare at it three times to make sure it was right.

I can't help scoff when I see the superfluous else in the following:

if (condition)
    return;
else
{
    statements();
}

 

Along the lines of what 402DF855 said, I really don't like this:

if(Loop == Open && Flag == Down) {Bell_Control_1_Write(1); Bell_Control_2_Write(1);}
    else return; // Here is what I mean
 Alarm_Write(0);
CyDelay(120000);
etc

What's the sense of executing the Bell_Control statements following the if() statement, then jumping round to the Alarm_Write(), which always executes if the Bell_Control statements do? I think it would be clearer to write it as:

if(Loop != Open || Flag != Down)
    return; 
Bell_Control_1_Write(1); 
Bell_Control_2_Write(1);
Alarm_Write(0);
CyDelay(120000);
etc

 

Along the lines of what 402DF855 said, I really don't like this:

if(Loop == Open && Flag == Down) {Bell_Control_1_Write(1); Bell_Control_2_Write(1);}
    else return; // Here is what I mean
Alarm_Write(0);
CyDelay(120000);
etc

What's the sense of executing the Bell_Control statements following the if() statement, then jumping round to the Alarm_Write(), which always executes if the Bell_Control statements do? I think it would be clearer to write it as:

if(Loop != Open || Flag != Down)
    return;
Bell_Control_1_Write(1);
Bell_Control_2_Write(1);
Alarm_Write(0);
CyDelay(120000);
etc

I very much agree. When using embedded returns, you really want to keep the logic near them simple and make them stand out -- otherwise you sacrifice most of the benefit from using them but pick up all of the downsides of doing so.

Also, remember that inverting a control statement is trivially easy.

Consider the snippet:

    if ( (Loop == Open) && (Flag == Down) )
    {
        Bell_Control_1_Write(1);
        Bell_Control_2_Write(1);
    }
    else
        return;
   
    Alarm_Write(0);
    CyDelay(120000);
    Bell_Control_1_Write(0);
    Bell_Control_2_Write(0);

Let's assume that the test expression was such that, for clarity and readability, you didn't want to use DeMorgan's on it. So simply invert it to get:

    if ( !((Loop == Open) && (Flag == Down)) )
        return;
   
    Bell_Control_1_Write(1);
    Bell_Control_2_Write(1);
    Alarm_Write(0);
    CyDelay(120000);
    Bell_Control_1_Write(0);
    Bell_Control_2_Write(0);

 

Hi,

The more cryptic the code the harder it is to maintain. Large programs can get very tedious to work with when the code is hard to read. The short answer is develop better coding habits.

As others have pointed out, indenting is important for readability. That can make a big difference when you go back over it years later.

I've even considered indenting in places where we dont normally indent just to make the code more readable. For example, when opening a file we normally dont indent between the open and close statements, but if we do we will always be able to see where the file opens and where it closes at a glance and that can save time reading the code months or years later.
Of course all loops and if/then statements should always be indented, and it is good to stay with the same style over all your files and projects.

To the point, the return statement gets you out of a heavily populated crop of code but there is always the chance that something turns out not as intended with multiple return statements. This means whenever possible just use one return statement. If it gets too complicated though then there might be no good reason to follow that rule.

 

Thanks for the advice guys.

 

Is it ok to use the "return" function like this, what I mean is will the local variables and stuff be cleaned up automatically?

This function is called in main, but is in a separate .c file linked by a header.

#include "project.h"
#include "Loop_Functions.h"
#include "Alarm_Macros.h"
void Loop_01(void)

{
    uint8 Loop;
    uint8 Flag;
    CyDelay(5000);
    Loop = Loop_1_Read();
    Flag = Flag_out_1_Read();
    if(Loop == Open && Flag == Down) {Bell_Control_1_Write(1); Bell_Control_2_Write(1);}
    else return; // Here is what I mean
    Alarm_Write(0);
    CyDelay(120000);
    Bell_Control_1_Write(0); Bell_Control_2_Write(0);
    CyDelay(5000);
    Loop = Loop_1_Read();
    Flag = Flag_out_1_Read();
    if(Loop == Open && Flag == Down) Bell_Control_2_Write(1);
    else return; // And here
    Flag_in_1_Write(Up);
    CyDelay(300000);
    Bell_Control_2_Write(0);
    Flagged_Write(0);
   
}

Yes, it is okay to do that, and here is why. C and C++ handle functions the same way- they are build on the stack. Every function or procedure that is built on the stack, has what is called a 'stack-frame'. That is, a bit of extra-space on the stack ahead of the function, and clean-up code at the end of the function to set up a temporary place for local variables to be kept, and registers to be preserved. When the clean-up code is called- which it is when you call 'return', this 'stack-frame is eliminated, thus freeing static storage. If you allocate anything from memory that is in the heap, and is not destroyed, so if you allocate anything that you wish to go away when the function ends- you need to deallocate it, and zero the pointer.

Lastly, the way the stack 'eliminates' the code and the stack-frame, is simply to set the stack-pointer back to where it was before the function was called.

 

Thanks Boba,

Yea, I have been reading up on the differences between the stack and the heap...good stuff.

 

Thanks Boba,

Yea, I have been reading up on the differences between the stack and the heap...good stuff.

One thing that is good to keep in the back of your mind is that there is actually no requirement that C use a stack at all. In fact, the language standard (or at least the draft standard for C99, which is what I have) doesn't even contain the word 'stack'. What are defined are the behaviors of the various language constructs and they leave it up to the implementers to adhere to them however they want to. Now, it's pretty evident that the standard writers assume that the implementation will be stack-based and I'm not aware of any implementation that is not stack-based, but it also wouldn't surprise me to find some compiler for some memory-starved microcontroller that isn't.

The point is that while the assumption of a stack-based architecture is usually valid and a very useful framework from which to envision things, be careful not to assume that something has to be true just because it would be a true of a stack-based architecture. It only has to be true if it is required by the language standard or specified by the compiler documentation.

 

One thing that is good to keep in the back of your mind is that there is actually no requirement that C use a stack at all. In fact, the language standard (or at least the draft standard for C99, which is what I have) doesn't even contain the word 'stack'. What are defined are the behaviors of the various language constructs and they leave it up to the implementers to adhere to them however they want to. Now, it's pretty evident that the standard writers assume that the implementation will be stack-based and I'm not aware of any implementation that is not stack-based, but it also wouldn't surprise me to find some compiler for some memory-starved microcontroller that isn't.

The point is that while the assumption of a stack-based architecture is usually valid and a very useful framework from which to envision things, be careful not to assume that something has to be true just because it would be a true of a stack-based architecture. It only has to be true if it is required by the language standard or specified by the compiler documentation.

Is this some sort of unwritten standard then given a particular platform?
For example in Windows i think the calling conventions are stdcall and cdecl both of which use the stack.
There has to be some way to know which convention is being used in order to interface in raw machine code (binary standard).

 

The PSoC IDE I'm using uses the stack, I know that for a fact.

So, I made these changes to my code.

1. Added the parentheses.
2. Changed the first if statement. (no more else)
3. Removed the second flag read, and the if condition (wasn't needed)
4. Added "turn off ready LED" (forgot that)

Programed my board, and everything works.

 

Is this some sort of unwritten standard then given a particular platform?
For example in Windows i think the calling conventions are stdcall and cdecl both of which use the stack.
There has to be some way to know which convention is being used in order to interface in raw machine code (binary standard).

Calling conventions are outside the scope of the language standard. I just downloaded N1570, which is the final draft of the C11 standard, and neither the word 'stack' nor 'stdcall' appear anywhere in the document.

Conventions such as cdecl and stdcall are calling conventions for X86-based processors the compilers commonly implement -- they are not required by the language and not every compiler implements them.

 

Calling conventions are outside the scope of the language standard. I just downloaded N1570, which is the final draft of the C11 standard, and neither the word 'stack' nor 'stdcall' appear anywhere in the document.

Conventions such as cdecl and stdcall are calling conventions for X86-based processors the compilers commonly implement -- they are not required by the language and not every compiler implements them.

Yeah that makes sense now that i think about it. Some systems may not even have a stack.

 

This is a good question because it comes up often even in very experienced teams. I've had code rejected in the past because I violated the "only one exit point" "rule". This is a style question and I have never supported this one exit point rule myself because it's not meaningful.

A function often has several exit reasons and making the developer mask this with one exit point is not conveying clarity.

I think its much better to clearly indicate each exit scenario with an immediate return, with the one exit point rule your code might reach a point where its just done, over, yet someone reading it likely won't know because the code is forced to artificially plod along until the end.

 

A recurring weakness even in modern imperative languages, is the tolerance of ignoring returned values, C, C++, Java, C# and so on, all allow this yet it's the seed of some bad practices and the compilers should at least issue a warning.

In functional languages this is forbidden, impossible.