Sign in to follow this  

Problem with fstream

This topic is 4375 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

for(long i = 0; i < m_TexturesInFile.Len(); i++)
	{
		sTemp = "Textures\\";
		sTemp += m_TexturesInFile[i].sName;
		fin.open(m_TexturesInFile[i].sName.data());
		if(!fin.is_open())
		{
			if(bOutput)
			{
				cout << m_TexturesInFile[i].sName << " does not exist" << endl;
			}
			bError = true;
		}
		fin.clear();
		fin.close();
	}

This is the code I'm using. However after it opens and closes the first file, it fails to open the rest (there are about 10 more). I've checked to make sure the files exist, and used the debugger to check that it's trying to open the correct files. I've had this problem before where I'm not able to reopen an fstream after I've closed it. Do I need to call some other functions besides .clear() and .close() before I reopen with a new name?

Share this post


Link to post
Share on other sites
try this.



for(long i = 0; i < m_TexturesInFile.Len(); i++)
{
sTemp = "Textures\\";
sTemp += m_TexturesInFile[i].sName;
ifstream fin;
fin.open(m_TexturesInFile[i].sName.data(), ios::in | ios::binary);
if(!fin))
{
if(bOutput)
{
cout << m_TexturesInFile[i].sName << " does not exist" << endl;
}
bError = true;
}
//fin.clear();
fin.close();

}




Share this post


Link to post
Share on other sites
I changed it to

for(long i = 0; i < m_TexturesInFile.Len(); i++)
{
sTemp = "Textures\\";
sTemp += m_TexturesInFile[i].sName;
fin.open(m_TexturesInFile[i].sName.data(), ios::in);
if(!fin)
{
if(bOutput)
{
cout << m_TexturesInFile[i].sName << " does not exist" << endl;
}
bError = true;
}
}
fin.close();


However I'm still having the same problem. After openning the first file the rest of them don't open.

Share this post


Link to post
Share on other sites
I've been trying to deal with a similar problem where I have an ifstream object in my database class and after it has been closed once, it will not open the file again. I made it into a pointer to an ifstream object, and created a new instance and deleted it each time that I was done with it, and that seems to work, but I would like to know why after closing the file it won't open again.

Share this post


Link to post
Share on other sites
Instead of using ifstream, you may take a look at _open () and _close (). Okay, these are obsolete, but I often use them in throw-away code by their simplicity:


for(long i = 0; i &lt; m_TexturesInFile.Len(); i++)
{
sTemp = "Textures\\";
sTemp += m_TexturesInFile[i].sName;
//fin.open(m_TexturesInFile[i].sName.data());
int iFHandle = _open (sTemp.c_str (), _O_BINARY) ;
//if(!fin.is_open())
if (iFHandle == -1)
{
if(bOutput)
{
cout &lt;&lt; m_TexturesInFile[i].sName &lt;&lt; " does not exist" &lt;&lt; endl;
}
bError = true;
}
//fin.clear();
_close (iFHandle) ;
}


You need to include fcntl.h, sys/stat.h, io.h for declarations and constants of these functions.

Share this post


Link to post
Share on other sites
Quote:
Original post by Chris41411
I've been trying to deal with a similar problem where I have an ifstream object in my database class and after it has been closed once, it will not open the file again. I made it into a pointer to an ifstream object, and created a new instance and deleted it each time that I was done with it, and that seems to work, but I would like to know why after closing the file it won't open again.


I just searched on google, and it seems that the problem we've both had isn't uncommon. I wonder if there's something actually wrong with the <fstream> code?

Share this post


Link to post
Share on other sites
Well what is the whole point of sTemp? It looks like you add the "Textures\\" prefix to it before adding on the texture name, but you do not use it in your code. Anyways, have you tried moving the ifstream into the loop so it's destroyed and recreated as needed?


for(long i = 0; i < m_TexturesInFile.Len(); i++)
{
ifstream fin;
sTemp = "Textures\\";
sTemp += m_TexturesInFile[i].sName;
fin.open(m_TexturesInFile[i].sName.data());
if(!fin.is_open())
{
if(bOutput)
{
cout << m_TexturesInFile[i].sName << " does not exist" << endl;
}
bError = true;
}
// fin.clear();
fin.close();
}

