FileSystemWatcher created event

Question

I wrote a little function that monitors some directories. If a file gets created, it will check in the extension list for a match. Can you please tell me if I could make my function better in any way?

private void sysWatcher_Created(object sender, FileSystemEventArgs e) {

    try {

        if (File.Exists(e.FullPath)) {

            var fileName = Path.GetFileName(e.FullPath);
            var fullPath = e.FullPath;
            var changeType = e.ChangeType;
            var fileExtension = Path.GetExtension(e.FullPath);

            _extensionList.RemoveAll(r => string.IsNullOrEmpty(r.Trim()));
            _ignoreList.RemoveAll(r => string.IsNullOrEmpty(r.Trim()));

            // Checking for match in Extension List
            bool _extResult = _extensionList.Any(s => e.FullPath.Contains(s));

            if (_extResult) {

                // Checking for match in Ignore List
                bool _ignResult = _ignoreList.Any(s => e.FullPath.Contains(s));

                if (!_ignResult) {

                    if (!GeneralSettings.autoMode) { 

                        App.Current.Dispatcher.Invoke(delegate {

                            if (File.Exists(e.FullPath)) {
                                var fileInfo = new FileInfo(e.FullPath);
                                var wFunc = new WarningItem(e.FullPath, e.Name, AppSettings.SizeSuffix(fileInfo.Length),
                                    fileInfo.CreationTime.ToString("g"), fileInfo.Extension);

                                var wWin = new WarningWin();
                                wWin.init(wFunc);
                                wWin.ShowDialog();

                                // Add to Warning List and List Box 
                                _warningList.Add(new WarningItem {
                                    DirName = Path.GetDirectoryName(e.FullPath),
                                    CreationDate = DateTime.Now.ToString("g"),
                                    Extension = fileInfo.Extension,
                                    FullPath = e.FullPath,
                                    FileName = e.Name,
                                    FileSize = AppSettings.SizeSuffix(fileInfo.Length),
                                    StatusImage = new Uri("/Images/inWatch.Folder.png", UriKind.RelativeOrAbsolute)
                                });
                                lstWarning.ItemsSource = _warningList;
                                tabWarning.IsSelected = true;
                                cmdSearchWarnings.Visibility = Visibility.Visible;
                            }
                        });
                    }
                    else {

                        Scan.scanFile(e.FullPath);
                    } 
                }
            }
        }            
    }
    catch (Exception ex) {
        GeneralSettings.LogException(ex);
    }
}

Answer 1

You have a good example of the arrow antipattern here but this can be easily resolved by inverting the conditions and using return.

if (!File.Exists(e.FullPath)) { return; }

_extensionList.RemoveAll(r => string.IsNullOrEmpty(r.Trim()));
_ignoreList.RemoveAll(r => string.IsNullOrEmpty(r.Trim()));

// Checking for match in Extension List
bool _extResult = _extensionList.Any(s => e.FullPath.Contains(s));

if (!_extensionList.Any(s => e.FullPath.Contains(s))) { return ; }

if (_ignoreList.Any(s => e.FullPath.Contains(s))) { return; }

if (GeneralSettings.autoMode) 
{
     Scan.scanFile(e.FullPath);
     return;
}
App.Current.Dispatcher.Invoke(delegate {

                        if (File.Exists(e.FullPath)) {
                            var fileInfo = new FileInfo(e.FullPath);
                            var wFunc = new WarningItem(e.FullPath, e.Name, AppSettings.SizeSuffix(fileInfo.Length),
                                fileInfo.CreationTime.ToString("g"), fileInfo.Extension);
....
.... 
....}

In this way your code becomes more readable because of the reduced horizontal spacing.

---

About @janos point about the `Pointless Variables":

These variables are not only pointless but they are also never used.

Answer 2

Several odd elements in this code.

Pointless variables

These variables look pointless:

        var fullPath = e.FullPath;
        var changeType = e.ChangeType;

You could use e.FullPath and e.ChangeType where you need it, the extra local variables don't seem to have any purpose.

Garbage not filtered out soon enough

It's very strange to execute this code every time a file is created:

        _extensionList.RemoveAll(r => string.IsNullOrEmpty(r.Trim()));
        _ignoreList.RemoveAll(r => string.IsNullOrEmpty(r.Trim()));

Why do you need to filter out the garbage every time instead of doing it just once?

Even better: how did garbage every enter these lists? It would be better to prevent that, and organize your code in a way that it's impossible to enter garbage into these lists.

Lastly, the result of r.Trim() will never be null, so the null-check is pointless.

Why not use a Set ?

Instead of putting the extensions and ignored extensions into lists, it would be better to use sets instead. It will it a lot more efficient to check if they contain an element or not, without the need for lambdas.

Exception handling

The try block is to large. It's better to limit the scope of try blocks, to make it easier to see the piece of code where an operation might fail.

Instead of catching a generic Exception, it's better to catch the most specific kind of exception that can be thrown.