1

How can I avoid the fact that not all control paths return a Container here:

        enum Type {Int, String};

        Container containerFactory(Type type)
        {
            switch(type)
            {
            case Int:
                return Container (std::vector<int>());
            case String: 
                return Container (std::vector<std::string>());
            }
        }

UPDATE: I was thinking that I could cast an exception here but I can't unit test this as it isn't possible to pass an invalid Type to the function.

Baz
  • 12,713
  • 38
  • 145
  • 268
  • Return an empty container? Maybe some kind of specialized version which indicates that there has been an error/no result? Without the declaration of `Container` it's a little bit difficult to tell. – Zeta Oct 08 '12 at 11:44
  • 1
    I would always return a *container or null, but maybe that's just me:( – Martin James Oct 08 '12 at 11:49
  • Side note: You don't need the double parentheses or the `break` statements. – Marcelo Cantos Oct 08 '12 at 11:51
  • @Martin James I would then need to change a lot of my code to work with boost::shared_ptr instead of Container and Container&. – Baz Oct 08 '12 at 11:53
  • @MarceloCantos - maybe some code-completion has put them in. – Martin James Oct 08 '12 at 11:53

3 Answers3

3

I tend to add an assert() at the bottom:

Container containerFactory(Type type)
{
    // ...
    assert( !"Unreachable code hit!" );
    return Container();
}
Frerich Raabe
  • 90,689
  • 19
  • 115
  • 207
  • It doesn't need to be unit tested; the function signature clearly states that only two values are possible (`Int` and `String`), so those are the function you need to test. – Frerich Raabe Oct 08 '12 at 12:01
2

Obviously, passing an invalid value is a programming error. In "my world"/rulebook/whatever, programming errors cause assertion failures, not exceptions.

EDIT: for this reason, I actually have at least two main assert statements: one that gets removed in a release build, and one that doesn't. Most end up staying in the code in case a bug gets triggered, but sometimes an assert can be too expensive to be worth doing it in the release version.

If your compiler doesn't recognize an assert as "something that doesn't come back", you can return whatever you want to shut it up. There is no "good return" anyway unless you want to add error checking to the call interface, as you don't know what type of container the caller expects to get from his "759843682" uninitialized request :-)

Christian Stieber
  • 9,954
  • 24
  • 23
  • [This answer](http://stackoverflow.com/a/2924154/91757) explains that `std::logic_error` is used to signal cases in which the programmer screwed up, too. – Frerich Raabe Oct 08 '12 at 11:55
  • Yes, your right. I was thinking that the exception would at least get rid of the compiler error but assert( !"Unreachable code hit!" ); followed by return Container(); makes much more sense! – Baz Oct 08 '12 at 11:56
1

Either throw, default to returning one of the Enum types or return a null_ptr.

Konstantin Dinev
  • 34,219
  • 14
  • 75
  • 100