Share this post


Link to post
Share on other sites
Quote:
Original post by Drew_Benton
Well what is the whole point of sTemp? It looks like you add the "Textures\\" prefix to it before adding on the texture name, but you do not use it in your code. Anyways, have you tried moving the ifstream into the loop so it's destroyed and recreated as needed?

*** Source Snippet Removed ***


Wow, I feel like a n00b, lol. I was suppose to be opening the file using sTemp.data(). It wasn't opening because the code I wrote wasn't using the "Textures\\" prefix.

Thanks, for all the help guys:)

*edit* to show the fixed code

for(long i = 0; i < m_TexturesInFile.Len(); i++)
{
sTemp = "Textures/";
sTemp += m_TexturesInFile[i].sName;
ifstream fin;
fin.open(sTemp.data(), ios::in | ios::binary);
if(!fin.is_open())
{
if(bOutput)
{
cout << m_TexturesInFile[i].sName << " does not exist" << endl;
}
bError = true;
}
fin.close();
}

Share this post


Link to post
Share on other sites

for(long i = 0; i < m_TexturesInFile.Len(); i++) {
// may as well use int for the loop index? o_O
// Oh, and this ".Len()" really, really worries me.
// You're using []'s on m_TexturesInFile later, suggesting
// that it's some kind of container, but the standard library
// containers normally call this method ".length()" or ".size()".
// What is it that you are using? Smells like a wheel reinvention...
sTemp = "Textures/";
sTemp += m_TexturesInFile[i].sName;
ifstream fin;
fin.open(sTemp.data(), ios::in | ios::binary);
// First off, you probably want to use .c_str() rather than .data().
// Second, where is the string declared? If it's in some bigger scope, is
// there some reason you need to remember the name of the last file? If not,
// why is it in a higher scope at all? And in that case, you don't even
// need the temporary. You also don't need a separate .open() call and there's
// no really good reason to separate that out. Behold:
// ifstream fin((string("Textures/") + m_TexturesInFile[i].sName).c_str(),
// ios::in | ios::binary);
// I wouldn't fault you for wanting to split that up a *bit* more though :)
if(!fin.is_open()) {
if(bOutput) {
cout << m_TexturesInFile[i].sName << " does not exist" << endl;
}
bError = true;
// Ew ew ew the names :( What style guide are you reading? Throw it away,
// and read this one instead:
// http://www.cuj.com/documents/s=7989/cujcexp1911hyslop/hyslop.htm
}
fin.close();
// Don't need to close the file with this approach; it'll be implicitly
// closed by the destructor and then reconstructed each loop.
}


Share this post


Link to post
Share on other sites
m_TexturesInFile is a LinkedList template that I wrote. I didn't know the Standard Library had a linked list. Does it have hash tables also? Everytime I try to look up linked lists or hash tables in the VS Express 2005 help, all I find is .NET code, which is no good because my code isn't managed.

ifstream fin((string("Textures/") + m_TexturesInFile[i].sName).c_str(), ios::in | ios::binary);

Cool, I didn't know you could create a temporary string with string("blah blah blah"), thanks for telling me!

Thanks for the link on naming too, honestly I haven't liked HN very much, but thought it was the industry standard so I was trying to learn it.

Share this post


Link to post
Share on other sites
Quote:
Original post by Procyon Lotor
m_TexturesInFile is a LinkedList template that I wrote. I didn't know the Standard Library had a linked list.


Yep. You should use the std::list if that's what you want.
You should not try to use or write an operator[] for any kind of linked list. The std::list deliberately does not provide one, because it is such a bad idea - it creates useless work and makes it easy to write really slow code accidentally when you put it in a loop, because it would have to re-search from the beginning every time.

Instead, use iterators to iterate over standard library containers. This has the advantage that they will work transparently (with just a little bit of typedef work) if you later change your mind about what kind of container to use.


