Function question

Having got my first 6k lump of code working, the first thing I want to do is to rewrite it to reduce the clutter and some duplication in it.

I've read all I can find about Functions, but I can't find anything that confirms that multiple arguments can return multiple values or indeed what limits there are on that... can I mix LCD instructions with int arguments? I'm guessing that I should be able to?

I suppose what I'm asking is, is it better 'practice' to try and build lots of simple functions or a fewer more complex ones if they do the job?

Any guidance is most welcome!

One function, one return value, irrespective of the number of arguments.
However, you can return values via pointer or reference arguments.

can I mix LCD instructions with int arguments

Not sure what you mean by that - can you explian, please?

One function, one return value, irrespective of the number of arguments.

Which includes returning a struct...

LotsOfStuff.h ...

typedef struct
{
  char        c1;
  short        s1;
  long        l1;
}
LotsOfStuff_t;

Sketch...

#include "LotsOfStuff.h"

LotsOfStuff_t Function( void )
{
  static LotsOfStuff_t    los;
  
  los.c1 = 'X';
  los.s1 = 1313;
  los.l1 = 0x12345678;
  
  return( los );
}

void setup( void )
{
  LotsOfStuff_t    los;
  
  los = Function();
}

void loop( void )
{
}

I suppose what I'm asking is, is it better 'practice' to try and build lots of simple functions or a fewer more complex ones if they do the job?

Years ago I read a research paper about the ideal function size. The author determined that approximately 25 lines of code was the ideal size. A few years later the author published an update: Using the same criteria (like bugs per 1KLOC) the author determined that approximately 43 lines of source code was the ideal size. What changed? The lines of text that could be displayed at one time on a monitor. A good rule of thumb is to try to keep functions small enough that you don't have to use the PageUp and PageDown keys too much.

Generally speaking, you want to extract code that is duplicated and turn it into a function.

LotsOfStuff_t Function( void )
{
  LotsOfStuff_t    los;
  
  los.c1 = 'X';
  los.s1 = 1313;
  los.l1 = 0x12345678;
  
  return( los );
}

Returning an automatic variable?
Tut-tut.

Returning an automatic variable?

Not any more. :wink:

