Designing a file class in C++

Started by
7 comments, last by Zahlman 16 years, 9 months ago
I am wondering what the best design for a class that contains a file would be. Basically the class needs to open a file and parse its header information and store it within the class' fields. Additionally I need the acctual contents of the file to be readable by the rest of the program. I think that returning the file stream would be a bad idea as it should probobly be a private member. My reasoning behind all this is because I want a modular program where possibly several parts of the program all need access to read the file and/or the allready parsed header info. As far as I can figure I might just have to wrap the fstream's read function in a class function. So does anyone have any ideas or am I going down the wrong path? If anyone would like to see the acctual source code I would be happy to post it if you remind me how the tags work again :D
Advertisement
If multiple other classes need that file data. I'd think the best idea is to just read the data and have a public accessor which returns a char pointer. A length field would also be necessary then.
How large is the file? If it's reasonably small, you're just as well loading the whole file into memory and doing as Sijmen suggests. If you're dealing with a huge file (> 50MB or so, or multiple smaller files), then you might want to just read the header, and then have an fread() style function to read data from the file itself - and if you're supporting multiple calls to read from the main file body, some kind of iterator class might be useful to seek to the correct position in the file before trying to read, so that two seperate classes trying to read from the file won't cause chaos.
Quote:Additionally I need the acctual contents of the file to be readable by the rest of the program.


No, you don't.

Your file class will take care of reading and positioning. The fstream or whatever implementation you choose is of no concern to anyone outside of your file class, in the same way actual file access isn't available to anyone outside of fstream class.

class Serializable{public:  virtual void write( std::ostream &os) {}  virtual void read( std::istream &is) {}}class X : public Serializable{public:  virtual void write( std::ostream &os) {    Serializable::write( os );    os << x << ... << ...;  }  virtual void read( std::istream &is) {    Seializable::read( is );    is >> x  >> ... >> ...;  }private:  int x;  ...  ...}


Then you have your file class:
class File{...  void read( Serializable *x )  {    x->read( /* your private fstream here */ );  }}


So whenever you need to read/write something to the file, just use the serialization. That goes for header as well.

File f;Header header;Index index;f.read( &header );f.read( &index );f.close();

... would be an example of such API.

But certainly don't go passing implementation details around.
Quote:Original post by antiquechrono
My reasoning behind all this is because I want a modular program where possibly several parts of the program all need access to read the file and/or the allready parsed header info.


You pretty much contradict yourself directly here. :) To be modular, you would want other parts of the program to go through the class' interface, and not have direct access to the file data.

What exactly does the file data represent? What does the *class* represent? (I.e., why don't you just pass a stream reference around?) What are you trying to do with the data?

Quote:If anyone would like to see the acctual source code I would be happy to post it if you remind me how the tags work again :D


I would. They work quite well, thank you. :)
I'd suggest to just use file mapping (Mac, Windows, Linux) and pass the resulting pointer around. It's also much more efficient than using the streams provided by the standard libraries.
Quote:Original post by Zahlman
You pretty much contradict yourself directly here. :) To be modular, you would want other parts of the program to go through the class' interface, and not have direct access to the file data.


I guess I worded it wrong, I do want the class to control all access to the file. There is no point in encapsulation if you don't :D My professors have put fear in my heart with reguards to exposing any internal class functionality lol.

The data itself is an NES rom file where as the class is meant to represent the rom controlling access to it and keeping the relevent header info.

Rom.h
#include <iostream>#include <fstream>#include <string>using namespace std;/*  iNes header description0-3      String "NES^Z" used to recognize .NES files.4        Number of 16kB ROM banks.5        Number of 8kB VROM banks.6        bit 0     1 for vertical mirroring, 0 for horizontal mirroring.         bit 1     1 for battery-backed RAM at $6000-$7FFF.         bit 2     1 for a 512-byte trainer at $7000-$71FF.         bit 3     1 for a four-screen VRAM layout.          bit 4-7   Four lower bits of ROM Mapper Type.7        bit 0     1 for VS-System cartridges.         bit 1-3   Reserved, must be zeroes!         bit 4-7   Four higher bits of ROM Mapper Type.8        Number of 8kB RAM banks. For compatibility with the previous         versions of the .NES format, assume 1x8kB RAM page when this         byte is zero.9        bit 0     1 for PAL cartridges, otherwise assume NTSC.         bit 1-7   Reserved, must be zeroes!10-15    Reserved, must be zeroes!16-...   ROM banks, in ascending order. If a trainer is present, its         512 bytes precede the ROM bank contents....-EOF  VROM banks, in ascending order.*///bug: does not read in the correct mapperclass Rom{private:	ifstream rom_image;	bool iNES;	int pgr_rom_pages;	int chr_rom_pages;	int mapper;	bool four_screen;	bool trainer;	bool battery;	bool h_mirror;	bool v_mirror;	int ram_pages;	bool pal;	string name;	int length;public:	Rom();	//bug: does not read in the correct mapper	//opens the rom and reads in the header info	bool load(string fname);	//closes the rom file	void close();	//returns true if a rom is open	bool is_open();};


