0

I've been working on some embedded code that handles a Bluetooth Low Energy (BLE) radio. BLE has 40 channels, numbered 0 through 39. One function for the radio driver takes in the channel and sets the appropriate register with the correct setting.

The obvious function prototype was SetChannel(uint8_t channel);. However, uint8_t can hold numbers from 0 to 255, but BLE only has 40 channels. Any channel over 39 is invalid.

Which brings up my question: How should one handle a variable that's clearly a digit, but has a range less than the storage type?

I thought of using an enum, like this:

typdef enum channels
{
   Channel0,
   Channel1,
   Channel2,
   Channel3,
   ...
   Channel39
}BleChannel;

thus making the function prototype SetChannel(BleChannel channel);, but that just doesn't feel right when the type is clearly a number, but with a limited range. Throwing an exception or otherwise terminating execution is not a great option due to this being an embedded device.

closed as too broad by gnat, ChrisF Jul 27 '16 at 21:04

Please edit the question to limit it to a specific problem with enough detail to identify an adequate answer. Avoid asking multiple distinct questions at once. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    On second thought, stick with enums. Principle of least astonishment. – Dan Pichelman Jul 27 '16 at 14:35
  • 1
    I would only use values Chanel0, Chanel1, Chanel2 etc. if the code was related to perfumes, not Bluetooth. – gnasher729 Jul 27 '16 at 14:36
  • 2
    It's a number. Use uint8_t channel, and treat numbers above 40 as an error. – Robert Harvey Jul 27 '16 at 14:53
  • 3
    It's too bad C++ and most of today's popular languages don't offer subranges. Pascal has subranges, so you can just declare a type for 0..39. – Erik Eidt Jul 27 '16 at 14:54
  • 3
    @CHendrix: My point is: you say that the data is "clearly" numerical, but that is by no means clear to me. Look at it this way: suppose we renamed the forty channels "purple, square, manager, boxcar, ... , fish, soup", because it is easier to say "use channel soup". Would you then have to encode somewhere in your program that "purple plus two is manager"? If not, then the quantities are not actually numbers in the first place. They've just been given numbers arbitrarily out of convenience. But if you need to be able to say "channel 24 is four above channel 20", then they are numbers. – Eric Lippert Jul 27 '16 at 15:27
5

Numbers with limited range arise often enough that (IMO) it's worthwhile adding a type specifically to handle them. I've used something on this order quite a few times:

template <class T, T lower, T upper>
class bounded { 
    T val;
    void assure_range(T v) {
        if ( v < lower || upper <= v)
            throw std::range_error("Value out of range");
    }
public:
    bounded &operator=(T v) { 
        assure_range(v);
        val = v;
        return *this;
    }

    bounded(T const &v=T()) {
        assure_range(v);
        val = v;
    }

    operator T() { return val; }
};

You'd then do something on the order of:

using channel = bounded<uint8_t, 0, 39>;

If you just pass an int8_t (or whatever), you stand a good chance of ending up with two problems. One is that there's some route through the code where the value is used without its range being checked. The other is that you waste time repeatedly checking the range on the same value. Worse, if the code base is large at all, you can quite easily end up with both problems at the same time.

All the usual benefits of DRY apply here. By centralizing the range checking in one place, it's easy to assure that what we have is correct, easy to change in the future if needed, easy to be certain1 that ranges are checked everywhere they need to be, without adding unnecessary checks of values that have already been checked. Most of all, of course, we get readable code, so what should have been a one-line functions don't end up with the real code buried somewhere under a layer of range checking that's irrelevant to the job at hand.

As far as using exception handling vs. not goes, the wording in the question ("Throwing an exception or otherwise terminating execution...") indicates that the OP has a mistaken belief about exceptions--that they lead to terminating the program. That's most assuredly not the case at all. An exception can terminate a program if it's never caught, but throwing and catching an exception can simply transfer the flow of execution from the code that detects a problem to code that knows how to handle that problem (without any intervening layers having to be aware of it at all, beyond being exception neutral). In short, this is exactly the sort of situation for which exception handling is designed, and absent a verifiable reason that it really can't be used here, it's clearly the right tool for the job.

It is, of course, possible that you always handle such an error in a single, specific way, or that you have a limited range of options that are all known at compile time. In the latter case, you can handle the error via a strategy class:

