HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Randomly choose a game executable from a folder

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
chooseexecutablerandomlygamefolderfrom

Problem

I've written a Universal Windows Application which allows you to select a folder and shows the icon and name of a random executable from anywhere in that folder. The intent is to be a Game Roulette, where if you install all your games in a certain folder, it will tell you what game to play.

MainPage.xaml:


    
        

        
        
        
           
      


MainPage.xaml.cs

```
using System;
using System.Collections.Generic;
using System.Linq;
using Windows.UI.Xaml;
using Windows.UI.Xaml.Controls;
using Windows.UI.Xaml.Media;
using Windows.UI.Xaml.Media.Imaging;
using Windows.Storage.Pickers;
using Windows.Storage;
using System.ComponentModel;

namespace GameRoulette
{
public sealed partial class MainPage : Page
{
private IEnumerable Files;
public MainPageViewModel PageViewModel { get; set; }
int i;
int fileCount;
int exeFileCount;
public MainPage()
{
InitializeComponent();
PageViewModel = new MainPageViewModel();
i = 0;
DataContext = PageViewModel;
}
private async void SelectGames_Click(object sender, RoutedEventArgs e)
{
FolderPicker folderPicker = new FolderPicker();
folderPicker.FileTypeFilter.Add(".exe");
PageViewModel.ProgressRingVisibility = Visibility.Visible;
StorageFolder selectedFolder = await folderPicker.PickSingleFolderAsync();
PageViewModel.ChooseGameText = "retrieving files";
IEnumerable retrievedFiles = await selectedFolder.GetFilesAsync(Windows.Storage.Search.CommonFileQuery.OrderByName);
fileCount = retrievedFiles.Count();
Files = retrievedFiles.Where(file => ValidateFile(file)).ToList();
if (!Files.Any())
{
PageViewModel.Filename = "No executables found.";
return;
}
exeFileCount = Files.Count();
PageViewMod

Solution

A few things noticed by just looking over the code in no particular order:

-
The blacklist should be a static readonly class member - there is no need to create the array on every call to ValidateFile.

-
The variable i should not be a class member. Where has an overload which passes the current index along so you should use that instead. Also fileCount should not be a class member since it's only used to track progress. The current index and total count should be passed in as parameters to the ValidateFile method rather than being class state.

-
Files gets set as ToList() yet you have another ToList() call on it in the ChooseGame_Click method. This will create another copy of the whole list and wastes time and memory for no reason at all. Just have the type as IList instead of IEnumerable. In that case exeFileCount becomes obsolete.

-
In ValidateFile the filename gets converted to lower case for every entry in the blacklist - again waste of memory and time. Just store the lowered file name in a local variable and compare against it. Also some LINQ could be employed to make this a bit more succinct.

-
The canonical way to invoke an event handler was this:

var handler = MyEventHandler;
if (handler != null)
{
    handler(this, new MyEventArgs());
}


This avoids a potential race where the last handler unsubscribes after checking against null but before actually executing the handler.

In C# 6.0 you can simply use the Null-conditional operator:

MyEventHandler?.Invoke(this, new MyEventArgs())


A simple rewrite of MainPage would look like this:

public sealed partial class MainPage : Page
{
    private IList Files;
    public MainPageViewModel PageViewModel { get; set; }

    public MainPage()
    {
        InitializeComponent();
        PageViewModel = new MainPageViewModel();
        DataContext = PageViewModel;
    }
    private async void SelectGames_Click(object sender, RoutedEventArgs e)
    {
        FolderPicker folderPicker = new FolderPicker();
        folderPicker.FileTypeFilter.Add(".exe");
        PageViewModel.ProgressRingVisibility = Visibility.Visible;
        StorageFolder selectedFolder = await folderPicker.PickSingleFolderAsync();
        PageViewModel.ChooseGameText = "retrieving files";

        var retrievedFiles = await selectedFolder.GetFilesAsync(Windows.Storage.Search.CommonFileQuery.OrderByName);
        var fileCount = retrievedFiles.Count;

        Files = retrievedFiles.Where((file, index) => ValidateFile(file, index, fileCount)).ToList();
        if (!Files.Any())
        {
            PageViewModel.Filename = "No executables found.";
            return;
        }

        PageViewModel.ProgressRingVisibility = Visibility.Collapsed;
        PageViewModel.ChooseGameText = string.Format("{0} games found. Click here to choose your game.", exeFileCount);
        PageViewModel.ChooseGameEnabled = true;
    }

    static readonly string[] blacklist = { "uninst", "system", "redist" };

    private bool ValidateFile(StorageFile file, int currentIndex, int totalNumberOfFiles)
    {
        PageViewModel.ChooseGameText = string.Format("{0} of {1} files checked.", currentIndex, totalNumberOfFiles);
        if (file.FileType != ".exe")
        {
            return false;
        }
        var lowerCaseFilename = file.Name.ToLowerInvariant();
        return blacklist.Any(e => lowerCaseFilename.Contains(e));
    }

    private async void ChooseGame_Click(object sender, RoutedEventArgs e)
    {
        if (!Files.Any())
        {
            return;
        }
        Random generator = new Random();
        int index = generator.Next(0, Files.Count - 1);
        StorageFile chosenGame = Files[index];
        Windows.Storage.FileProperties.StorageItemThumbnail thumbnail = await chosenGame.GetScaledImageAsThumbnailAsync(Windows.Storage.FileProperties.ThumbnailMode.SingleItem, 64);
        BitmapImage image = new BitmapImage();
        image.SetSource(thumbnail);
        PageViewModel.FileThumbnail = image;
        PageViewModel.Filename = chosenGame.DisplayName;
    }
}


This could probably still be improved but it's somewhat cleaner now.

Code Snippets

var handler = MyEventHandler;
if (handler != null)
{
    handler(this, new MyEventArgs());
}
MyEventHandler?.Invoke(this, new MyEventArgs())
public sealed partial class MainPage : Page
{
    private IList<StorageFile> Files;
    public MainPageViewModel PageViewModel { get; set; }

    public MainPage()
    {
        InitializeComponent();
        PageViewModel = new MainPageViewModel();
        DataContext = PageViewModel;
    }
    private async void SelectGames_Click(object sender, RoutedEventArgs e)
    {
        FolderPicker folderPicker = new FolderPicker();
        folderPicker.FileTypeFilter.Add(".exe");
        PageViewModel.ProgressRingVisibility = Visibility.Visible;
        StorageFolder selectedFolder = await folderPicker.PickSingleFolderAsync();
        PageViewModel.ChooseGameText = "retrieving files";

        var retrievedFiles = await selectedFolder.GetFilesAsync(Windows.Storage.Search.CommonFileQuery.OrderByName);
        var fileCount = retrievedFiles.Count;

        Files = retrievedFiles.Where((file, index) => ValidateFile(file, index, fileCount)).ToList();
        if (!Files.Any())
        {
            PageViewModel.Filename = "No executables found.";
            return;
        }

        PageViewModel.ProgressRingVisibility = Visibility.Collapsed;
        PageViewModel.ChooseGameText = string.Format("{0} games found. Click here to choose your game.", exeFileCount);
        PageViewModel.ChooseGameEnabled = true;
    }

    static readonly string[] blacklist = { "uninst", "system", "redist" };

    private bool ValidateFile(StorageFile file, int currentIndex, int totalNumberOfFiles)
    {
        PageViewModel.ChooseGameText = string.Format("{0} of {1} files checked.", currentIndex, totalNumberOfFiles);
        if (file.FileType != ".exe")
        {
            return false;
        }
        var lowerCaseFilename = file.Name.ToLowerInvariant();
        return blacklist.Any(e => lowerCaseFilename.Contains(e));
    }

    private async void ChooseGame_Click(object sender, RoutedEventArgs e)
    {
        if (!Files.Any())
        {
            return;
        }
        Random generator = new Random();
        int index = generator.Next(0, Files.Count - 1);
        StorageFile chosenGame = Files[index];
        Windows.Storage.FileProperties.StorageItemThumbnail thumbnail = await chosenGame.GetScaledImageAsThumbnailAsync(Windows.Storage.FileProperties.ThumbnailMode.SingleItem, 64);
        BitmapImage image = new BitmapImage();
        image.SetSource(thumbnail);
        PageViewModel.FileThumbnail = image;
        PageViewModel.Filename = chosenGame.DisplayName;
    }
}

Context

StackExchange Code Review Q#118627, answer score: 3

Revisions (0)

No revisions yet.