2
\$\begingroup\$

This code will be used to map a set of unique keys to a container of objects that are unique to that key. I would like to be able to reuse this code in future projects hence the template, and not hard coding the types into the class. You can also access the code here.

main.cpp

#include <set>
#include <string>
#include <fstream>
#include "file_to_map.h"

int main(int argc, char *argv[])
{
  file_to_map<std::string, std::multiset<std::string> > ftm("opengl_functions");
  std::ifstream ifs;
  ifs << ftm;
  std::cout >> ftm;
  return 0;
}

file_to_map.h

#ifndef FILE_TO_MAP_H_
#define FILE_TO_MAP_H_

#include <map>
#include <vector>
#include <string>
#include <fstream>
#include <iostream>
#include <iterator>
#include <algorithm>

template <class T>  
struct print
{
  print(std::ostream &out) : os(out) {}
  void operator() (T x) { os << x << ' ' << std::endl; }
  std::ostream & os; 
};

template <class key, class container>
class file_to_map
{
public:
  file_to_map() 
  {
    m_file = ""; 
  }

  file_to_map(std::string file)
  {
    m_file = file;
  }

  ~file_to_map() 
  {
  }

  std::map<key, container>& getMap() {
    return m_map;
  }

  friend std::ostream& operator>> (std::ostream &out, file_to_map<key, container> &obj)
  {
    typedef typename std::map<key, container>::const_iterator mapItr;
    mapItr mbi = obj.m_map.begin();
    mapItr emi = obj.m_map.end();
    while (mbi != emi) {
      out << " -- " << mbi->first << " -- " << std::endl;
      ++mbi;
    }   
    return out;
  }

  friend std::istream& operator<< (std::ifstream &in, file_to_map<key, container> &obj)
  {
    if (in.is_open()) 
        in.close();

    if (obj.m_file == "") 
      return in; 

    in.open(obj.m_file.c_str(), std::ios::in);

    if (in.fail() || in.bad()) {
      in.close();
      return in; 
    }

    std::vector<key> tmp;
    typedef std::istream_iterator<key> string_input;
    copy(string_input(in), string_input(), back_inserter(tmp));
    typename std::vector<key>::iterator bvi = tmp.begin();
    typename std::vector<key>::iterator evi = tmp.end();
    while (bvi != evi) {
        obj.m_map[*(bvi)] = container();
        ++bvi;
    }

    in.close();
    return in;
  }
private:
  std::map<key, container> m_map;
  std::string m_file;


};
#endif//FILE_TO_MAP_H_

Makefile

file_to_map : file_to_map.h main.cpp
        g++ -o file_to_map -Wall ./file_to_map.h ./main.cpp
\$\endgroup\$
2
\$\begingroup\$

For one thing, you have operators << and >> reversed for the streams.

I wouldn't recommend overloading ifstream operator, at least not like this. operator>> does not normally open and close files, it reads a value from an already open stream. Basically your class is just asking the caller to provide what the class could create itself. Neither would chaining this operator use do any good, because the returned stream is not good anyway.

Personally I think that operator>> should either be able to parse what operator<< outputs, or it shouldn't be implemented at all.

The constructor itself should load the file and/or the class should provide a named method for loading a file.


As to populating the map keys, is it really necessary to put the keys in a vector first?

\$\endgroup\$
0
\$\begingroup\$

The main issue I see here is that you are loading in a map from a file and I see you load in the keys but cannot see anywhere where you load in the values.

Loading in all the keys first isn't necessarily a bad idea, and if you do it that way then you do need to use a vector or similar container as a temporary container (so you retain the order when you read in the values).

Unless you already have files in your file system that you need to load into a map, and you can't change these files, then I would take a somewhat different approach for a "generic" map read (and write). For example if you have a function to read a vector then you can use that also to read the map (by reading all the keys) then again to read the values. Note that these cannot read until "end of stream" (well particularly the key-reader cannot) so there must be a special marker to determine where these end, or a header section so you know how far to read.

Once you have determined how the layout is going to be, then write the code for it. I would suggest you make this section use a generic "stream" rather than a file as such.

\$\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.