2
\$\begingroup\$

I am writing a program that has a row-based data table class. It also has an encoder class that is intended to take in some .csv data and convert it into a 2darray of encoded data of unknown type at compile time.

The idea is that the user will point to a .csv file they would like to read from and specify a runtime schema (column 1 == int, column 2 == bool, column 3 == string, etc) that will be used to encode the raw .csv data into the proper types needed for comparison and such elsewhere in the program.

I have been reading a lot about this and decided to try a union based approach. I mocked this up in a simple project to see how this might work out on one single piece of data ("DataPoint."). Here is the code.


#include "pch.h"
#include <iostream>
#include <sstream>


union DataPoint
{
    bool Bool;
    int  Integer;
    float FloatingPoint;
    //std::string String;
};

enum DataTypes
{
    Bool,
    Integer,
    FloatingPoint,
    //String
};


DataPoint EncodeData(DataPoint InDataPoint, DataTypes InDataTypes, std::string InData)
{
    std::stringstream DataStream(InData);

    switch (InDataTypes)
    {
    case Bool: 
        std::cout << "Type is Bool" << std::endl;
        if (InData == "True" || InData == "true"  || InData == "1") InDataPoint.Bool = true;
        else if (InData == "False" || InData == "false" || InData == "0") InDataPoint.Bool = false;
        else std::cout << "This data is not compatable with the bool type." << std::endl;
        break;
    case Integer: 
        std::cout << "Type is Integer" << std::endl;
        DataStream >> InDataPoint.Integer;
        break;
    case FloatingPoint: 
        std::cout << "Type is Floating Point" << std::endl;
        DataStream >> InDataPoint.FloatingPoint;
        break;
    /*
    case String: InDataPoint.String = InData;
        break;
    */
    default:
        break;
    }

    return InDataPoint;
}

void PrintData(DataPoint InDataPoint, DataTypes InDataTypes)
{
    switch (InDataTypes)
    {
    case Bool: std::cout << "Encoded data value : " << InDataPoint.Bool << std::endl;
        break;
    case Integer: std::cout << "Encoded data value : " << InDataPoint.Integer << std::endl;
        break;
    case FloatingPoint: std::cout << "Encoded data value : " << InDataPoint.FloatingPoint<< std::endl;
        break;
    /*
    case String:
        break;
    */
    default:
        break;
    }
}

int main()
{
    DataPoint DataPoint1{};
    DataTypes DataType{ Bool };
    std::string Data{ "true" };

    std::cout << "Raw data value : " << Data << std::endl;

    PrintData(EncodeData(DataPoint1, DataType, Data), DataType);

    return 0;
}

This all seems to work, however, as you can see... I have commented out string entries because I cannot figure out a good way to handle them. One issue I run into is though unions apparently support strings in c++ now, I error out when I try to add a string as a union member. Here is the error I get if I uncomment the string member of the union.

Severity Code Description Project File Line Suppression State Error (active) E1776 function "DataPoint::DataPoint(const DataPoint &)" (declared implicitly) cannot be referenced -- it is a deleted function

Is there some way to handle the string? Also, in general, does this approach seem sound?

\$\endgroup\$

closed as off-topic by esote, Graipher, t3chb0t, Mathias Ettinger, Gerrit0 May 14 at 17:44

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Code not implemented or not working as intended: Code Review is a community where programmers peer-review your working code to address issues such as security, maintainability, performance, and scalability. We require that the code be working correctly, to the best of the author's knowledge, before proceeding with a review." – esote, Graipher, t3chb0t, Mathias Ettinger, Gerrit0
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ I'm afraid this question does not match what this site is about. Code Review is about improving existing, working code. Code Review is not the site to ask for help in fixing or changing what your code does. Once the code does what you want, we would love to help you do the same thing in a cleaner way! Please see our help center for more information. \$\endgroup\$ – Mathias Ettinger May 14 at 9:04
1
\$\begingroup\$

I don't have time to check all now but I found some things I want to share

If you are using C++17 consider using std::variant instead of the weak union inherited from c. It will probaly also solve youre isue with std::string. See: https://en.cppreference.com/w/cpp/utility/variant

Consider using '\n' instead of std::endl. Im pretty sure you only want a newline and not also a expensive flush of the buffer: https://stackoverflow.com/questions/213907/c-stdendl-vs-n.

Consider using the more safe enum class instead of plain c enum. It is available since C++11.

\$\endgroup\$
  • \$\begingroup\$ The std::variant is a much better option it seems. Thanks for the other notes as well. \$\endgroup\$ – akami May 13 at 1:15

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