Rom.cpp
#include "Rom.h"Rom::Rom(){	iNES=false;	pgr_rom_pages=0;	chr_rom_pages=0;	mapper=0;	four_screen=false;	trainer=false;	battery=false;	h_mirror=false;	v_mirror=false;	ram_pages=0;	pal=false;}bool Rom::load(string fname){	name = fname;	if(rom_image.is_open()){		rom_image.close();	}	rom_image.open(name.c_str(), ios::binary);		if(rom_image.is_open()){				rom_image.seekg(0, ios_base::end);		length = rom_image.tellg();		rom_image.seekg(0, ios_base::beg);		int header_test;		rom_image.read((char*)& header_test, 4);				if(header_test==0x1A53454E){			cout << "iNes header detected" << endl;			char temp;						rom_image.read(&temp, 1);			pgr_rom_pages=(int)temp;						rom_image.read(&temp,1);			chr_rom_pages=(int)temp;						rom_image.read(&temp,1);			if(temp&0x01) //vert/hor				v_mirror=true;			else				h_mirror=true;			if(temp&0x02) //batram				battery=true;			if(temp&0x04) //trainer				trainer=true;			if(temp&0x08) //4screen				four_screen=true;						char map_tmp=temp;			map_tmp &=0x0F;				//get next byte			rom_image.read(&temp,1);						temp &=0xF0;			mapper = (int)(temp|map_tmp);				//get next byte			rom_image.read(&temp,1);						//if(temp&) //ram banks				//get next byte			rom_image.read(&temp,1);						if(temp) //pal				pal=true;			rom_image.ignore(7);		    cout << endl;			cout << "Name      " << name << endl;			cout << "Length    " << length <<" bytes" << endl;			cout << "PGR       " << pgr_rom_pages << " x16 kb" << endl;			cout << "CHR       " << chr_rom_pages << " x8 kb" << endl;			cout << "Mapper    " << mapper << endl;			cout << "4-screen  " << four_screen << endl;			cout << "trainer   " << trainer << endl;			cout << "battery   " << battery << endl;			cout << "h mirror  " << h_mirror << endl;			cout << "v mirror  " << v_mirror << endl;			cout << "ram pages " << ram_pages << endl;			cout << "pal       " << pal << endl;		}		else{			rom_image.seekg(0,ios_base::beg);			cout << "no iNES header found...assuming raw image";		}		return true;	}	return false;}void Rom::close(){	if(rom_image.is_open())		rom_image.close();}bool Rom::is_open(){	return rom_image.is_open();}


[Edited by - antiquechrono on July 8, 2007 12:39:45 AM]
You're looking at the wrong level of abstraction... you don't want to think about the raw file data, you want to think about what it represents. (In this case, raw binary data from the cartridge's ROMs.)

Don't think of your class as a container for a file; think of it as a ROM/memory interface. (Actually, think of two separate memory interfaces; there's no real reason why the Program ROM and the Video ROM should be part of the same object.)

For the same reason, don't use a stream-like interface; streams imply sequential access, whereas NES ROMs are accessed in a largely random fashion.

Remember, NES ROMs are small; the largest commercial ROM is 768 KB, and no known mapper supports anything larger than 6 MB (which is the maximum allowed by the INES header anyway). You should just read the entire file into memory at startup and then close the file.