template <class T, T lower, T upper, class error> {

    // ...
    if ( v < lower || upper <= v)
        error("Value out of range");

Even if the released device only ever handles this error in one specific way (e.g., uses one specific channel) it can be worthwhile to use a template parameter rather than hard-coding the error handling directly into the code that detects the problem. There are at least a couple of obvious advantages:

  1. You might change the one and only way of handling the problem.
  2. It's easy to change the behavior during testing so such problems cause an immediate break to the debugger or at least log the problem.

1. Obviously here I'm discounting the possibility of somebody doing something like this:

uint8_t bad_channel = 127;
channel &c = *reinterpret_cast<channel *>(&bad_channel);

Given how C++ is defined, it's nearly impossible to keep people from shooting themselves in the feet if they try hard enough though. At the same time, reinterpret_cast (or an equivalent C-style cast) should always be examined with extreme care--it can be used to bypass almost any type-based assurances you try to give yourself.

  • Finally, a little sense among other garbage answers. Using c++ without exceptions is just stupid. – BЈовић Jul 28 '16 at 7:55
  • +1 this is an excellent answer. While I think that adding special code to restrict the range of the type for this particular value is over-design, a general type to do this is very elegant and makes a lot of sense. And once it exists, why not apply it in this case? You have changed my mind about the best approach here. Also, good discussion of exceptions. – user82096 Jul 28 '16 at 8:48
3

Don't over-think this.

  • Use an integer.
  • Test your code appropriately.

Now move on to the parts of the design that matter.

With all due respect, this is a very simple piece of code that seems incredibly unlikely to result in any bugs. There is going to be some code that sets the channel, or increments or decrements it, and this code needs to only allow valid channels. That is very simple to implement and you are going to do it correctly (given that you are clearly conscientious about this). And assuming you have some reasonable testing in place, this will result in some very obvious test cases that would catch any problems.

It's the things you haven't thought of that are going to cause bugs, not this.

1

If it's a number with a limited range, treat it as a number. Make the type of the channel parameter int. Since you can't express the valid range by type, it doesn't make sense to even try. If passing any value other than 0 to 39 is a programming bug, add an assert in your development code so that it gets caught (passing int has the advantage that passing 256 will also be caught as an error); in the shipping product probably best to replace any invalid value with a valid one or rebooting; your job is to determine what is likely to cause less damage.

  • 3
    I have to disagree on nearly all counts. First and foremost, as my answer shows, you most certainly can represent the valid range by type. Essentially all your other arguments are based on that, rendering all invalid. The only part that's open to question is the top level, so to speak: what to do when/if something does receive an incorrect channel number (and since we don't know that, the proper answer is to throw an exception, so something at the proper level can make that determination). – Jerry Coffin Jul 27 '16 at 16:40
1

On the assumption that your code can be tested via an automated test framework on your development machine and that the code can be structured such that the SetChannel function can be mocked/swapped during tests, I'd take the following approach:

  1. Use the SetChannel(uint8_t channel) signature.
  2. Do no defensive coding in that function; if eg 40 is passed in, then let the device fail, because,
  3. Mock the SetChannel in your tests of the rest of the system with one that causes the test to fail if passed a number > 39 and then thoroughly test the rest of the code to ensure 40+ is never passed to the function.

By ensuring any calls to SetChannel behave themselves and only pass a valid channel number, the need to worry about out of range numbers goes away.

  • 1
    Despite the downvotes on this answer, David has a point. This is an embedded device, so unless the channel numbers are coming from some outside source, this really ought to work properly without validation. – Robert Harvey Jul 27 '16 at 15:44
  • 1
    I agree with this answer. There is no reason to constantly double check that you yourself are doing things correctly, and testing should easily cover this completely. Apart from that, it's not clear that the failure mode for the defensive approach on an embedded device would be any better than the failure for a non-defensive approach. – user82096 Jul 27 '16 at 16:02
  • I would prefer int versus uint8_t. To me, uint8_t implies that the width of the variable is important but that's not the case here. – kkrambo Jul 27 '16 at 17:25
  • @kkrambo, very good point. – David Arno Jul 27 '16 at 17:27
0

I don't think an enum is ideal for two reasons:

  • It doesn't actually enforce the validity
  • Unless you're hardcoding channel numbers in your source code, it doesn't really give you any safety compared to an integer. You still need to parse user input into a channel identifier, which needs to handle invalid inputs anyways.

I'd consider a custom class which

  • Has a function to construct it given an integer and/or string. This should be either an explicit constructor or a static method. Probably a static method, since that way you can return an error instead of throwing an exception.
  • A way to enumerate all valid channels. This can be used to display the channels to select from in a UI (if you have one).
  • The biggest advantage of this approach isn't that it enforces the range, but it makes accidentally passing some unrelated int as a channel identifier a compile time error. Since you don't need to do arithmetic on on the channel, the custom class doesn't need much boilerplate code either.
  • That immutable object can have readonly properties for e.g. frequency.

Or just use an int or even string:

  • It's the simplest approach and the alternatives don't buy much safety in practice.
  • If different devices support a different number of channels, you can only check the validity when you associate it with a device.
  • Or just use an int or string. << this. – Robert Harvey Jul 27 '16 at 15:16
  • @RobertHarvey Personally I'd probably go with a minimal custom class, just because it makes passing some other kind of integer a compile time error and not because it enforces the range. – CodesInChaos Jul 27 '16 at 15:22
  • It's not a compile-time problem. – Robert Harvey Jul 27 '16 at 15:28
  • @RobertHarvey Passing e.g. a device ID or a frequency in a place where the code expects a channel ID looks like a compile-time problem to me. – CodesInChaos Jul 27 '16 at 15:29
  • 1
    This isn't rocket science. The folks who design radios don't think about things like this; they just hook it up and make it work. They think all of our hand wringing about compile time errors is ridiculous (which it is). Imagine a police scanner throwing an exception. – Robert Harvey Jul 27 '16 at 15:31
-2

I would go with this:

void SetChannel( const int channel );

then check the channel's value. If it is out of range, throw an exception.

Having an enum in this case is an awful solution, since it brings nothing into the code clarity.

  • Perfectly fine solution. Wait, it's exactly what I posted! Somebody doesn't get it. – gnasher729 Jul 27 '16 at 14:42
  • "Throwing an exception or otherwise terminating execution is not a great option due to this being an embedded device" – David Arno Jul 27 '16 at 14:44
  • @gnasher729 I posted 5 minutes ago, you 3. Therefore you posted exactly what I posted. – BЈовић Jul 27 '16 at 14:45
  • @DavidArno Are you kidding? I am programming embedded devices last 15 years, and been using them all the time. Even on a DSP having 32k memory. – BЈовић Jul 27 '16 at 14:46
  • 2
    @BЈовић, just quoting the OP. I'm not passing judgement on whether throwing exceptions on embedded devices is a good or bad thing. The OP explicitly stated they didn't want to do that though. – David Arno Jul 27 '16 at 14:48

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