Pages: [1] 2   Go Down
Author Topic: How to avoid code duplication in a function?  (Read 1258 times)
0 Members and 1 Guest are viewing this topic.
Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hey guys,

I've got a state machine in my code (a big switch statement), and in that state machine in several states I need to check to see if a switch has been flipped, and perform an action.. namely light an LED, play sound, and change a mod variable.  This check is always the same.  So I end up with duplicate code.

In other programming languages I've used, I'd just declare a function within a function, and that function would have access to all the parent function's variables, but I can't do that in C.

I've been told using #defines is bad if you can avoid it, and honestly after having to spend hours debugging one multi-line assembly function because whoever wrote this stupid language decided it was okay for invisible spaces at the end of a line to cause obscure compiler errors, I would tend to agree with that assessment, so I can't use those to fake my function.

I also can't use an inline function, because although the code in those is supposed to just replace the function call, the compiler can decide not to do that which means trying to call variables local to the function holding my switch statement is a no-no.

The only solution that I've come up with so far then is to make my mode variable and my mode constants global, so I can just make a second function and be done with it.  And I should probably just do that instead of wasting time trying to get rid of globals since I already have so many, but using globals everywhere just isn't good coding practice.   So I'm looking for a solution which avoids that.

So any suggestions?

TLDR;  I've got a function with a switch statement inside of it, and inside of that I have duplicated code that I want to reduce down to a single function call without resorting to global variables for the variables said code needs to use.  And I don't want to use #define.  And inline won't work.
Logged

Global Moderator
Dallas
Offline Offline
Shannon Member
*****
Karma: 200
Posts: 12773
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
The only solution that I've come up with so far then is to make my mode variable and my mode constants global, so I can just make a second function and be done with it.  And I should probably just do that instead of wasting time trying to get rid of globals since I already have so many, but using globals everywhere just isn't good coding practice.   So I'm looking for a solution which avoids that.

It's important to remember why "using globals everywhere just isn't good coding practice".  Essentially, the point is to bring data and the corresponding code together in one place.   We're talking about an Arduino sketch so, for the most part, the data and code are already together.  In other words, within an Arduino sketch, is there any practical difference between...

Code:
static uint8_t mode;

void loop( void )
{
  switch ( mode )
  {
...
  }
}

...and...

Code:
void loop( void )
{
  static uint8_t mode;

  switch ( mode )
  {
...
  }
}


Is one going to be easier or harder to debug?  If mode is not the expected value, do you have to search through dozens or hundreds of source files where mode is referenced?  Is it possible for code outside of the sketch to modify mode in unexpected ways?
Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 474
Posts: 18696
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
So any suggestions?

Function objects:

http://www.gammon.com.au/forum/?id=4181
Logged

SE USA
Offline Offline
Faraday Member
**
Karma: 41
Posts: 3783
@ssh0le
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
I'd just declare a function within a function

burn it with fire! this is a bad idea even if a language allows it as its very easy to never exit a function and return to normal eating up ram stupid fast

sounds like a case of organization, nick has a darn good suggestion though


« Last Edit: December 30, 2012, 03:34:59 am by Osgeld » Logged


Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
I'd just declare a function within a function

burn it with fire! this is a bad idea even if a language allows it as its very easy to never exit a function and return to normal eating up ram

What are you talking about?  How does having a function within a function make it any easier to have a bug which causes a function to never exit?  And if your program is stuck in a single function, who cares how much ram it's using?  That's the least of your worries.

(Maybe if you're coding an app for a multitasking OS it matters somehow, but one would also expect in that case that the OS would keep the app in its own walled garden and when the app locks up the user would close it.)
Logged

North Queensland, Australia
Offline Offline
Edison Member
*
Karma: 65
Posts: 2110
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
I also can't use an inline function, because although the code in those is supposed to just replace the function call, the compiler can decide not to do that which means trying to call variables local to the function holding my switch statement is a no-no.

You are also using GCC in the Arduino environment, so you can use GCC function attribute 'always inline':

'__attribute__( ( always_inline ) )'

You can also combine it with inline:

Code:
#define _INLINE_ __attribute__( ( always_inline ) ) inline
#define _NO_INLINE_ __attribute__( ( noinline ) )

_INLINE_ void MyFuncA(){
  return;
}

_NO_INLINE_ void MyFuncB(){
  return;
}




Logged


Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

It's important to remember why "using globals everywhere just isn't good coding practice".  Essentially, the point is to bring data and the corresponding code together in one place.   We're talking about an Arduino sketch so, for the most part, the data and code are already together.  In other words, within an Arduino sketch, is there any practical difference between...

Is one going to be easier or harder to debug?  If mode is not the expected value, do you have to search through dozens or hundreds of source files where mode is referenced?  Is it possible for code outside of the sketch to modify mode in unexpected ways?

Is one going to be harder to debug?  I would say the answer is yes.  In which case I'm not sure what point you're trying to make here.  Are you assuming my project is small?  It is not.  I use several large libraries and have a couple thousand lines of code myself.  Is it likely some other lib uses a global mode variable and modifies it?  Probably not.  Would the compiler throw an error even if it did?  Maybe.

Really, the reason I do this is not because I think I need to, but because it would be embarrassing to have other coders look at my code and see I've used globals everywhere. smiley
Logged

SE USA
Offline Offline
Faraday Member
**
Karma: 41
Posts: 3783
@ssh0le
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

depends on the language lua for example

Code:
run = function()
    letsDoSomething()
end

letsDoSomething = function()
    stuff
    -- we are done lets loop
    run()
end

run runs a function and waits for it to end, that function runs run and waits for it to end, run runs a function and waits for it to end, that function runs run and waits for it to end

8 seconds later your program puked as it ate all available ram, hell I have seen situations like that that could suck 32 megs of system ram (on a 32 meg system) faster than you can blink... its bad mojo

Quote
I've used globals everywhere.
I just use a global array, its clean and kiss
« Last Edit: December 30, 2012, 03:56:55 am by Osgeld » Logged


Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
I also can't use an inline function, because although the code in those is supposed to just replace the function call, the compiler can decide not to do that which means trying to call variables local to the function holding my switch statement is a no-no.

You are also using GCC in the Arduino environment, so you can use GCC function attribute 'always inline':

I would prefer not to use compiler specific things.  I'm trying to get into good coding practices in C, not GCC specifically.  
Logged

Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

depends on the language lua for example

Code:
run = function()
    letsDoSomething()
end

letsDoSomething = function()
    stuff
    -- we are done lets loop
    run()
end

run runs a function and waits for it to end, that function runs run and waits for it to end, run runs a function and waits for it to end, that function runs run and waits for it to end

8 seconds later your program puked as it ate all available ram, hell I have seen situations like that that could suck 32 megs of system ram (on a 32 meg system) faster than you can blink... its bad mojo

That's called recursion, and it's very useful.  I wrote a sprite system a few years ago which allowed you to make one sprite a child of another sprite, and using recursion the system was able to handle all the transformations from one space to another, so you could position, scale, or rotate the parent and all the children would animate with it exactly how you would expect.

As for your example there, if you do that and you didn't intend for your function to be recursive and you don't have an exit condition, then you deserve what you get. :-)
Logged

