4

I have tried to implement the C++11 feature (I've used this answer as a reference Can I call a constructor from another constructor (do constructor chaining) in C++?). Obviously, I've done it wrong but I don't understand why.

I get several warnings in the following piece of code:

  • Member _output was not initialized in this constructor
  • Member _protocol_scanner was not initialized in this constructor
  • Member _state was not initialized in this constructor
  • Member _source was not initialized in this constructor

This is the code:

class UartScanner {
public:
    UartScanner(periph::IStreamDevice *source, periph::IStreamDevice *output);
    UartScanner(periph::IStreamDevice *source);
    ~UartScanner();

private:
typedef enum
{
    WAITING_SYNC,
    WAITING_UBLOX_MSG,
    WAITING_NOVATEL_MSG
} states_t;

    periph::IStreamDevice *_source;
    periph::IStreamDevice *_output;
    ProtocolScanner *_protocol_scanner;
    states_t _state;
};

UartScanner::UartScanner(periph::IStreamDevice *source, IStreamDevice *output):
    _source(source),
    _output(output),
    _state(WAITING_SYNC)
{
    _protocol_scanner = new ProtocolScanner(source,output);
}

UartScanner::UartScanner(periph::IStreamDevice *source): UartScanner(source,0) 
{
}

class IStreamDevice {
public:
    virtual ~IStreamDevice() {}
    virtual uint32_t read(uint8_t* data, uint32_t size) = 0;
    virtual uint32_t write(const uint8_t* data, uint32_t size) = 0;
};
Community
  • 1
  • 1
staroselskii
  • 365
  • 1
  • 3
  • 16
  • 1
    What compiler are you using? – arne Oct 28 '13 at 14:22
  • Don't use leading underscores in member names. Leading underscores are reserved for standard library implementation. – Felix Glas Oct 28 '13 at 14:23
  • 2
    That only applies to names in the global namespace, @Snps; leading underscores in member names, as we have here, are fine. – Rob Kennedy Oct 28 '13 at 14:25
  • 2
    @RobKennedy As long as they are followed by a digit or lowercase letter. – Angew is no longer proud of SO Oct 28 '13 at 14:27
  • @arne I'n using GCC ARM Embedded 4.7. I have to follow this naming convention. We don't use STL, so it's not a problem. – staroselskii Oct 28 '13 at 14:27
  • 2
    @RobKennedy It's allowed if the underscore is not followed by an uppercase letter, but why make it a cognitive challenge everytime? It's easier to just stay away from leading underscores. Also it's ugly and obfuscating. – Felix Glas Oct 28 '13 at 14:32
  • 1
    @Snps No, it's beautiful and helps reading the code. – lapk Oct 28 '13 at 14:37
  • 1
    @PetrBudnik It gives me headaches when reading codes with leading underscores. I keep waiting for it to disappear and be gone forever. – Kakalokia Oct 28 '13 at 14:39
  • [OT]: you may use `nullptr` instead of `0`. – Jarod42 Oct 28 '13 at 14:40
  • 4
    I appreciate that broadening the rule makes it easier to remember and follow, @Snps, but you shouldn't then repeat your broadened rule as though it's the real rule that everyone must follow. Your previous comment wrongly suggests that *all* leading-underscore names are reserved when that's not really true. – Rob Kennedy Oct 28 '13 at 14:41
  • 1
    Not reproducible here with gcc 4.7.3 or clang 3.1 (cygwin). Can you post an SSCCE? – n. m. could be an AI Oct 28 '13 at 14:41
  • 1
    @AliAlamiri Let's focus on the issue ;-) P.S. I don't like this convention, though. It's the rules I just need to follow. – staroselskii Oct 28 '13 at 14:42
  • @enter_the_bot It'd be good if you specified compiler and compilation flags... – lapk Oct 28 '13 at 14:46
  • 1
    @enter_the_bot are you actually getting compiler errors or are they from the IDE !? –  Oct 28 '13 at 14:46
  • @DieterLücking these are warnings. Not errors! I use only -Wall flag. – staroselskii Oct 28 '13 at 14:50
  • @enter_the_bot then please provide us with an [SSCCE](http://sscce.org), including full compiler settings. We will try to reproduce the warnings then. For the `_protocol_scanner` it's right, since you assign to it but don't *initialize* it in the implementing constructor. – Arne Mertz Oct 28 '13 at 14:55
  • Why not just provide `0` as a default value for the `output` parameter in your first constructor, and just eliminate the second one? – twalberg Oct 28 '13 at 15:32
  • 1
    @twalberg I wanted to use something new and get it to work! – staroselskii Oct 28 '13 at 15:43
  • @ArneMertz but _output, _source and _state are initialized. What's the deal? – staroselskii Oct 28 '13 at 22:02
  • 1
    The answer is *probably* that whatever compiler you're using doesn't yet fully implement constructor delegation. However, by continuing to ignore the multiple questions about your compilation environment, it's impossible to confirm that. – Rob Kennedy Oct 29 '13 at 06:35
  • 1
    @enter_the_bot "We don't use STL, so it's not a problem" on the contrary, "_If a program declares or defines a name in a context where it is reserved, other than as explicitly allowed by this Clause, its behavior is undefined...Each name that contains a double underscore `__` or begins with an underscore followed by an uppercase letter is reserved to the implementation for any use. Each name that begins with an underscore is reserved to the implementation for use as a name in the global namespace._" Your naming is fine because the names aren't in global NS, not because you aren't using STL. – boycy Oct 30 '13 at 09:28

1 Answers1

2

I take a look at your code and I changed a couple of things. I have created a file named Test1.hpp and put your code in it. The following code compiled correctly with GCC 4.7 and Clang 3.3 with the -Wall -Werror -pedantic-errors attributes. Let's see what's the contain of the HPP file.

#ifndef TEST1_HPP
#define TEST1_HPP
// Some inclusions here ...
namespace periph
{
   class IStreamDevice{ // Something here... };
}

class ProtocolScanner {
    public:
       ProtocolScanner(periph::IStreamDevice *source, periph::IStreamDevice *output) 
          : _source(source), _output(output) { }

    private:
       periph::IStreamDevice *_source;
       periph::IStreamDevice *_output;
};


class UartScanner {
    public:
        UartScanner(periph::IStreamDevice *source, periph::IStreamDevice *output)
          : _source(source), _output(output), _protocol_scanner(new ProtocolScanner(source,output)), _state(states_t::WAITING_SYNC) { }

        UartScanner(periph::IStreamDevice *source) 
          : UartScanner(source, nullptr) { }

        ~UartScanner() { } // I suppose that something is done in the destructor.

    private:
        enum class states_t : uint8_t {
            WAITING_SYNC,
            WAITING_UBLOX_MSG,
            WAITING_NOVATEL_MSG
        };

        periph::IStreamDevice *_source;
        periph::IStreamDevice *_output;
        ProtocolScanner *_protocol_scanner;
        states_t _state;
};

class IStreamDevice {
    public:
        virtual ~IStreamDevice() {}
        virtual uint32_t read(uint8_t* data, uint32_t size) = 0;
        virtual uint32_t write(const uint8_t* data, uint32_t size) = 0;
};

#endif

Notice that I added a namespace and some other classes you used. Since I don't know their definition, I let them empty to make that working. Now, let's review this code.

In C++11, if you want to initialize a pointer to NULL, I suggest you to use the nullptr keyword to do the job. This answer about nullptr should help you to understand.

I also replace the following code

typedef enum
{
    WAITING_SYNC,
    WAITING_UBLOX_MSG,
    WAITING_NOVATEL_MSG
} states_t;

By the this one (strongly typed enum since C++11)

enum class states_t : uint8_t {
    WAITING_SYNC,
    WAITING_UBLOX_MSG,
    WAITING_NOVATEL_MSG
};

A short explanation of the enum class

About the delegating constructor, it seems to be correct. It should be used only in constructor initializer list like you did. If this was the problem, maybe the compiler you use doesn't let you use the delegating constructor. If you are using Visual C++, this answer should help you. With GCC 4.7 and higher and clang 3.1 and higher, I'm sure it's working.

Hope that helps you.

Community
  • 1
  • 1
Gabriel L.
  • 4,678
  • 5
  • 25
  • 34