Solved

a findfiles question

Posted on 2001-07-25
8
433 Views
Last Modified: 2010-04-02
i wrote a piece of code that searches for a file.  I would like someone to look through this code and see if they can see anything wrong with it.  could someone improve it for me?  What its suppose to do is search through a given directory and everything below it for a file. it really doesn't work that way right now.  so it needs improvement.

WIN32_FIND_DATA fileFindData;

CString FindFiles(CString strStartDir, CString strFileName)
{
     HANDLE hFile = FindFirstFile(strStartDir + "\\" + strFileName, &fileFindData);
     CString csFileName(fileFindData.cFileName);
     CString csSubDir(""), strFound("ERROR - file not found");
         
     if (hFile != INVALID_HANDLE_VALUE)
     {
          do
          {
               csFileName = fileFindData.cFileName;
               if ((fileFindData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) && (csFileName != ".") && (csFileName != ".."))
               {
                    csSubDir = strStartDir + "\\" + csFileName;
                    FindFiles(csSubDir);
               }

               if (csFileName.Find(".txt") != -1)
                    strFound = strStartDir + "\\" + fileFindData.cFileName;
          } while (FindNextFile(hFile, &fileFindData) && strFound == "ERROR - file not found");
          FindClose(hFile);    
     }
     return strFound;
}
0
Comment
Question by:simongod
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 4
  • 3
8 Comments
 
LVL 22

Expert Comment

by:nietod
ID: 6319137
The basic idea looks fine.  

One problem I see is that you are not filtering out the two "special" directories "." and "..".   Every directory has an entry called "." that is a directory.  this entry ferers to the directory itself!.  So if you come to direcotry  "." and then seearch "." you will be searching the directory you were already searching.  And that dearch will find "." and search the directory again.  and again and again.  You will recurse forever.

Similarly all directories except for the root directory has an entry called  ".." which is for the parent directory.  if you search a directory and encouter ".." you will then search the parent directory, which you were searching earlier.  That will then lead to this directory, and then back to the parent,  then to this director and so on forever.

So you need to skipp these two directories.

Other than that it looks fine.  What is the problem you are getting?  maybe that was the problem?
0
 
LVL 22

Expert Comment

by:nietod
ID: 6319159
opps,   No you have that in there don't you.  I missed that.


how can this be right

>>                  FindFiles(csSubDir);

The FindFiles function takes 5wo paremeters, you are passing only one.  that shouldn't even compile.

That should be

                  FindFiles(csSubDir,strFileName);

Also the loop condition

>>        } while (FindNextFile(hFile, &fileFindData) && strFound == "ERROR - file not found");

is very strange.  Using a string to control a loop like that is very unuusual and pretty innefficient.  Why are you doing that?

0
 
LVL 2

Author Comment

by:simongod
ID: 6319203
sorry it seems that i forgot to mention a few things.

the findfiles can accept 0,1or 2 arguments.  I have the function declared with the two arguments equaling NULL.  Does this help?

as for the string controling the loop.  The default for the strFound is "ERROR - file not found".  what the loop is looking for is for that to change.  if it changes then the file and its directory were found.

does that help?


The problem that i am having is when the file is found it jumps out of that while loop and returns the file name (return strFound;).  After it has completed the the next line of execution is the FindFiles(csSubDir) that is located in the if.  That seems strange and causes and assertion
0
Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
LVL 2

Expert Comment

by:The Master
ID: 6319257
The problem looks to be in your while statement.  It's going to analyze both conditions in order before evaluating the entire while().  In other words, it's going to call FindNextFile() before it evaluates strFound.  Change your while() statement to...

} while (strFound == "ERROR - file not found" && FindNextFile(hFile, &fileFindData));

... and that should do the trick.

Hope this helps!
0
 
LVL 2

Expert Comment

by:The Master
ID: 6319263
Oops - nevermind.  I thought you had named your func FindNextFile as well.  My bad.
0
 
LVL 22

Expert Comment

by:nietod
ID: 6319278
>> he findfiles can accept 0,1or 2 arguments.  I have the function
>> declared with the two arguments equaling
>> NULL.  Does this help?
Not at all.

I don;t see how you could expect it t work when an argument is not specified.  i..e I don't know what you want to have happen AND  I don't think I see any code that looks for this case and will make something happen in this case.

I woudl expect to see code like

if (strStartDir.length() == 0); // If no start directory, then
   strStartDir = "\\"; // use the root directory.

if (strFileName.length() == 0); // If no file name, then
   strFileName = "*.*"; // Look for any file.


Now that I think about it, the code

   FindFirstFile(strStartDir + "\\" + strFileName, &fileFindData);

is probably wrong too.  This will search that directory for any file name matching strFileName's contents.  That is fine since that is ultimately what you want. BUT it will also return only directories that match that as well.  You want to find ALL directory names and any files names that match the search skeleton.   There is no way to do that using FindFirstFile, so you have to do it manually.  You need to search the directory for *.*.  This will get your all direcotries--whic is what you want.  It also gets you all files, you will need to test each file name to see if it matches the specified search string.

OR

use two passes per directory.  First search the directory fir any files that match the search string.  Then if there is none found, search the drectory in a 2nd loop for *.* and look for entries that are sub-directories.  Then search these.    

This 2nd method is the one I usually use.

>>  if it changes then the file and its directory were found.
Why not use a boolean varaible to indicate when a match is found.  Its likely to be 100s or even 1000s of times faster.

Finally I don't see how its supposed to return information to the caller.  What is it supposed to return?  When you mke a recusrive call you ignore the return value. That is probalby wrong.

It might help if you explained what it is supposed to do.  In detail.   its very hard to understand what the goal is.
0
 
LVL 22

Expert Comment

by:nietod
ID: 6319293
>> The problem looks to be in your while statement
It looks to me like there are little problems everywhere.  Its unclear what is a problem and what isn;t since its not clearwhat its supposed to do, but lots of things don't add up that is for sure.
0
 
LVL 2

Accepted Solution

by:
The Master earned 100 total points
ID: 6319309
Ok, let's take another stab at this.  I don't understand your last comment - that you have the function declared with the arguments equalling NULL.  Are these CString pointers you're passing in?

However, if the FindFiles(csSubDir) actually recurses into the same function, then it looks like a problem.  Let's say you have the following directories on your hard drive:

C:\test
C:\test\subdir

and you are looking in "test" for a file with the extension ".txt", but the only text file exists in "test\subdir".  So your code will recurse into FindFiles again (via the FindFiles(csSubDir) in the if block), find the text file, and return.  But it's returning to the original FindFiles(), just after FindFiles(csSubDir).  Your strFound that is local to that function is still going to equal "ERROR - file not found".

Try changing that line in the if block to:

strFound = FindFiles(csSubDir);

Good luck!
0

Featured Post

Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

In days of old, returning something by value from a function in C++ was necessarily avoided because it would, invariably, involve one or even two copies of the object being created and potentially costly calls to a copy-constructor and destructor. A…
Go is an acronym of golang, is a programming language developed Google in 2007. Go is a new language that is mostly in the C family, with significant input from Pascal/Modula/Oberon family. Hence Go arisen as low-level language with fast compilation…
The goal of the tutorial is to teach the user how to use functions in C++. The video will cover how to define functions, how to call functions and how to create functions prototypes. Microsoft Visual C++ 2010 Express will be used as a text editor an…
The viewer will learn how to pass data into a function in C++. This is one step further in using functions. Instead of only printing text onto the console, the function will be able to perform calculations with argumentents given by the user.

738 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question