SE USA
Offline Offline
Faraday Member
**
Karma: 41
Posts: 3783
@ssh0le
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

that's ghetto recursion and since you got it figured out I will leave you to it

one last time ... organization, my current project is porting 3 java programs and a disk emulator off of SD for an apple II to avr, its 90% done with 3k of program memory and a couple hundred bytes of ram free at its peek usage

I have 2 global arrays, a handful of #defines as constants, and zero functions calling functions, I am sure you can manage in your project
Logged


Global Moderator
Dallas
Offline Offline
Shannon Member
*****
Karma: 200
Posts: 12773
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Is one going to be harder to debug?  I would say the answer is yes.

How so?  In one case mode is function scope and the other its module scope.  Do you have multiple "mode" variables in the module in question?

Quote
In which case I'm not sure what point you're trying to make here.

There are valid reasons to broaden the scope of data.  From your description, the data cries out to be module scope.

Or, you can bite the bullet and make your code much more object-oriented.

Quote
Are you assuming my project is small?  It is not.

You have an interesting definition of "not small".  We are talking about an AVR processor.   smiley-wink

Quote
Is it likely some other lib uses a global mode variable and modifies it?  Probably not.  Would the compiler throw an error even if it did?  Maybe.

A problem easily avoided by putting "static" in front of mode.

Quote
Really, the reason I do this is not because I think I need to, but because it would be embarrassing to have other coders look at my code and see I've used globals everywhere. smiley

Limiting scope is always a good thing.  But, limit it too much and you'll be unnecessarily pulling your hair out.
Logged

New Jersey
Offline Offline
Faraday Member
**
Karma: 65
Posts: 3638
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I'm clearly missing something here; why not pass the relevant variables by reference?
Logged

Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

How so?  In one case mode is function scope and the other its module scope.  Do you have multiple "mode" variables in the module in question?

No, but a library I link in later might have a global variable called "mode" which conflicts with my own.  That's one reason to avoid using globals.


Quote
There are valid reasons to broaden the scope of data.  From your description, the data cries out to be module scope.

The device I'm programming is a light and sound effects board.  My main program is skeleton code which will be used in every case.  I then have a specific function, we'll call it statemachine(), which is different for every project.  That code runs in the main loop, uses the timing code from the main loop, uses the inputs read in the main loop, and then does whatever it wants to do with that data.

My goal is to get the code to a state where the only function you need to change is that one statemachine() function, and maybe a few helper function to keep the code neat.  But having a bunch of globals in the main program that have to be altered as well is a no-no.  I don't want myself or the end user to have to alter the main program.  That makes a mess of things.

Since this mode variable is specific to the project, I don't want it mixed in with the other global variables.

I guess the ideal way to handle this would be to have each project be it's own library so it can have its own globals, but I ran into issues with that.  I don't remember what those issues were, but I don't have time right now to muck about with it and try to shove it all into a separate library again.    

Anyway that's why I don't want to make the variable global.

Quote
You have an interesting definition of "not small".  We are talking about an AVR processor.   smiley-wink

I meant relative to your typical Arduino project that blinks a few LEDs. Also, I've got a 1284P in my board so I do have a little more program space and ram to work with than is typical. smiley


Quote
Is it likely some other lib uses a global mode variable and modifies it?  Probably not.  Would the compiler throw an error even if it did?  Maybe.

Quote
A problem easily avoided by putting "static" in front of mode.

I don't understand.  Static on a global variable?
Logged

Manchester, New Hampshire
Offline Offline
Edison Member
*
Karma: 4
Posts: 1359
Propmaker
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I'm clearly missing something here; why not pass the relevant variables by reference?

I suppose I could use a function parameter, but that doesn't allow me to access the constants used to define the different modes.  Also it's not a particularly pretty solution if I have several variables I need to access.
Logged

Pages: [1] 2   Go Up
Jump to: