0

I am working on algorithm implemented in C++ that maintains several enum types. Say 3 to 4 enum types each with at least 4 different values. Plus the code maintains several boolean variables. The code is multi-threaded. Across different threads, it uses the state of the algorithm indicated by these enum variables and boolean values to drive code flow. This led to following:

  1. Code is littered with nested if-else blocks of depth 2 to 5 checking state and doing whats required to be done.
  2. The if-else-if conditions are composite involving ANDing / ORing these enum and boolean variables.
  3. Many functions have a check for these states for deciding whether it should proceed to do what they indicate to do by their names.

I guess this made the code somewhat less intuitive, bug prone and high maitenance. I know there are several threads discussing that using boolean variables for determining code behavior isnt a good idea. (For example, this and this.)

But, now I have following doubts:

Q1. Whether we use enum or boolean variables, above three points stays. So, is using enum variables also bad?
Q2. If the approach to check variable values for determining code flow itself is a bad idea, what is good idea / how should we approach this problem?

PS: Will love to have some before and after code snippets explaining solutions / discussion, if possible.

5
  • You've tagged your post embedded-systems, for which it is common to restrict the set of C++ features used (often, "no dynamic memory allocation"). Do you have any such restrictions in your codebase? Apr 7 at 10:06
  • 1
    can you give us some "before" snippets so we know what you mean?
    – Ewan
    Apr 7 at 10:32
  • 1
    Does this answer your question? Style for control flow with validation checks (tl;dr - use specification pattern)
    – gnat
    Apr 7 at 10:54
  • 3
    I think you get better answers if you post snippets to codereview.stackexchange.com and ask the experts there how to improve the code.
    – Doc Brown
    Apr 7 at 11:44
  • Have you looked into state machines? They encapsulate behaviour like this into state classes that can handle the conditions and transitions from one state to another.
    – Adam B
    Apr 10 at 15:51

2 Answers 2

4

You are misunderstanding things. Booleans are there to control code behaviour. What’s bad is calling a method with three arguments f(true, true, false) with no indication what each argument means without looking at documentation. (You remember there is an argument making an object visible. Or invisible. And you can’t remember if it’s the first, second or third argument).

You handle complex conditions by writing a function which takes an enum argument and returns exactly what you want to know.

The best way is to have enums represent the original facts and have functions that determine behaviour from the facts. Say you are selling goods in different categories and need to charge VAT according to the category. Which category an item belongs to is a fact (enum), which VAT is paid depends on the enum, and you have one method for that.

And please please please don’t replace enums with integer constants or strings because someone told you enums are bad. Fake enums are always, always worse.

2

Your question is vague, because we can only guess around how your code might look like, but I think it is still possible to answer your two questions directly:

Q1: Whether we use enum or boolean variables, above three points stays. So, is using enum variables also bad?

Using boolean values or enum values for control flow isn't good or bad per se. Only when it leads to code which is hard to understand or hard to maintain, both techniques can be "equally bad". Still, state expressed in enum variables give you some means to sometimes express certain state combinations in a more explicit (and hence more readable) fashion than a bunch of boolean values. This boils down to

  • whether your enum creates a better abstraction than a simple boolean

  • whether you can come up with more expressive names for the different enum constants, more expressive than just "true" or "false" or a combination of multiple true/false values.

In one of the posts you linked to is an example for the latter.

Q2. If the approach to check variable values for determining code flow itself is a bad idea, what is good idea / how should we approach this problem?

The approach is only a bad idea when the code becomes badly readable from it. This can have different root causes:

  • as I wrote above, bad naming. Solution: choose better, more expressive names. Sometimes you just need to rename the variable, sometimes the enum constants, sometimes the enum.

  • having the same kind of testing logic repeated or distributed over several places. Solution: centralize the logic. Maybe in a function, maybe in an object (see gnat's answer here), maybe by replacing some conditional by polymorphism

  • the state variables are global variables (you wrote something about shared state between threads, so I guess this could be your case). Solution: refactor your code to smaller classes or components with explicit interfaces, so the necessary state, or necessary state query functions are passed into at construction time, or as parameters to the functions which require them. This may lead to the technique of Dependency Injection.

These are just a few examples. IMHO there is no simple set of two or three rules which can be applied to improve your code without knowing the code. If you want to learn several techniques to deal with boolean or enum values in a more maintainable way, I would recommend to get a copy of Fowler's refactoring book and identify the refactorings from it's catalog which apply to your cases.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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