# Listing files by last write time

## Recommended Posts

szecs    2990
A want to list files by their last write time. I separate the file names (without extensions) with a newline character. Problem: sorting is not good, otherwise works fine (returns exact file number, cuts extensions fine, handles no files fine, adds newlines fine) Code:
int ListSaveFiles(char *out)
{
int fileNumber = 1;

WIN32_FIND_DATA findFileData[20];
int index[20];
HANDLE hFind = FindFirstFile((LPCSTR)"*.sav", &findFileData[0]);

*out = '\0';

if( hFind  == INVALID_HANDLE_VALUE )
return 0;
while( fileNumber < 20 && FindNextFile(hFind, &findFileData[fileNumber]) )
fileNumber++;

FindClose(hFind);

for( int i = 0; i < fileNumber; i++ )
index[i] = i;

for( int i = 0; i < fileNumber-1; i++ )
{
for( int k = i+1; k < fileNumber; k++ )
{
int store;

if( findFileData[k].ftLastWriteTime.dwHighDateTime <
findFileData[i].ftLastWriteTime.dwHighDateTime )
{
store = index[i];
index[i] = index[k];
index[k] = store;
}
else if( findFileData[k].ftLastWriteTime.dwHighDateTime ==
findFileData[i].ftLastWriteTime.dwHighDateTime
&& findFileData[k].ftLastWriteTime.dwLowDateTime <
findFileData[i].ftLastWriteTime.dwLowDateTime )
{
store = index[i];
index[i] = index[k];
index[k] = store;
}
}
}

strncpy(out,&findFileData[index[0]].cFileName[0],
strlen(&findFileData[index[0]].cFileName[0])-4);
out[strlen(&findFileData[index[0]].cFileName[0])-4] = '\0';

for( int i = 1; i < fileNumber; i++ )
{
strcat(out,"\n");
strncat(out,&findFileData[index[i]].cFileName[0],
strlen(&findFileData[index[i]].cFileName[0])-4);
}

return fileNumber;
}

My guesses:
• I can't make bubble sort
• I can't swap 2 values
• some other idiotic bug
• I misunderstood the high, low time parts
• Resolution of the write time isn't good enough (NTFS) Don't critique the code, I want to make it work first. Maybe I should store the exact time in the file? Thanks for answers in advance!

##### Share on other sites
ApochPiQ    23064
Your handling of the file timestamp structure is incorrect. Compare with this example.

Also, your code itself is highly problematic. I won't get into details since you asked for no critiques on the code, but there's a few much safer and simpler ways to get at what you want to do [smile]

##### Share on other sites
szecs    2990
Quote:
 Original post by ApochPiQYour handling of the file timestamp structure is incorrect. Compare with this example.Also, your code itself is highly problematic. I won't get into details since you asked for no critiques on the code, but there's a few much safer and simpler ways to get at what you want to do [smile]
Okay, maybe I would accept some critique too...

I saw that example, but I though it's easier to work with two values, instead of all the date/time stuff.

Or maybe I could convert it to a string (as in the example), and do a lexical sort?

(I know that strcat/strcopy etc has a safer version too.)

##### Share on other sites
ApochPiQ    23064
Why not just use the CompareFileTime function directly? You can wrap CompareFileTime into a comparison predicate and then use std::multimap to order the files for you based on their relative timestamps.

##### Share on other sites
szecs    2990
Yup, I haven't have to much sleep.

