3

I am in the process of cleaning up my code and making it easier to maintain. I am doing this by turning my 5000+ line file into separate smaller files.

I have successfully created separate source and header files for obvious things like utilities and other components that are not tightly integrated with the application.

I am now thinking it would be good to break up the rest of my application into smaller files also. But the issue mostly concerning me is that everything that is left uses a bunch of Structs that are Globally declared.

So this is the choice moving forward (happy to hear other suggestions also):

1) I could leave everything in the same file and that way I could just define all functions as static (which has its benefits, and is often recommended). It is then a simple choice to leave the Globals at the top of this large source file.

OR

2) I could separate into smaller files. It is relatively simple to work out which parts should go into which new files. It is also not too difficult to work out the structural hierarchy (which parts call which parts). But I am uncertain on how best to deal with all these Globals. Where should they be placed, since every function uses them in some way?

NOTE: I am aware that the use of Globals should be avoided. However this is an embedded device with limited memory. Globals are the best way to reduce memory usage, and the more memory I save, the more I can perform a certain task.

  • 1
    Global state is usually considered a design smell, especially once you consider things such as multithreaded usage of your code. It is often helpful to pass a context parameter to your functions, which has a similar role to objects in OOP programming, in that it contains the (not any longer global) state of your code. – amon Sep 29 '14 at 11:21
  • 3
    It is C, not C++...in other words it is not an OOP. Also this is an application on a embedded device with limited memory. Globals are the best way to use as little memory as possible. The more memory I save, the more I can perform a certain task. But thanks for your input. – Remixed123 Sep 29 '14 at 11:37
  • 3
    The context parameter @amon suggested can be held in a global if you want. Passing it in as a parameter is still a good idea because it makes the dependencies explicit, allows you to test the code, and future-proofs it in case later on there's more than one context. – Doval Sep 29 '14 at 15:46
  • I'm not sure that splitting that 5000 lines of C file into several files is a good idea. I often have such large files. – Basile Starynkevitch Oct 1 '14 at 13:07
3

I would recommend typedef-ing all the structs, and putting those declarations in a separate include file. Then define all your actual instances of the structs in whatever your "main" file ends up being. Then, simply include the header and declare the needed instances extern in the other files. The linker will take care of matching up all the references, it shouldn't be any problem at all.

  • This looks like a good option to create separate files and then have them all use the same "Globals". According to this post - stackoverflow.com/questions/496448/… - Using extern with variables will not allocate new memory, but with functions it will allocate new memory. Also using the function method requires a lot of rewriting and extra code, due to the way my application uses the variables. – Remixed123 Sep 30 '14 at 3:49
  • Verified that this method works...using the details provided in this post - stackoverflow.com/questions/496448/… – Remixed123 Sep 30 '14 at 6:45
5

There is a third, hybrid alternative, which is to put each global into its own module and write functions to support it. This gives you the OOP-y benefit of all code relating to the variable being isolated in one place and much easier to re-use elsewhere if needed. Putting the support functions in the header so they can be inlined instead of forcing calls gives you the benefits of having everything in one big file.

For example:

// foo.h

typedef int FooType;

extern FooType foo_global_do_not_use_directly;

static inline void foo_init() {
    foo_global_do_not_use_directly = 0;
}

static inline void foo_set(FooType new_foo) {
    foo_global_do_not_use_directly = new_foo;
}

static inline FooType foo() {
    return foo_global_do_not_use_directly;
}

.

// foo.c

// Always include the header even if you're not using a
// typedef so it gets compiled at least once.
#include "foo.h"

FooType foo_global_do_not_use_directly;

You will, of course, have to go through the rest of your source and adjust every use of your globals to use the functions instead of referring to the symbols directly. In the process, you may find other things that are done repeatedly and deserve to be their own functions, which may have benefits down the road if you ever need to change how those operations work.

  • Thanks. I think I need to review this more as I am not seeing the advantage over what I currently have. Which is some very nicely structured Structs that use clearly communicated Enums when required. – Remixed123 Sep 29 '14 at 13:09
  • @Remixed123: One other advantage you get out of this is the ability to unit test each piece outside the big blob you have now. – Blrfl Sep 29 '14 at 13:26
  • Thanks, I am now understanding how this works. Hmmm, will this use extra memory? I think it will use a lot more memory. As the functions you suggest will require memory to be allocated for the variables inside them. I will then also have variables in each of my functions which will have their own memory allocated. Unfortunately there would be no memory left in my application if I did it this way. – Remixed123 Sep 29 '14 at 13:34
  • @Remixed123: Where do you see space for variables being allocated other than the single instance in the .c file? – Blrfl Sep 29 '14 at 14:20
  • That is my question in my choice 2 above. How should I create separate files, with each of these separate files all using the same globals. I was thinking one way would be to create a separate file where I place all my globals, then include that in each of my other files. Of course these globals could not be static, but this may be less of a concern than having one large file. I am looking for opinions and suggestions on how to move forward, or perhaps to leave things as they are. – Remixed123 Sep 29 '14 at 14:35
1

I'd go with your Option 1: leave the globals in a single file where it's easy to find them.

As you've noted in the comments, this is not OOP. The "rules of OOP" (while very good in the general sense) don't necessarily apply to a smallish project on an embedded device.

5000 lines in one file is a pain to work with. 5000 lines in a solution is pretty small. If you've already moved the other stuff out, chances are what's left isn't all that big. If you split it up you'll just spend a lot of time searching for the one global you want to change.

Remember, the goal is to keep the code organized so you can clearly follow the structure, make needed changes (and understand their impact) and separate "stuff that changes" from "stuff that doesn't".

OOP is the common way of meeting that goal on modern general purpose computers. It's not the only way to do things, and it may not be the best way on a memory-limited embedded device.

  • Thanks. I should have been clearer, it is 5000+ lines after I have performed the refactoring. But still, 5000 is not that large. And yes, the rules and compromises required in embedded programming can be quite different. – Remixed123 Sep 29 '14 at 14:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.