typedef std::list<Texture> texContainer;

texContainer textures;
// put stuff into it here

for(texContainer::iterator it = textures.begin();
it != textures.end(); // don't use < with iterators; not all provide it
++it) { // preincrement the iterator because the compiler may not be
// able to optimize it - the iterator is a class type itself, and this
// way is standard idiom
// Do stuff with the texture
}


Chances are good you want std::vector, unless you're adding or removing things at the beginning or in the middle, or need to keep the items sorted. (Although this point is somewhat controversial too; many argue for std::vector as a "default" and many others for std::deque instead.)

Quote:
Does it have hash tables also?


It does not have a hash table per the classic definition, but it does have a tree-based associative container called std::map, which provides the same interface. It is algorithmically slower for lookup, but it also keeps data in sorted order (this in turn requires that the key values can be compared to each other). Both those facts follow from the way it is implemented, as a self-balancing binary search tree.

Quote:
Cool, I didn't know you could create a temporary string with string("blah blah blah"), thanks for telling me!


It's amazing how many people don't seem to grasp this, but you can make a temporary like this for any class type, in just the same way that you can make a "temporary integer" in an expression like "3 * 4" without needing variables to hold those numbers (which would be stupid). It's done for symmetry, you know.

Quote:
Thanks for the link on naming too, honestly I haven't liked HN very much, but thought it was the industry standard so I was trying to learn it.


"The" industry standard... I must suppress a giggle. :) Seriously, though, it is better to consider these things for yourself, and prefer standards for which you have heard rational arguments that you accept, over standards that "everyone conforms to".

By the way, instead of trying to grab the sName() directly from a Texture, you should probably make that data private, and instead provide a function to return the full concatenated filename. You might even consider making it return the ifstream object (not sure if that works so well with ifstream objects - they don't like to be copied, you know). Although actually, it should probably do all the work that the inner loop will eventually contain, and return some other object constructed from the file data (what are these other files you're opening?)

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
Quote:
Original post by Procyon Lotor
m_TexturesInFile is a LinkedList template that I wrote. I didn't know the Standard Library had a linked list.


Yep. You should use the std::list if that's what you want.
You should not try to use or write an operator[] for any kind of linked list. The std::list deliberately does not provide one, because it is such a bad idea - it creates useless work and makes it easy to write really slow code accidentally when you put it in a loop, because it would have to re-search from the beginning every time.

My LinkedList template uses a cursor, so if you use the operator[] it starts from the cursor (which is left at the last item accessed) or at the head of the list, whichever is closer. That way if you're accessing things in sequencial order there's basicly zero search time.
Quote:

Quote:
Does it have hash tables also?


It does not have a hash table per the classic definition, but it does have a tree-based associative container called std::map, which provides the same interface. It is algorithmically slower for lookup, but it also keeps data in sorted order (this in turn requires that the key values can be compared to each other). Both those facts follow from the way it is implemented, as a self-balancing binary search tree.

I was reading a guide on the STL (http://www.sgi.com/tech/stl/), after you told me it contained so many pre-written templates that I could be taking advantage of. And they talked about hash table templates (http://www.sgi.com/tech/stl/HashedAssociativeContainer.html) Hashed Associative Containers they call them.
Does that look like a good guide to the STL, or maybe there's a better one I should read?

Quote:
By the way, instead of trying to grab the sName() directly from a Texture, you should probably make that data private, and instead provide a function to return the full concatenated filename. You might even consider making it return the ifstream object (not sure if that works so well with ifstream objects - they don't like to be copied, you know). Although actually, it should probably do all the work that the inner loop will eventually contain, and return some other object constructed from the file data (what are these other files you're opening?)

The code fragment I posted is part of a dev tool, all it does is check that a map has been created correctly. That loop is checking that all the texture files needed for the map actually exist. m_TexturesInFile doesn't actually hold any textures, it only holds strings (sName) that contain the name of a texture, which were read from a file listing all the textures used on the map.


Thanks sooo much for all the help and advice, you guys are the best:)

Share this post


Link to post
Share on other sites

This topic is 4375 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this