Thanks for that (I have no idea why I didn't think, that this comparison is common enough to have its own function)

##### Share on other sites
szecs    2990
Nope.
CompareFileTime gives exactly the same results. I exit and run the program before I make the comparison (and use fclose too), so I'm not comparing opened files.

Code:
int ListSaveFiles(char *out){	int fileNumber = 1;	WIN32_FIND_DATA findFileData[20];	int index[20];	HANDLE hFind = FindFirstFile((LPCSTR)"*.sav", &findFileData[0]);	*out = '\0';	if( hFind  == INVALID_HANDLE_VALUE )		return 0;	while( fileNumber < 20 && FindNextFile(hFind, &findFileData[fileNumber]) )		fileNumber++;	FindClose(hFind);	for( int i = 0; i < fileNumber; i++ )		index[i] = i;	for( int i = 0; i < fileNumber-1; i++ )	{		for( int k = i+1; k < fileNumber; k++ )		{				int store;			if( CompareFileTime(&findFileData[k].ftLastWriteTime,					&findFileData[i].ftLastWriteTime) > 0 )			{				store = index[i];				index[i] = index[k];				index[k] = store;			}		}	}	strncpy(out,&findFileData[index[0]].cFileName[0],			strlen(&findFileData[index[0]].cFileName[0])-4);	out[strlen(&findFileData[index[0]].cFileName[0])-4] = '\0';	for( int i = 1; i < fileNumber; i++ )	{		strcat(out,"\n");		strncat(out,&findFileData[index[i]].cFileName[0],			strlen(&findFileData[index[i]].cFileName[0])-4);	}	return fileNumber;}

File write:
...
fp = fopen(filename_ext,"wb");
...
fclose(fp);

I use C, and I don't know how to use std.

##### Share on other sites
szecs    2990
I still don't know what can be the problem. Thanks

##### Share on other sites
szecs    2990

##### Share on other sites
Gage64    1235
I think the sorting is incorrect. You should use the indices in the 'index' array when comparing the entries in 'findFileData'. Something like:

for( int i = 0; i < fileNumber-1; i++ ){	for( int k = i+1; k < fileNumber; k++ )	{			int store;		if( CompareFileTime(&findFileData[index[k]].ftLastWriteTime,				&findFileData[index[i]].ftLastWriteTime) > 0 )		{			store = index[i];			index[i] = index[k];			index[k] = store;		}	}}

##### Share on other sites
szecs    2990
Yes.

No matter how long I'm staring at a code, how many times I start a new task/go for a walk/sleep/whatever, if I don't see the obvious, the I don't see the obvious.

[embarrass]

Thanks for helping out one of your blind fellow-beings.

##### Share on other sites
Gage64    1235
Glad to hear you got it to work.

One thing I wanted to suggest is that you should try to divide your code into several functions, one for each logical task.

For example, here there are basically 3 tasks:

1) Collect file information.
2) Sort the files.
3) Build a string of the file names.

You can even put the swapping in its own function:

void swap(int arr[], int i, int j) {   int temp = arr[i];    arr[i] = arr[j];    arr[j] = temp;}

If each function is mostly independent (which also means that it doesn't use global variables), then testing also becomes easier because you can test each function individually, instead of testing the program all at once.

##### Share on other sites
szecs    2990
Thanks for the advice, but the sorting was a later added feature. The data collection, and string building worked fine before.
I'm not bad at debugging, I just didn't assume I can make a fault in a trivial task like that (I guess that's a big fault).

You see, I can debug much worse stuff than that, it's the simplest thing I miss sometimes (if statement instead of else if for example).
Even if I debugged the sorting individually, I would never spot the bug. I would have jumped out of the window.

##### Share on other sites
Zahlman    1682

#include <string>#include <algorithm>#include <ostream>#include <iterator>#include <vector>// I'll even handle unicode stuff for you:#ifdef UNICODE#define Character wchar_t#define ostream std::wostream#else#define Character char#define ostream std::ostream#endiftypedef std::basic_string<Character> string;string name_of_file(const WIN32_FIND_DATA& file) {  const Character* data = file.cFileName;  return string(data, char_traits<Character>::length(data) - 4);  // We don't even need strlen() ;)}bool earlier_last_write(const WIN32_FIND_DATA& a, const WIN32_FIND_DATA& b) {  return a.ftLastWriteTime < b.ftLastWriteTime;}class file_iterator {  HANDLE h; // initial result  bool success; // most recent result  WIN32_FIND_DATA found;  file_iterator(const file_iterator&);  file_iterator& operator=(const file_iterator&);  public:  file_iterator(): h(NULL), success(false) {} // represents the "end" iterator.  file_iterator(const string& glob) {    h = FindFirstFile(glob.c_str(), &found);    success = h;  }  // A bit of a hack.  bool operator==(const file_iterator& other) {    if (!(success || other.success)) { return true; } // both are at-end    return h == other.h && found == other.found;  }  WIN32_FIND_DATA& operator*() { return found; }  WIN32_FIND_DATA operator*() const { return found; }  file_iterator& operator++() { success = FindNextFile(h, found); return *this; }  ~file_iterator() { if (h) { FindClose(h); } }};int ListSaveFiles(ostream& out) {  std::vector<WIN32_FIND_DATA> files(file_iterator("*.sav"), file_iterator());  std::sort(files.begin(), files.end(), earlier_last_write);  std::transform(    files.begin(), files.end(),    ostream_iterator<string>(out, _T("\n")),    name_of_file  );  return files.size();}// Edited several times... proof-reading is a lot of work, especially when// I'm not properly set up to compile Windows applications...

##### Share on other sites
szecs    2990
Er... thanks!