Yeah, mixing .read() operations with operator>> is a pretty good sign of doing something wrong. The former implies unformatted, binary data, while the latter implies formatted, "text" (i.e. human-readable) data. It's unusual for a file to mix those.
That said:
- It looks like you're using some kind of union to reinterpret integers as byte arrays. Don't do that. That's what reinterpret_cast is for.
- You don't need to initialize the header before you read into it, because the reading-in is going to initialize it for you. Unless you're worried that file reading will fail *and* the uninitialized header happens to contain "BIG4" in the right spot - a valid concern, but if the file reading fails, everything's going to abort *anyway*.
- Use a helper function to read the integers, since you do it often. I have a templated one that I reproduce commonly in forum posts :/
- A bigger, design problem: Don't write 'load' functions. That work properly belongs in the constructor; that's what constructors are for. Also, don't hold on to the stream object within the BIGFile (you don't need it after you're done loading; or at least, you can reopen it via the saved file name) nor the resource_count or associated getter (it's redundant). And what's the 'dest' currently for?
- Oh, and don't make things 'protected' without a good reason. Private is private. "protected" exists for the sake of derived classes. Why would you have derived classes of BIGFile?
- Prefer to pass strings (and other class instances) by const reference.
- Are you *really* sure that you need to endian-swap the resource_count, but NOT any of the other ints in the file?
- Don't use std::string::compare() in "normal" code. The class is defined to make comparison operators work the way you expect, and it's much more readable that way. .compare() exists for when you have to do 3-way comparison and you have a slick way of using the resulting return value.
class BIGFile { struct Resource { std::streampos file_offset; uint32_t resource_size; std::string resource_name; }; struct BIG4Header { char id[4]; // must contain 'BIG4' uint32_t x1; uint32_t resource_count; uint32_t x2; }; std::vector<Resource> resc_list; std::string filename; public: BIGFile(const std::string& filename); // From what I've seen so far, you probably don't need a destructor. uint32_t GetCount() { return resc_list.size(); } std::string GetNames(); void Extract(const std::string&, const std::string&);};// Read into existing T. Return whether successful (unless the stream// is configured to throw an exception on failure).template <typename T>bool readPrimitive(istream& is, T& result) { is.read(reinterpret_cast<char *>(&result), sizeof(T)); return is;}// Read a new T. Return default T on failure (unless the stream// is configured to throw an exception on failure).template <typename T>T readPrimitive(istream& is) { T result; readPrimitive(is, result); return result;}BIGFile::BIGFile(const std::string& filename) : filename(filename) { ifstream in; // Bail out immediately if anything goes wrong, automatically cleaning things // up due to RAII in.exceptions(ios::bad_bit | ios::fail_bit | ios::eof_bit); in.open(file.c_str()); BIG4Header b4h = readPrimitive<BIG4Header>(in); char* resource_count_swapper = reinterpret_cast<char *>(&b4h.resource_count); std::swap(resource_count_swapper[0], resource_count_swapper[3]); std::swap(resource_count_swapper[1], resource_count_swapper[2]); // construct a string from the header ID, explicitly specifying the range of // bytes to use, so as to avoid problems with the lack of a null terminator. std::string type(b4h.id, b4h.id + 4); if (type != "BIG4") { throw ios::failure("File type not recognized: " + type); } Resource r; for (uint32_t i=0; i<resource_count; ++i) { readPrimitive(in, r.file_offset); readPrimitive(in, r.resource_size); std::stringbuf sbuff; in.get(sbuff,'\0'); r.resource_name = sbuff.str(); resc_list.push_back(r); }}
