8
\$\begingroup\$

I'm not very good with templates, so any general tips would be appreciated.

Basically this class parses a CSV file with a very specific format.
My original idea for this was that I wanted this to be useful in MFC desktop applications and non-MFC applications such as Windows services. Basically, I wanted this class to be compatible with std::string, CString, and possibly _bstr_t for strings and FILE, std::fstream, maybe HANDLE, and CStdioFile for files. I have implemented the std library specializations and now, I'm not sure if implementing all the others would be a good idea.

Another goal by using templates was to reduce the number of header files in my header file. I'm not sure this is a good reason to use templates.

// TrainInfo.h

#pragma once

#ifndef TRAININFO_H
#define TRAININFO_H

#include <vector>

template <typename StringType>
struct TrainInfo {
    enum { 
        // Values omitted 
        NUM_ENUM
    };

    std::vector <StringType> m_info ;

    int GetNumberOfWheels () const ;

    // Minimum number of elements that should be parsed.
    static int const NELEM_MIN_PARSED = TrainInfo::NUM_ENUM - 1 ;
};

template <typename StringType>
struct WheelInfo {
    enum {
        // Values omitted
        NUM_ENUM
    };

    std::vector <StringType> m_info ;

    // Minimum number of elements that should be parsed.
    static int const NELEM_MIN_PARSED = WheelInfo::FileIndex + 1 ;
};

struct ProfilePoint {
    ProfilePoint (int x = 0, int y = 0) : x (x), y (y) {}
    ~ProfilePoint () {}
    double x ;
    double y ;
};

template <typename StringType>
struct WheelProfile {
    StringType m_strAxleSequenceNumber ;
    StringType m_strTrainSide ;
    StringType m_strNumPoints ;
    std::vector <ProfilePoint> m_vecPoints ;

    int GetNumberOfPoints () const ;

    static const int NELEM_WHEEL_PROFILE_HEADER = 3 ;
};

template <typename StringType, typename FileType>
class CSVInfo {
public:
    TrainInfo <StringType> m_trainInfo ;
    std::vector <WheelInfo <StringType> > m_wheels ;
    std::vector <WheelProfile <StringType> > m_profiles ;

    CSVInfo () ;
    ~CSVInfo() ;
    void Load (const StringType &strFile) ;

private:
    TrainInfo <StringType>                      ParseTrainData      (FileType &file) const ;
    std::vector <WheelInfo <StringType> >       ParseWheels         (const int nWheels, FileType &file) const ;
    std::vector <WheelProfile <StringType> >    ParseWheelProfiles  (const int nWheels, FileType &file) const ;

    void                        ParseWheelProfileHeader (const StringType &strLine, WheelProfile <StringType> &wheelprofile) const ;
    std::vector <ProfilePoint>  ParseWheelProfile       (const int nPoints, FileType &file) const ;

    template <typename RetType>  
    RetType TokenizeLine (const StringType &strLine) const ;

    template <typename RetType>  
    RetType TokenizeLine (const StringType &strLine, const int nMinExpected) const ; 
};

template <typename ExceptionBaseClass, typename StringType>
class CSVInfoException : public ExceptionBaseClass
{
public:
    CSVInfoException () : ExceptionBaseClass () {} 
    CSVInfoException (const StringType &strWhat) : ExceptionBaseClass (strWhat) {}
    ~CSVInfoException () {}
};

#endif

Here are the specializations I have so far.

// TrainInfo.cpp

#include "StdAfx.h"
#include "TrainInfo.h"
#include "My_Exception.h"

#ifndef TSTRING_TYPEDEF
#define TSTRING_TYPEDEF
#include <tchar.h>
#include <string>
typedef std::basic_string <TCHAR> tstring ;
#endif

#ifndef TSSTREAM_TYPEDEF
#define TSSTREAM_TYPEDEF
#include <sstream>
typedef std::basic_stringstream <TCHAR> tsstream ;
#endif

#ifndef TIFSTREAM_TYPEDEF
#define TIFSTREAM_TYPEDEF
#include <fstream>
typedef std::basic_ifstream <TCHAR> tifstream ;
#endif

#ifndef VECTSTRING_TYPEDEF
#define VECTSTRING_TYPEDEF
typedef std::vector <tstring> VecTString ;
#endif

#ifndef MyCSVEXCEPTION_TYPEDEF
#define MyCSVEXCEPTION_TYPEDEF
typedef CSVInfoException <MyLib::MyException, tstring> MyCSVInfoException ;
#endif

static void SanityCheckString (const tstring &str, const tstring &strExpected) ;
static void ThrowErrorParsingSection (const tstring &strSection, const tstring &strLastLineParsed) ;
static void ThrowUnexpectedTokenCount (const int nTokens, const int nTokensExpected, 
                                const tstring &strLine) ;

