C++ Windows Directory Watcher

Question

This was a small project I did the other day because I wanted to explore a solution to hot-reload some files in another project.

My requirements were:

• Act in it's own thread
• Be able to watch specific file extensions
• Work on Windows

So since I knew my application would only run on windows I decided to use WinApi.

I am mostly concerned about modern C++ stuff and particularly in handling the thread but if there are any other glaring issues obviously any help is appreciated. I know the FileAction class enum might be a little strange but it just makes the switch statement prettier, not sure if it's frowned upon.

Last note before code is the reason I deleted the copy constructors is because std::thread is not copyable (afaik) but it can be moved so it didn't make sense to allow a copy of the DirectoryWatcherWin class if I couldn't copy all of it's members - hope that's correct..

DirectoryWatcherWin.h

#pragma once
#include <atomic>
#include <thread>
#include <string>
#include <vector>

namespace mikand
{
    enum class FileAction {
        CREATED = FILE_ACTION_ADDED,
        REMOVED = FILE_ACTION_REMOVED,
        MODIFIED = FILE_ACTION_MODIFIED
    };

    class DirectoryWatcherWin
    {
    public:
        using DWWCallback = std::function<void(std::string, FileAction)>;
    private:
        HANDLE m_DirectoryHandle;
        std::atomic<bool> m_Running;
        std::thread m_Thread;
        std::string m_DirectoryToWatch;
        std::vector<std::string> m_Extensions;
        DWWCallback m_Callback;
    public:
        DirectoryWatcherWin(const std::string& directoryToWatch, DWWCallback callback);
        DirectoryWatcherWin(const std::string& directoryToWatch, const std::vector<std::string>& extensionsToWatch, DWWCallback callback);

        DirectoryWatcherWin(const DirectoryWatcherWin& other) = delete;
        DirectoryWatcherWin& operator=(const DirectoryWatcherWin& other) = delete;

        DirectoryWatcherWin(DirectoryWatcherWin&& other) = default;
        DirectoryWatcherWin& operator=(DirectoryWatcherWin&& other) = default;

        ~DirectoryWatcherWin();

        void start();
        void stop();
    private:
        void watch();
    };
}

DirectoryWatcherWin.cpp

#include "pch.h"
#include "DirectoryWatcherWin.h"
#include "StringUtil.h"

namespace mikand
{
    DirectoryWatcherWin::DirectoryWatcherWin(const std::string& directoryToWatch, DWWCallback callback)
        : m_DirectoryToWatch(directoryToWatch), m_Callback(callback)
    {

    }

    DirectoryWatcherWin::DirectoryWatcherWin(const std::string& directoryToWatch, const std::vector<std::string>& extensionsToWatch, DWWCallback callback)
        : m_DirectoryToWatch(directoryToWatch), m_Extensions(extensionsToWatch), m_Callback(callback)
    {

    }

    DirectoryWatcherWin::~DirectoryWatcherWin()
    {
        if (m_Thread.joinable())
        {
            stop();
        }
    }

    void DirectoryWatcherWin::start()
    {
        std::wstring dirWStr = std::wstring(m_DirectoryToWatch.begin(), m_DirectoryToWatch.end());
        m_DirectoryHandle = CreateFile(dirWStr.c_str(),
            GENERIC_READ,
            FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
            NULL,
            OPEN_EXISTING,
            FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OVERLAPPED,
            NULL);
        m_Running = true;
        m_Thread = std::thread(&DirectoryWatcherWin::watch, this);
    }

    void DirectoryWatcherWin::stop()
    {
        m_Running = false;
        CancelIoEx(m_DirectoryHandle, 0);
        m_Thread.join();
        CloseHandle(m_DirectoryHandle);
    }

