Generic programming (not really Allegro related)
TeamTerradactyl

In these attachments, I have a very simple program that simply takes command-line arguments and checks whether or not they are valid filenames.

It seems to work well, the debugger didn't report any SEGFAULTS, etc. The only problem I have is when I pass "invalid" filenames to the program: it "forgets" the next "valid" filename(s).

Example:

program.exe file1 file2 file3 file4

file[134] exist, while 'file2' does not.

My output SHOULD be: 

Valid filenames found:
  file1
  file3
  file4

Instead, output is: 

Valid filenames found:
  file1
  file4

Any idea why 'file3' is not showing up after 'file2' is NOT found?

Thanks for any ideas!

Indeterminatus

Filename &operator= is missing a return *this; statement.

Other than that, are you not allowed to use std::vector or std::list? Probably std::set would make even more sense, unless of course the order is important.

TeamTerradactyl

I added <b>return this; to the &operator= as follows, but it did not seem to help:

Filename &Filename::operator=(const Filename &orig)
{
   numFiles = orig.numFiles;
   array = new string[numFiles];
   
   for (int curFilename = 0; curFilename < numFiles; curFilename++)
   {
      array[curFilename] = orig.array[curFilename];
   }
   
   return *this;
}

I'm thinking that the problem has to be lying within the 'filename.cpp' instead of the 'filename.h' file: The insertion and removal SEEMS to be working okay, but the "checking whether file is valid" loop would be where my guess is.

Instead of using 

ifstream fin;
fin.open(...);
if (!fin || fin.fail())
{
   ...
}
fin.close();

I wouldn't mind using "exist()" (or is it "exists()" with an s?), but can't remember which library I need to include :-).

Indeterminatus: No real reason why I'm avoiding std::stuff other than the fact that I just want to make sure I'm understanding everything. (Better to reinvent the wheel so I can learn in the process, right?)

Thanks for your feedback; you were right about forgetting to return that value.

Does anyone have any ideas why this is skipping those files?

tobing

The problem is your usage of ifstream.

This works:

1Filename checkValidFilenames(int argc, char **argv)
2{
3 Filename validFiles;
4 ifstream fin;
5 
6 for (int currentFile = 1; currentFile < argc; currentFile++)
7 {
8 fin.open(argv[currentFile]);
9 
10 if (fin.fail())
11 {
12// fin.ignore();
13 fin.clear();
14 }
15 else
16 {
17 validFiles.insert(argv[currentFile]);
18 fin.close();
19 }
20 }
21
22 return validFiles;
23}

Seems that the ifstream doesn't open a file correctly if closed after the failed open, or when ignore has been used.

Would be even better this way:

1Filename checkValidFilenames(int argc, char **argv)
2{
3 Filename validFiles;
4 
5 for (int currentFile = 1; currentFile < argc; currentFile++)
6 {
7 ifstream fin;
8 fin.open(argv[currentFile]);
9 
10 if (!fin.fail())
11 {
12 validFiles.insert(argv[currentFile]);
13 fin.close();
14 }
15 }
16
17 return validFiles;
18}

ifstream.open just uses fopen inside (implementation dependent of course), so it might be more efficient to use fopen directly.

Another remark: Your Filename leaks memory. Look at operator=(). And, all the reallocation of memory is REALLY inefficient. Not that it matters with that tiny application, but handling memory like this can slow down your app to a halt.

TeamTerradactyl

I think I found the problem with the missing "if file is valid":

1StringArray checkValidFilenames(const int &argc, char **argv)
2{
3 StringArray validFiles;
4 ifstream fin;
5 
6 for (int currentFile = 1; currentFile < argc; currentFile++)
7 {
8 fin.open(argv[currentFile]);
9 
10 if (fin.is_open())
11 validFiles.insert(argv[currentFile]);
12 
13 fin.close();
14 }
15 
16 return validFiles;
17}

Thanks for your feedback on that.

As far as the memory leak, tobing, I'll take a look at it. I'm not very good at assignment operators yet, so thanks for showing where the problem lies.

-TT

Thread #588169. Printed from Allegro.cc