template <>
CSVInfo <tstring, tifstream>::CSVInfo ()
{
}

template <>
CSVInfo <tstring, tifstream>::~CSVInfo ()
{
}

template <>
void CSVInfo <tstring, tifstream>::
    Load (const tstring &strFile)
{
    tifstream file ;
    file.exceptions (tifstream::badbit | tifstream::failbit) ;

    try {
        file.open (strFile.data ()) ;

        // Parse train info
        m_trainInfo = this->ParseTrainData (file) ;

        // For all wheels, parse Wheel Info
        int nWheels = m_trainInfo.GetNumberOfWheels () ;
        if (nWheels < 1) {
            throw MyCSVInfoException (_T ("Wheel data is missing.")) ;
        }
        m_wheels = this->ParseWheels (nWheels, file) ;

        // Parse wheel profiles
        m_profiles = this->ParseWheelProfiles (nWheels, file) ;
    }

    catch (std::ios_base::failure) {
        tsstream tss ;
        tss << _T ("An I/O error occurred. Please verify the file: ") 
            << strFile << _T (".\r\nLast error code: ") << ::GetLastError () << _T (".") ;
        throw MyCSVInfoException (tss.str ()) ;
    }

    catch (MyCSVInfoException &e) {
        tsstream tss ;
        tss << _T ("An error occurred for file: ") << strFile << _T (".") ;
        e.AddDetail (tss.str ()) ;
        throw ;
    }
}

template <> template <>
ProfilePoint CSVInfo <tstring, tifstream>::
    TokenizeLine <ProfilePoint> (const tstring &strLine) const
{
    ProfilePoint point ;
    TCHAR comma = _T (',') ;
    tsstream tss ;
    tss.exceptions (tsstream::failbit | tsstream::badbit) ;

    try {
        tss << strLine ;
        tss >> point.x ;
        tss >> comma ;
        tss >> point.y ;
    }

    catch (std::ios_base::failure) {
        tsstream tssErr ;
        tssErr << _T ("Failed to tokenize profile point: ") << strLine << _T (".") ;
        throw MyCSVInfoException (tssErr.str ()) ;
    }

    return point ;
}

template <> template <>
VecTString CSVInfo <tstring, tifstream>::
    TokenizeLine <VecTString> (const tstring &strLine) const
{
    tsstream tss (strLine) ;
    VecTString vecData ;

    tstring strCell ;
    while (std::getline (tss, strCell, _T (','))) {
        vecData.push_back (strCell) ;
    }

    return vecData ;
}

template <> template <>  
VecTString CSVInfo <tstring, tifstream>::
    TokenizeLine (const tstring &strLine, const int nMinExpected) const
{
    VecTString vecData = this->TokenizeLine <VecTString> (strLine) ;
    if (vecData.size () < (VecTString::size_type) nMinExpected) {
        ThrowUnexpectedTokenCount (vecData.size (), nMinExpected, strLine) ;
    }

    return vecData ;
}

template <>
TrainInfo <tstring> CSVInfo <tstring, tifstream>::
    ParseTrainData (tifstream &file) const
{
    tstring strLine ;
    TrainInfo <tstring> traininfo ;
    try {
        // Skip line Section Train
        std::getline (file, strLine) ;
        SanityCheckString (strLine, _T ("Section Train")) ; // Sanity check all sections.

        // Skip line Train Columns
        std::getline (file, strLine) ;

        // Parse Train Info
        std::getline (file, strLine) ;  
        traininfo.m_info = this->TokenizeLine <VecTString> (strLine, TrainInfo <tstring>::NELEM_MIN_PARSED) ;
    }

    catch (std::ios_base::failure) {
        ThrowErrorParsingSection (_T ("Section Train"), strLine) ;
    }

    return traininfo ;
}

template <>
std::vector <WheelInfo <tstring> > CSVInfo <tstring, tifstream>::
    ParseWheels (const int nWheels, tifstream &file) const
{
    tstring strLine ;
    std::vector <WheelInfo <tstring> > vecWheels ;
    try {
        // Skip line Section Wheel
        std::getline (file, strLine) ;
        SanityCheckString (strLine, _T ("Section Wheel")) ; // Sanity check all sections.

        // Skip line Wheel Columns
        std::getline (file, strLine) ;

        // Parse all wheels
        for (int n = 0; n < nWheels; ++n) {
            std::getline (file, strLine) ;
            WheelInfo <tstring> wheel ;
            wheel.m_info = this->TokenizeLine <VecTString> (strLine, WheelInfo <tstring>::NELEM_MIN_PARSED) ;
            vecWheels.push_back (wheel) ;
        }
    }

    catch (std::ios_base::failure) {
        ThrowErrorParsingSection (_T ("Section Wheel"), strLine) ;
    }

    return vecWheels ;
}