    void DirectoryWatcherWin::watch()
    {
        while (m_Running.load())
        {
            std::vector<BYTE> byteBuffer(4096);
            DWORD bytesReturned;
            OVERLAPPED overlapped = { 0 };
            overlapped.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);

            ReadDirectoryChangesW(m_DirectoryHandle,
                &byteBuffer[0],
                byteBuffer.size(),
                TRUE,
                FILE_NOTIFY_CHANGE_LAST_WRITE | FILE_NOTIFY_CHANGE_CREATION | FILE_NOTIFY_CHANGE_FILE_NAME,
                &bytesReturned,
                &overlapped,
                NULL);

            GetOverlappedResult(m_DirectoryHandle, &overlapped, &bytesReturned, TRUE);

            FILE_NOTIFY_INFORMATION *fni = (FILE_NOTIFY_INFORMATION*)(&byteBuffer[0]);
            do
            {
                std::string filenameStr = StringUtil::wcharToString(fni->FileName, fni->FileNameLength);
                if (m_Extensions.size() > 0)
                {
                    for (auto& extension : m_Extensions)
                    {
                        if (StringUtil::endsWith(filenameStr, extension))
                        {
                            m_Callback(filenameStr, FileAction(fni->Action));
                            break;
                        }
                    }
                }
                else
                {
                    m_Callback(filenameStr, FileAction(fni->Action));
                }
                fni = (FILE_NOTIFY_INFORMATION*)((BYTE*)(fni) + fni->NextEntryOffset);
            } while (fni->NextEntryOffset != 0);
        }
    }
}

StringUtil.h

namespace mikand
{
    class StringUtil
    {
    public:

        inline static bool endsWith(const std::string& str, const std::string& suffix)
        {
            return (str.length() >= suffix.length()) && (str.compare(str.length() - suffix.length(), suffix.length(), suffix) == 0);
        }

        inline static bool startsWith(const std::string& str, const std::string& prefix)
        {
            return (str.length() >= prefix.length()) && (str.compare(0, prefix.size(), prefix) == 0);
        }

        inline static std::string wcharToString(wchar_t* in, unsigned long length)
        {
            if (length < 1)
            {
                return std::string();
            }
            std::wstring_convert<std::codecvt_utf8<wchar_t>, wchar_t> converter;
            std::wstring tempWstr(in, length);
            std::string s = converter.to_bytes(tempWstr);
            int i = s.length() - 1;
            while (s.at(i) == '\0')
            {
                s.pop_back();
                i--;
            }
            return s;
        }
// UNUSED METHODS OMITTED
    };
}

pch.h

#ifndef PCH_H
#define PCH_H

#include <windows.h>
#include <string>
#include <vector>
#include <array>
#include <locale>
#include <codecvt>
#include <iostream>


#endif

main.cpp

#include "pch.h"
#include "DirectoryWatcherWin.h"

using namespace mikand;

void printFilename(std::string filename, FileAction action)
{
    switch (action)
    {
    case FileAction::CREATED:
        std::cout << "Created: " << filename << std::endl;
        break;
    case FileAction::REMOVED:
        std::cout << "Removed: " << filename << std::endl;
        break;
    case FileAction::MODIFIED:
        std::cout << "Modified: " << filename << std::endl;
        break;
    default:
        break;
    }
}

int main()
{
    DirectoryWatcherWin::DWWCallback callback = printFilename;
    DirectoryWatcherWin watcher("C:\\temp", std::vector<std::string>{".txt"}, callback);

    watcher.start();

    std::string waitForPress;
    std::getline(std::cin, waitForPress);

    watcher.stop();

    system("pause");
}

Answer 1

Style

• Subjective but don't indent on namespaces.

• I would group the ctors and operators together instead of grouping them by their status (default/delete).

• Your ctor is a bit hard to read. Consider formatting it differently. E.g.:

  DirectoryWatcherWin::DirectoryWatcherWin(
          const std::string& directoryToWatch, 
          const std::vector<std::string>& extensionsToWatch, 
          DWWCallback callback)
      : m_DirectoryToWatch(directoryToWatch)
      , m_Extensions(extensionsToWatch)
      , m_Callback(callback)
  {}    
  

  This way the parameters are visually separated from the member init list and you don't break any line length recommendations either.

---

Code

• We need to talk about your interface.
  Generally you should start with public and private should come last. Also do not use those keywords more than once per class definition.
  I would probably either ditch the DWWCallback declaration, or move it into the namespace, seeing as there is probably no use-case where people use your namespace but not the DWWCallback.

• Prefer prefix over postfix.

• Prefer using \n over std::endl.

• Don't using namespace. Prefixing your code with the namespace is really not too much to ask.

• for (auto& extension : m_Extensions)
  

  Consider using const in loops like this if you don't intend to modify the loop variable in the body.

• If you have strings that you don't modify consider using string_view as it can be faster.

• When you want to initialize a struct and all its members consider using foostruct = {}; See this for more info.

• std::wstring dirWStr = std::wstring(m_DirectoryToWatch.begin(), m_DirectoryToWatch.end());
  

  I don't quite follow the idea behind this. Can you not pass this directly as something else like maybe mystring_view.data()? If it has to be a wstr why not keep it as such in the class as well?