Also, if the INES header is not detected, you should simply exit, rather than attempting to use the file anyway. In practice, there is no such thing as a raw image; and without the header to identify the mapper used, a raw image for any but the oldest games (i.e. those that don't use a mapper) is useless.

Like anthony said. A ROM is, well, read-only memory, so your interface logically consists of functions that look at, but can't modify the data. (Except of course for the parts that are actually RAM. :) ) There's no point in keeping the header information around; it's only there so you can make sense of the rest of the file. We don't want to keep the *stream* around; just load data into memory and use it there - it's much easier, and also more robust (think about e.g. what's involved in writing to "battery-backed RAM" with each approach). And you definitely don't want functions like load/close/is_open; use RAII instead. (Even if you *did* want to fetch from the disk each time, you'd still arrange for the constructor of your object to open the stream, and then the destructor would automatically close it, and any valid object - i.e. one where no exception was thrown when trying to create it - would always be "open".)

As a rough sketch:

.h
// Don't use namespace std in headers! Your cpp files will be like// "OH GOD I CANNOT UNUSE IT". Also, don't forget inclusion guards.#ifndef ROM_H#define ROM_H#include <vector>#include <string>const int KILOBYTE = 1024;struct ROMBank {  char data[8 * KILOBYTE];};struct RAMBank {  char data[8 * KILOBYTE];};struct VROMBank {  char data[16 * KILOBYTE];};class Rom {  std::vector<ROMBank> program_data;  std::vector<VROMBank> video_data;  std::vector<RAMBank> writable_data;  std::string name;  // and definitely don't remember an overall file length! Useless here.  // Stuff that we'll need to remember if we want to write out the file again.  char config[4];  public:  // Use RAII: initialize in constructor.  // The vectors will automatically deallocate, so we don't need any .close()  // or whatever. (The actual file on disk will close at the end of the ctor.)  Rom(const std::string& filename);  // Read-only access to program and video data.  // These might need more complicated logic, depending on how mapping works...  const ROMBank& ROM(int which) const { return program_data[which]; }  const VROMBank& VROM(int which) const { return video_data[which]; }  // Read-write access to writable data.  const RAMBank& RAM(int which) const { return writable_data[which]; }  RAMBank& RAM(int which) { return writable_data[which]; }  // TODO: Fix the destructor so it writes out the data back to the original  // file. Left as an exercise. This implements "battery-backed RAM".  ~Rom() {}};#endif


.cpp
#include <fstream>#include "Rom.h"using namespace std;// I'm going to make a helper function to read values.template <typename T>T readValue(istream& is) {  T result;  // Note the new-style casting.  is.read(reinterpret_cast<char*>(&result), sizeof(T));  return result;}template <typename T>void readArray(istream& is, T* begin, int count) {  is.read(reinterpret_cast<char*>(begin), sizeof(T) * count);}Rom::Rom(const string& filename) : name(filename) {  // Use initialization lists where possible, as shown.  // We create the stream locally.  ifstream ifs;  // Unfortunately, the standard library doesn't use RAII very well in the  // case of file streams, so we have to do a little wrapping:  ifs.exceptions(ifstream::eofbit | ifstream::failbit | ifstream::badbit);  ifs.open(filename.c_str());  // Now, if anything goes wrong with the loading, an exception will be thrown.  // Although we have to do that manually for testing the header :)  // For now, we will reject "raw" dumps without a header.  if (readValue<int>(ifs) != 0x1A53454E) {    throw ifstream::failure("not an iNes ROM");  }  // Set up our rom pages.  program_data.resize(readValue<char>(ifs));  video_data.resize(readValue<char>(ifs));      char config[4];  readArray<char>(ifs, config, 4);  // To interpret that data, we make *member functions* that rearrange the bits  // and report an appropriate value.  // e.g.  // bool ROM::isVerticallyMapped() { return config[0] & 0x01; }  // But we should ONLY do that for things that calling code will care about.  // (Or at least, make the others private.)  // The calling code should NOT have to check what the "mapping" is in order  // to get at data; the file loading and/or access functions should apply  // the mapping.  // Check that there are 6 zero bytes.  const int padding = 6;  char reserved[padding];  readArray<char>(ifs, reserved, padding);  for (int i = 0; i < padding; ++i) {    if (reserved) {      throw ifstream::failure("not an iNes ROM");    }  }  // FIXME: Also check for required zero bits in the other four bytes.  // THEN, read the whole rest of the file and store "pages" in the vectors.  // Use the config information to apply whatever part of the "mapping" work  // makes sense at this point.}

This topic is closed to new replies.

Advertisement