I have a C++ class which acts as an abstraction for an IO device which is controlled through file descriptors. As I am very new to C++, I would be glad if you can give me devastating feedback on things I've done wrong.This especially covers:Code styleDesign patternsError handlingThings which are open for failuresC++ approaches which can be used over C approaches (file IO)gpio.h#ifndef SRC_GPIO_H_#define SRC_GPIO_H_#define GPIO_IN     0#define GPIO_OUT    1#define GPIO_LOW    0#define GPIO_HIGH   1class GPIO {    public:        explicit GPIO(int id);        ~GPIO();        int  Value();        void Value(int value);        int  Direction();        void Direction(int value);    private:        int _id;        int _value_fd;        int _direction_fd;        bool Exists();        void Export();        void Unexport();        void OpenValueFd();        void OpenDirectionFd();        void CloseValueFd();        void CloseDirectionFd();        void SeekToTopOfValueFd();        void SeekToTopOfDirectionFd();};#endifgpio.cc#include ../src/gpio.h#define GPIO_PATH_EXPORT        /sys/class/gpio/export#define GPIO_PATH_UNEXPORT      /sys/class/gpio/unexport#define GPIO_PATH_DIRECTORY     /sys/class/gpio/gpio%d#define GPIO_PATH_DIRECTION     /sys/class/gpio/gpio%d/direction#define GPIO_PATH_VALUE         /sys/class/gpio/gpio%d/valueGPIO::GPIO(int id) {    Export();    OpenValueFd();    OpenDirectionFd();}GPIO::~GPIO() {    CloseValueFd();    CloseDirectionFd();    Unexport();}boolGPIO::Exists() {    char * path;    if (asprintf(&path, GPIO_PATH_DIRECTORY, id) < 0)        throw Error generationg GPIO directory path.;    int result = access(path, F_OK);    free(path);    return static_cast<bool> result++;}voidGPIO::Export() {    char * id;    if (Exists()) return;    if (asprintf(&id, %d, _id) < 0)        throw Error converting id to char.;    int fd = open(GPIO_PATH_EXPORT, O_WRONLY);    if (fd < 0)        throw Error opening GPIO export.;    if (write(fd, id, strlen(id)) < 0)        throw Error writing to GPIO export.;    if (close(fd) < 0)        throw Error closing GPIO export.;    free(id);}voidGPIO::Unexport() {    char * id;    if (!Exists()) return;    if (asprintf(&id, %d, _id) < 0)        throw Error converting id to char.;    int fd = open(GPIO_PATH_UNEXPORT, O_WRONLY);    if (fd < 0)        throw Error opening GPIO unexport.;    if (write(fd, id, strlen(id)) < 0)        throw Error writing to GPIO unexport.;    if (close(fd) < 0)        throw Error closing GPIO unexport.;    free(id);}intGPIO::Value() {    char data[2];    SeekValueFdToTop();    data[1] = 0;    if (read(_value_fd, data, 1) < 0)        throw Error reading from GPIO value.;    if (strncmp(data, 0, 1) == 0)        return GPIO_LOW;    if (strncmp(data, 1, 1) == 0)        return GPIO_HIGH;    throw Invalid GPIO value.;}voidGPIO::Value(int value) {    SeekValueFdToTop();    switch (value) {        case GPIO_LOW:            if (write(_value_fd, 0\n, 2) < 0)                throw Error writing to GPIO value.;            break;        case GPIO_HIGH:            if (write(_value_fd, 1\n, 2) < 0)                throw Error writing to GPIO value.;            break;        default:            throw Error cannot set invalid GPIO value.;    }}intGPIO::Direction() {    char data[4];    SeekValueFdToTop();    data[3] = 0;    if (read(_direction_fd, data, 1) < 0)        throw Error reading from GPIO direction.;    if (strncmp(data, in, 2) == 0)        return GPIO_IN;    if (strncmp(data, out, 3) == 0)        return GPIO_OUT;    throw Invalid GPIO direction.;}voidGPIO::Direction(int value) {    SeekValueFdToTop();    switch (value) {        case GPIO_IN:            if (write(_value_fd, in\n, 3) < 0)                throw Error writing to GPIO direction.;            break;        case GPIO_OUT:            if (write(_value_fd, out\n, 4) < 0)                throw Error writing to GPIO direction.;            break;        default:            throw Error cannot set invalid GPIO direction.;    }}voidGPIO::OpenValueFd() {    char * path;    if (asprintf(&path, GPIO_PATH_VALUE, _id) < 0)        throw Error generating GPIO value path.;    if ((_value_fd = open(path, O_RDWR)) < 0)        throw Error opening GPIO value.;    free(path);}voidGPIO::OpenDirectionFd() {    char * path;    if (asprintf(&path, GPIO_PATH_DIRECTION, _id) < 0)        throw Error generating GPIO direction path.;    if ((_direction_fd = open(path, O_RDWR)) < 0)        throw Error opening GPIO direction.;    free(path);}voidGPIO::CloseValueFd() {    if (close(_value_fd) < 0)        throw Error closing GPIO value.;}voidGPIO::CloseDirectionFd() {    if (close(_direction_fd) < 0)        throw Error closing GPIO direction.;}voidGPIO::SeekToTopOfValueFd() {    if (lseek(_value_fd, SEEK_SET, 0) < 0)        throw Error seeking to top of GPIO value.;}voidGPIO::SeekToTopOfDirectionFd() {    if (lseek(_direction_fd, SEEK_SET, 0) < 0)        throw Error seeking to top of GPIO direction.;}Source can be found here
Abstraction for an IO device
c++;linux;io
Prefer the  C++ wayAs you seem to expect, one of the key issues with your code is that it's not very C++y.1. Prefer std::string over C-style strings.std::string is better in almost every way. It takes care of its own memory and grows when needed. The speed is usually comparable to C-style strings (sometimes faster, sometimes slower). The code written using string is often more readable. For example, compare your code:if (strncmp(data, in, 2) == 0)with the std::string variant:if (data == in)2. Prefer iostream to FILE*.The C++ library for IO is typesafe, extensible, object-oriented and should be preferred. Use an ifstream to read from a file and ofstream to write, or fstream for both.3. Consider throwing exceptions derived from std::runtime_error and std::logic_errorBy having an exception hierarchy, you can have fine grained control over which errors to catch where. Your code will also play nice with others, who might expect your exceptions to behave like the standard ones. Want to catch everything? Catch std::exception. Want to catch only your own? Do so. Want to get the error message? Call .what().Remember that exceptions should always be thrown by value, and catched (caught) by reference(-to-const).4. Prefer enum or consts to #defined constants.Instead of#define GPIO_IN     0#define GPIO_OUT    1writeenum IODirection { GPIO_IN = 0, GPIO_OUT = 1 };For your path name #defines, use const std::string instead. (And consider moving them into class scope as static, or into a namespace.)You get type safety, easier debugging, normal scope rules; the works.5. Avoid leading underscores in identifiers.The following identifiers are reserved:Any name that begins with an underscore and is in the global namespace.Any name that begins with an underscore followed by a capital letter.Any name that contains two consecutive underscores.While your _id and similar variables are technically correct, don't rely on yourself or others memorizing these rules. Play it safe and move the underscore to the other side: id_, or drop it altogether.Other thingsThese are mostly nitpicking, but you asked for a very critical review :-)6. #include the proper headers.I'm guessing you already are, and that maybe they were removed when posting the code for review. I'm mentioning it just in case. It's nice to include #includes in the review code, too.7. Let the build system take care of include paths.Instead of: #include ../src/gpio.h, prefer to just use #include gpio.h and let the preprocessor and build system take care of finding the right header for you. This makes it a lot easier to move files at a later point.8. Don't cast unless you have to.Instead ofreturn static_cast<bool> result++;writereturn result++ != 0;(And use parenthesis on your casts.)9. Use fclose() instead of close().The C library function is called fclose(). (This is a moot point, however, since you should prefer C++ IO anyway.)10. Use descriptive names and a clear interfaceYour public interface functions have names that imply that they don't change the state of the class. If they don't, make the functions const. (Your Exists() function should at least be const. The same may apply to functions that are only reading.) Consider separating the state changing functionality from the non-state-changing functionality, and putting the latter into const functions.Rename functions that do change state to reflect that.Finally, data is a classic, horrible variable name. I'm sure you can do better!