3
\$\begingroup\$

I'm making a class that takes a path to a text file as an argument in its constructor, and then parses the text file and pulls out a lot of data that it stores as private members. Most of these members are vectors of ints, floats, and strings.

The class will be used by different versions of VS, so my understanding is that I need to be careful about exporting templates like the STL containers (including string). I also assume I need to be careful about passing templates into the library. Therefore, everything going into or out of the public interface is kept as a primitive type.

//header
class FileLoader
{
public:
    FileLoader(const char* path);

    int GetIterationCount();
    const char* GetName();
    int GetID();

    int GetPathValue(int iteration);
    int GetAttemptsMadeCount(int iteration);
    float GetStartTime(int iteration); 
    float GetEndTime(int iteration);
    const char* GetLargestContributer(int iteration);

private:
    int iterationCount;
    std::string GetName;
    int ID;   

    std::vector<int> pathValue;
    std::vector<int> attemptsMadeCount;
    std::vector<float> startTime;
    std::vector<float> endTime;
    std::vector<std::string> largestContributer;
};

//associated cpp
int FileLoader::GetPathValue(int iteration)
{
    return pathValue.at(iteration);
}    
const char* GetLargestContributer(int iteration)
{
    return largestContributer.at(iteration).c_str();
}
 //similar structure for other accessors

There's something like 15 data items, some are vectors and some are just single items.

This really clutters up the public interface, but I don't see a good way around it. I considered adding an enum at the top:

enum class DataTypes
{
    PathValue;
    AttemptsMadeCount;
    StartTime;
    EndTime;
    LargestContributer;
};

And then I could just have a single function for results:

int GetResults(DataTypes::PathValue, int iteration);

And grab the appropriate underlying data with a switch. However, I don't think there's any way that'll work because the return types are different for different results. The ints need to be ints because they're the type of things people will want to do comparison operations on.

Another thought was to have a results struct that contained all the different data items for a given iteration. Then the interface is just a function that takes an iteration number and spits out the struct, and the user can choose which data item they care about.

That's the most promising to me, but the normal user will probably be more interested in all the iterations for a given data item at once. As I was typing this I was concerned about memory, since getting all the iterations for one item would mean getting it for all or most of the others. This is many hundreds of megabytes of data so that might not be trivial. However, it occurs to me that my function could populate the struct with const references to the underlying data.

Is that being too inefficient just for the sake of prettiness? It does work fine now, but I figure if there's a better way to do it I'd learn something in the process.

\$\endgroup\$
  • \$\begingroup\$ Have you considered a hybrid between your two suggestions? I.e. readIntField(DataTypes::DataItem, int iteration), etc. \$\endgroup\$ – Taekahn Jul 17 '15 at 4:14
  • \$\begingroup\$ @MatsMug the fact that there is more code doesn't invalidate this subset ... \$\endgroup\$ – o11c Jul 17 '15 at 4:18
  • 3
    \$\begingroup\$ I guess it's too much of a stub. I'll try to iron it out tomorrow. \$\endgroup\$ – mock_blatt Jul 17 '15 at 4:21
  • \$\begingroup\$ Could you explain what these methods do, and what the iteration parameter means? \$\endgroup\$ – 200_success Jul 17 '15 at 7:54
1
\$\begingroup\$

I think you can use templates to good use to simplify the user interface and still retain the flexibility.

I would suggest declaring some structs instead of using enum class DataTypes.

struct DataType_1;
struct DataType_2;
struct DataType_3;
struct DataType_4;

You may define them to be empty structs but that is not necessary for what I am going to suggest.

// This is OK too but not necessary.
struct DataType_1 {};
struct DataType_2 {};
struct DataType_3 {};
struct DataType_4 {};

Then create template class that captures the notion of traits corresponding to the above structs.

template <typename T> struct DataTypeTrait;

template <> struct DataTypeTrait<DataType_1>
{
   using type = int;
};

template <> struct DataTypeTrait<DataType_2>
{
   using type = int;
};

template <> struct DataTypeTrait<DataType_3>
{
   using type = float;
};

template <> struct DataTypeTrait<DataType_4>
{
   using type = char const*;
};

Create a wrapper class around the traits class.

template <typename T> struct DataType
{
   using type = DataTypeTrait<T>::type;
};

I understand that you have some concerns about using containers from the standard library. However, it's OK to use of member function templates described below since they don't involve use of standard library containers in the interface of the member functions.

Here's how I see FileLoader to look like:

class FileLoader
{
   public:
      FileLoader(const char* path);

      int GetIterationCount();

      // This is all you need in the public interface.
      template <typename T>
         typename DataType<T>::type getData(int iteration) const
         {
            return getData(iteration, (T*)nullptr);
         }

   private:

      // The dummy arguments allows the function overloading to
      // work seamlessly from the member function template.

      int getData(int iteration, DataType_1* dummy) const;
      int getData(int iteration, DataType_2* dummy) const;
      float getData(int iteration, DataType_3* dummy) const;
      char const* getData(int iteration, DataType_4* dummy) const;
};

User code

char const* filepath = "some/file/path";
FileLoader loader(filepath);
int val1 = loader.getData<DataType_1>(0);
float val2 = loader.getData<DataType_3>(0);
float val3 = loader.getData<DataType_4>(0); // Error.
char const* val4 = loader.getData<DataType_4>(0);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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