template <>
std::vector <WheelProfile <tstring> > CSVInfo <tstring, tifstream>::
    ParseWheelProfiles (const int nWheels, tifstream &file) const
{
    tstring strLine ;
    std::vector <WheelProfile <tstring> > vecProfiles ;
    try {
        // Skip line Section Wheel Profiles
        std::getline (file, strLine) ;
        SanityCheckString (strLine, _T ("Section Wheel Profiles")) ;

        for (int n = 0; n < nWheels; ++n) {
            // Skip Header AxleSequenceNumber,TrainSide,NumberOfProfilePoints
            std::getline (file, strLine) ;

            // Parse header information.
            std::getline (file, strLine) ;
            WheelProfile <tstring> wheelProfile ;
            this->ParseWheelProfileHeader (strLine, wheelProfile) ;

            // Skip Header X,Y
            std::getline (file, strLine) ;
            int nPoints = wheelProfile.GetNumberOfPoints () ;
            wheelProfile.m_vecPoints = this->ParseWheelProfile (nPoints, file) ;
            vecProfiles.push_back (wheelProfile) ;
        }
    }

    catch (std::ios_base::failure) {
        ThrowErrorParsingSection (_T ("Section Wheel Profiles"), strLine) ;
    }

    return vecProfiles ;
}

template <>
void CSVInfo <tstring, tifstream>::
    ParseWheelProfileHeader (const tstring &strLine, WheelProfile <tstring> &wheelprofile) const
{
    VecTString vecData = this->TokenizeLine <VecTString> (strLine, WheelProfile <tstring>::NELEM_WHEEL_PROFILE_HEADER) ;

    wheelprofile.m_strAxleSequenceNumber = vecData [0] ;
    wheelprofile.m_strTrainSide = vecData [1] ;
    wheelprofile.m_strNumPoints = vecData [2] ;
}

template <>
std::vector <ProfilePoint> CSVInfo <tstring, tifstream>::
    ParseWheelProfile (const int nPoints, tifstream &file) const
{
    std::vector <ProfilePoint> vecPoints ;
    for (int n = 0; n < nPoints; ++n) {
        tstring strLine ;
        // Parse X,Y
        std::getline (file, strLine) ;      
        ProfilePoint point = this->TokenizeLine <ProfilePoint> (strLine) ;
        vecPoints.push_back (point) ;
    }

    return vecPoints ;
}

template <>
int TrainInfo <tstring>::GetNumberOfWheels (void) const
{
    int nWheels = 0 ;
    const tstring &strAxleCount = m_info [TrainInfo::NumberOfAxles] ;
    tsstream tss (strAxleCount) ;
    tss.exceptions (tsstream::failbit | tsstream::badbit) ;

    try {
        tss >> nWheels ;
        nWheels *= 2 ;
    }

    catch (std::ios_base::failure) {
        tsstream tssErr ;
        tssErr << _T ("Failed to parse number of axles from value: ") << strAxleCount << _T (".") ;
        throw MyCSVInfoException (tssErr.str ()) ;
    }

    return nWheels ;    
}

template <>
int WheelProfile <tstring>::GetNumberOfPoints (void) const
{
    int nPoints = 0 ;
    tsstream tss (m_strNumPoints) ;
    tss.exceptions (tsstream::failbit | tsstream::badbit) ;

    try {
        tss >> nPoints ;
    }

    catch (std::ios_base::failure) {
        tsstream tssErr ;
        tssErr << _T ("Failed to parse number of points from value: ") << m_strNumPoints << _T (".") ;
        throw MyCSVInfoException (tssErr.str ()) ;
    }

    return nPoints ;
}

//////////////////////
// Static Functions //
//////////////////////

static void SanityCheckString (const tstring &str, const tstring &strExpected)
{
    if (str != strExpected) {
        tsstream tss ;
        tss << _T ("A sanity check on a string has failed.") _T ("\r\nExpected string: ") << strExpected
            << _T ("\r\nActual string: ") << str ;
        throw MyCSVInfoException (tss.str ()) ;
    }
}

static void ThrowErrorParsingSection (const tstring &strSection, const tstring &strLastLineParsed)
{
    tsstream tss ;
    tss << _T ("An I/O error occurred while parsing: ") << strSection << _T (". ")
        << _T ("Last line parsed: ") << strLastLineParsed ;
    throw MyCSVInfoException (tss.str ()) ; 
}