Sidebar: Wow! I love this AVR-GCC compiler! Because Function was called only once, the compiler inlined the code. Even though los was an automatic variable, the code will work! (but DON'T rely on that; returning automatic variables is a big no-no)

Thinking about it further, I don't think there is anything actually wrong with returning an automatic (because the contents will be copied), but the problem is returning a pointer to an automatic, which is a sure-fire way to screw up.

Thanks for the responses!!

Quote:
can I mix LCD instructions with int arguments

Not sure what you mean by that - can you explian, please?

@Groove, I think you kindly gave me the anser to that question in your first two lines with;
,

One function, one return value, irrespective of the number of arguments.

I was thinking of something like; get a keypress from a button, do a calculation on some variables and display something on the LCD as each of the previous steps took place.

I dont know the correct vocabulary but a 'subroutine' might have been what I think I meant(?)

I'm now thinking that it would be something more like individual functions for;

update LCD
get button press
debounce button
get variable from sensor 1
get variable from sensor 2
update LCD
calculation on variables
update LCD

Rather than jamming all of that into a single function - which now I write it out like that looks like a nightmare.

Generally speaking, you want to extract code that is duplicated and turn it into a function.

ok, I understand that. If there's no "exact" duplication in a couple of loops in a sketch, perhaps only 60% is dulpicated the way it's currently written (better practice question again) do you rewrite in such away that the duplicated parts CAN become a function, and the non duplicated parts, either do or dont become different functions?

The simplified model I have in my head is of building blocks... you can spell any word you want, if you have the blocks with the right letters on them. I guess I'll understand this much better after doing it!

automatic variable

Whoa!! ..I can't find anything about these things in the usual places I tend to look, do I need to know what they are (if only to avoid the variable Police visiting)?

My head is starting to hurt again and I've only just got all the blood off the walls :slight_smile:

I guess I'll understand this much better after doing it!

You'll wonder why you ever tried it any other way!

An automatic variable is one assigned on the stack (unlike a global or static variable), so it only has validity during the lifetime of the function.
If you try to return a pointer to qan automatic from a function, the pointer itself is valid (it points to memory that exists), but because that memory is on the stack, its contents could be reused at any time, by for instance, the next function to be called.

do you rewrite in such away that the duplicated parts CAN become a function, and the non duplicated parts, either do or dont become different functions?

I'm afraid I don't understand. If you don't mind, please reword the question.

The simplified model I have in my head is of building blocks... you can spell any word you want, if you have the blocks with the right letters on them. I guess I'll understand this much better after doing it!

Typically, the more application specific the code is the less it can be used like a building block. eeprom_read_byte is a good example of something that IS like a building block. It's a function we can both find useful and it serves the same purpose for both of us.

Let's say you have a function named "UpdateLCD" in your Sketch. It is unlikely that your UpdateLCD is useful to me. I may be able to extract things from it that are useful to me but the function as it is in your Sketch isn't very useful to me. UpdateLCD is not like a building block. In my experience, the THIRD time you use some piece of code is when you should consider turning it into a building block.

Focus on tasks you want the Arduino to perform. Let's say you're developing a digital thermometer. You want "T: ---.-" displayed in setup and the actual temperature displayed when it's read. You might create this function...

const float NoReading = -999;

void DisplayTemperature( float T )
{
  // Move the LCD cursor to the correct location
  // Output the fixed part "T: "
  if ( T > NoReading )
  {
    // Convert and format T
    // Output the text to the LCD
  }
  else
  {
    // Output "---.-" to the LCD
  }
}

Then call it like this from setup...

  DisplayTemperature( NoReading );

And like this when the temperature has been read...

  DisplayTemperature( CurrentReading );

Thanks

Quote:
Generally speaking, you want to extract code that is duplicated and turn it into a function.

ok, I understand that. If there's no "exact" duplication in a couple of loops in a sketch, perhaps only 60% is dulpicated the way it's currently written (better practice question again) do you rewrite in such away that the duplicated parts CAN become a function, and the non duplicated parts, either do or dont become different functions?

The reworded question, (don't mind at all)

In a situation where the contents of one loop is not completely repeated, perhaps only 60% of it is repeated in a second loop. Is it better practice to redesign the structure and rewrite the code in such a way that the repeated "60%" can be utilised as a function?

The answer would appear to be "no". As you mention shortly after, the THIRD time you use a piece of code is the time to consider using it as a function. But I guess one of the skills involved in generating optimal code comes from knowing how to structure the sketch in such a way that you don't duplicate unneccessarily.

Building block analogy; yes, understood. The temperature example is very relevant to what I was thinking... put the elements that dont change into setup and just update the numerals when they change.

That's been most useful and very helpful. Time to go bang my head against the vertical learning curve!

Thanks again for your time.

Whether a block of code gets to be a function on it's own or whether it is part of another function is not always easy to determine.

Certainly if some block of code is used over and over, make it a function, rather than replicating the code in many places.

But, even if the code is used only once, there are benefits to creating a function anyway.

Each function should perform a single function. Read a key press on a keypad. Write the date and time to an LCD. Things that can easily be made into a function should be.

Moving a small to medium size block of code to a function shortens the function that had contained the code, and makes the logic and flow of that function easier to see and understand.

Seeing a function like ShowDateAndTimeOnLCD() being called, and one would not go looking there for the source of a bug in reading the data from a temperature sensor.

But, if all the code in that function were in loop, along with the code to read the temperature sensor, one would have to read through the code to make sure that the bug did not live (or originate) there.

Modular code comes with experience. Experience comes from writing non-modular code.

In a situation where the contents of one loop is not completely repeated, perhaps only 60% of it is repeated in a second loop. Is it better practice to redesign the structure and rewrite the code in such a way that the repeated "60%" can be utilised as a function?

In my case, the answer is usually "yes". But that is a personal choice. I REALLY do not like duplicated code. The answer to the question is really a matter of style, patience, and time.

As you mention shortly after, the THIRD time you use a piece of code is the time to consider using it as a function.

I misunderstood what you meant by the phrase "building block". I took "building block" to mean something you could use to build up ANY program (like eeprom_read_byte is so generic it can be used by any program). So, I'll rephrase what I wrote the hopes of eliminating any confusion...

In my experience, the THIRD time you use some piece of code is when you should consider turning it into a function or class that can be used in multiple programs.

Yes, I see the difference in the definition and scale of "building block" we were using. My thinking is still blinkered by a myopic focus on this one sketch, I hadn't actually considered the thought of it's strategic usefulness in other sketches in the future too.

One of the advantages of Arduino and therefore a major attraction to noobs such as me is the quantity and variety of code snippets, sketches and tutorials available. The large size of the building block of a complete sketch, is comfortable enough to get my dyspraxic fingers around!

Now that I know I can get the software and hardware to function (no pun intended). I think I'll play around with some flow charts on paper and see if I can optimise it there first...

But that is a personal choice. I REALLY do not like duplicated code.

One I can understand! I know that there are a couple of places in the sketch that could be 'better' than they are. At the moment they are minor irritations but I can see that easily evolving into a strong dislike.

But, if all the code in that function were in loop, along with the code to read the temperature sensor, one would have to read through the code to make sure that the bug did not live (or originate) there.

There was a bug I mentioned in another thread, that was a good example of that thought. It took me a while to track it down as I had to go through all of the code to find it. The function approach would certainly have helped reduce that time.

Another small step, thanks again for your time guys.