GPIO wrapper class

I've received reviews on the last version here, but as an update would make the answers not fitting anymore, I think it is wise to open a new post.

I followed most of the hints of the last review and now want to check if I done things write or made new mistakes.

If you are interested in the complete source click here

gpio.h

#ifndef SRC_GPIO_H_
#define SRC_GPIO_H_

#include 
#include 
#include 

using std::string;
using std::fstream;

enum GPIODirection {
    GPIO_IN  = 0,
    GPIO_OUT = 1
};

enum GPIOValue {
    GPIO_LOW  = 0,
    GPIO_HIGH = 1
};

class GPIO {
    public:
        explicit GPIO(int id);
        ~GPIO();

        int  Value();
        void Value(int value);
        int  Direction();
        void Direction(int value);

    private:
        int id_;

        fstream value_;
        fstream direction_;

        bool Exists();
        void Export();
        void Unexport();

        static const string PATH_EXPORT;
        static const string PATH_UNEXPORT;
        static const string PREFIX;
        static const string POSTFIX_VALUE;
        static const string POSTFIX_DIRECTION;
};

#endif

gpio.cc

```
#include "gpio.h"

#include
#include
#include
#include
#include
#include

using std::ios;
using std::endl;
using std::string;
using std::stringstream;
using std::logic_error;
using std::runtime_error;

const string GPIO::PATH_EXPORT = "/sys/class/gpio/export";
const string GPIO::PATH_UNEXPORT = "/sys/class/gpio/unexport";
const string GPIO::PREFIX = "/sys/class/gpio/gpio";
const string GPIO::POSTFIX_VALUE = "/value";
const string GPIO::POSTFIX_DIRECTION = "/direction";

GPIO::GPIO(int id) {
id_ = id;

Export();

stringstream value_path;
stringstream direction_path;

value_path > value;

if (value == "0") return GPIO_LOW;
if (value == "1") return GPIO_HIGH;

throw logic_error("Invalid GPIO value.");
}

void
GPIO::Value(int value) {
value_.seekp(0);

switch (value) {

Some quick remarks:

-
To me, value and direction don't sound like stream names. Maybe they should be called something else? It depends on the domain terminology (which I'm not familiar with).

-
Don't ever pollute the global namespace by using using in headers. Write the full qualifiers.

-
Your stringstream operations look like they can be factored out into a separate function.

-
Note that std::endl forces a stream flush. If you just want the newline, use

-
I think it's nice to sort #includes alphabetically.

-
In C++11, fstream::open takes a std::string argument as well.

-
Consider changing the names of Value and Direction (like William Morris points out in the other review). If a function changes the state of the object, it should be named differently (probably with a verb).

-
Prefer to initialize variables using initializer lists in constructors.

-
Your Value and Direction functions are nearly identical.

You can refactor to something like this:

std::string GPIO::ReadFromBeginningOfStream(std::fstream& stream)
{
    stream.seekg(0); // NOTE: seekg, *not* seekp
    std::string tmp;
    stream >> tmp;
    return tmp;
}

int
GPIO::Value() {
    std::string const& value = ReadFromBeginningOfStream(value_);

    if (value == "0") return GPIO_LOW;
    if (value == "1") return GPIO_HIGH;

    throw logic_error("Invalid GPIO value.");
}

And then similar but with passing in the other stream in Direction().

NOTE: Seek in an input stream by calling seekg(); in an output stream by calling seekp().



• Exists() should be const, like this: bool Exists() const;`

StackExchange Code Review Q#28467, answer score: 4