static void ThrowUnexpectedTokenCount (const int nTokens, const int nTokensExpected, 
                                       const tstring &strLine)
{
    tsstream tss ;
    tss << _T ("Parsed incorrect number of tokens. Number of tokens: ") 
        << nTokens << _T (", number expected: ") 
        << nTokensExpected << _T (". Line: ") << strLine ;
    throw MyCSVInfoException (tss.str ()) ;
}

Now some questions,

  1. I am starting to think I should just stick with the standard library and not finish the specializations for MFC and Windows. Would it be better to leave the class as it is, or throw all the template stuff out?
  2. If I decide to continue implementing the specializations, I assume I would put each specialization into its own .cpp file to keep the headers separated. In this case though, I would have to distribute many different .cpp files along with my header. This seems like a bad smell. What is the preferred way of handling this?
  3. As I stated earlier, one of the reasons I wanted to use templates was to remove header files from my own header file. Now I'm not sure that was a good idea. Is it a common or bad practice to use templates to avoid having headers files in your header file?
  4. Are there any other design flaws or issues that I should be worried about?
\$\endgroup\$
4
\$\begingroup\$

I've long since wanted to explore doing this sort of thing myself. In different code bases, different string classes are so much more prevalent, and thus more appropriate to keep using. Your approach isn't quite the right way to go, as it scales on the wrong axis. I can totally see removing the templates and using just a single tstring (or even wstring), and realistically I would probably go that way. There are just so many downsides to using templates for large swaths of functionality (like how much typically needs to be moved to the headers). But if you want to keep the string type flexibility, a better approach would be to use traits classes.

Traits classes help reduce coupling. With a traits class approach, you don't need to specialize your class methods for what changes. Instead you specialize what needs to change for how you use it. Create and specialize traits helper methods for each operation you need on a string. The payoff comes in your next method or next class that only uses functionality you've already put on the traits class. The only hard part is figuring out what to make your traits calls look like.

Here's a simplified beginning to traits class following std::string and std::stringstream's semantics. I hope it's easy to follow how this changes which way your code has to scale. I'm far from certain it's doing things "correctly", as MSVC's traits_type implementations on basic_string and friends look very different. (I'm still wrapping my head around how to use traits classes.)

template <typename StringType>
struct StreamTraits
{
    typedef std::basic_stringstream<
        typename StringType::value_type,
        typename StringType::traits_type
    > value_type;
};

// Later to handle non basic_stringstream cases, use specialization
// template <>
// struct StreamTraits<CString>
// {
//      typedef CStringStream value_type;
// };

template <typename StringType>
struct StringTraits
{
    typedef StringType value_type;
    typedef typename StreamTraits<StringType>::value_type stream_type;
};

// Use traits to insulate callers from need to specialize
template <typename StringType>
struct TrainInfo
{
    typedef StringTraits<StringType> StringTraits;
    typedef typename StringTraits::value_type String;
    typedef StreamTraits<StringType> StreamTraits;
    typedef typename StreamTraits::value_type Stream;

    int GetNumberOfWheels() const;
};

template <typename StringType>
int TrainInfo<StringType>::GetNumberOfWheels() const
{
    int nWheels = 0;
    const String& strAxleCount = "2";
    Stream s(strAxleCount);
    // the following should probably be encapsulated in a StreamTraits helper method
    s.exceptions(Stream::failbit | Stream::badbit);

    s >> nWheels;
    return nWheels * 2;
}

Note that none of this solves the points your raise. At best it helps focus the headers, as you could have a header with all your string-specific specializations. This header could then be used by your train classes, or by any other class with similar needs. Users of your train classes that need a string type you didn't plan for should be able to provide traits class specializations and then use their own string class.

But it's still a lot of programming and maintenance overhead, so I would avoid it unless you know you need it. I certainly wouldn't do it just to cut down on the number of headers I was including.

\$\endgroup\$
  • \$\begingroup\$ +1 For "as it scales on the wrong axis". This was what I was unsure how to solve. As evil as this sounds, throwing all the work onto other users is what I wanted to do so that I do not have to continuously change this interface. In most cases, I will be the "other users" just in different programs that may use different APIs. Realistically though, as you said, I'm probably better off throwing out the templates and just using tstring (I have to support ASCII and UTF-16, so I can't just stick to wstring). \$\endgroup\$ – jliv902 Nov 25 '13 at 16:14
  • \$\begingroup\$ @jliv902 The result of putting work in the right place is that he who needs the functionality can be the one to implement it. That's not evil; that's practical. :) As for wstring vs ASCII, note that tstring is only one, and chosen at compile time (not file read time). You'd be better off consistently converting to Unicode on file read, only having a Unicode internal interface, and convert back at some future file write. \$\endgroup\$ – Michael Urman Nov 25 '13 at 16:30

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.