Sign in to follow this  
szecs

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 this post


    Link to post
    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 this post


    Link to post
    Share on other sites
    szecs    2990
    Quote:
    Original post by ApochPiQ
    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]
    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 this post


    Link to post
    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 this post


    Link to post
    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 this post


    Link to post
    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 this post


    Link to post
    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 this post


    Link to post
    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 this post


    Link to post
    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 this post


    Link to post
    Share on other sites
    Zahlman    1682
    Since you didn't ask:


    #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
    #endif
    typedef 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 this post


    Link to post
    Share on other sites

    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