# Randomly choose a game executable from a folder 2.0

After asking Randomly choose a game executable from a folder, I applied the recommended changes. I also redesigned the program to make use of the MVVM (Model-View-Viewmodel) paradigm, using Prism 6.0. This was such a large change that I bumped the version number to 2.0.

The project can be found at https://github.com/nzall/GameRoulette. I will copy the relevant files below:

MainPage.xaml (.cs is basically empty and is not worth sharing):

<Page
x:Class="GameRoulette.MainPage"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:local="using:GameRoulette"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
xmlns:grv="using:GameRoulette.Views"
mc:Ignorable="d" Width="400" Height="400">
<Grid>
<grv:GameRouletteView></grv:GameRouletteView>
</Grid>
</Page>


GameRouletteView.xaml (again, associated .cs file is basically empty, so no point sharing):

<UserControl
x:Class="GameRoulette.Views.GameRouletteView"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:local="using:GameRoulette.Views"
xmlns:prism="using:Prism.Windows.Mvvm"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
mc:Ignorable="d"
d:DesignHeight="300"
d:DesignWidth="400"
prism:ViewModelLocator.AutoWireViewModel="True">
<Grid Background="White">
HorizontalAlignment="Left" Margin="110,50,0,0"
VerticalAlignment="Top" Height="40" Width="240"
Command="{Binding SelectCommand}"/>
<Button x:Name="btnChooseGame" Content="{Binding ChooseGameText}"
HorizontalAlignment="Left" Margin="110,150,0,0"
VerticalAlignment="Top" Width="240" Height="40"
Command="{Binding ChooseCommand}"
IsEnabled="{Binding ChooseGameEnabled}"/>
<ProgressRing HorizontalAlignment="Left" Margin="200,100,0,0"
VerticalAlignment="Top" RenderTransformOrigin="1.05,1.983"
Height="45" Width="45" IsActive="True" Visibility="{Binding ProgressRingVisibility}"/>
<Image x:Name="imgFileIcon" HorizontalAlignment="Left"
Source="{Binding FileThumbnail}"
Height="64" Margin="110,224,0,0"
VerticalAlignment="Top" Width="64" />
<TextBlock x:Name="lblFileName" HorizontalAlignment="Left"
Margin="179,224,0,0" TextWrapping="Wrap"
Text="{Binding Filename}"
VerticalAlignment="Top" Width="171" Height="64"/>
</Grid>
</UserControl>


GameRouletteViewModel.cs

using Prism.Mvvm;
using Windows.Storage;
using Windows.Storage.Pickers;
using Windows.UI.Xaml;
using Windows.UI.Xaml.Media;
using System;
using System.Collections.Generic;
using System.Linq;
using Windows.UI.Xaml.Media.Imaging;
using GameRoulette.Models;
using Prism.Commands;

namespace GameRoulette.ViewModels
{
public class GameRouletteViewModel : BindableBase
{

private ImageSource fileThumbnail;
private Visibility progressRingVisibility;
private bool chooseGameEnabled;
private string filename;
private string chooseGameText;
private GameRouletteModel Model;
public DelegateCommand SelectCommand { get; private set; }
public DelegateCommand ChooseCommand { get; private set; }

public GameRouletteViewModel()
{
progressRingVisibility = Visibility.Collapsed;
chooseGameEnabled = false;
ChooseGameText = "First select a folder";
filename = "";
FileThumbnail = new BitmapImage();
Model = new GameRouletteModel();
SelectCommand = DelegateCommand.FromAsyncHandler(SelectFolderAsync);
ChooseCommand = DelegateCommand.FromAsyncHandler(OnChooseGameClick);
}

{

FolderPicker folderPicker = new FolderPicker();
ProgressRingVisibility = Visibility.Visible;
StorageFolder selectedFolder = await folderPicker.PickSingleFolderAsync();

ChooseGameText = "retrieving files";

int fileCount = await Model.SelectGameExecutables(selectedFolder);

ProgressRingVisibility = Visibility.Collapsed;
ChooseGameEnabled = true;
}

{

Tuple<BitmapImage, string> fileProperties = await Model.ChooseGame();
FileThumbnail = fileProperties.Item1;
Filename = fileProperties.Item2;

}

public string ChooseGameText
{
get { return chooseGameText; }
set
{
SetProperty(ref chooseGameText, value);
}
}

public ImageSource FileThumbnail
{
get { return fileThumbnail; }
set
{
SetProperty(ref fileThumbnail, value);

}
}
public Visibility ProgressRingVisibility
{
get
{
return progressRingVisibility;
}
set
{
SetProperty(ref progressRingVisibility, value);

}
}
public bool ChooseGameEnabled
{
get { return chooseGameEnabled; }
set
{
SetProperty(ref chooseGameEnabled, value);

}
}
public string Filename
{
get { return filename; }
set
{
SetProperty(ref filename, value);

}
}
}
}


GameRouletteModel.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using Windows.Storage;
using Windows.UI.Xaml.Media.Imaging;

namespace GameRoulette.Models
{
class GameRouletteModel
{

string[] blacklist = { "uninst", "system", "redist", "framework", "physx", "builder", "processor", "helper" };
IList<StorageFile> Files;

{
IEnumerable<StorageFile> retrievedFiles = await folder.GetFilesAsync(Windows.Storage.Search.CommonFileQuery.OrderByName);
var fileCount = retrievedFiles.Count();

Files = retrievedFiles.Where((file, index) => ValidateFile(file, index, fileCount)).ToList();

return Files.Count;
}

private bool ValidateFile(StorageFile file, int index, int fileCount)
{
if (file.FileType != ".exe")
{
return false;
}
var lowerCaseFileName = file.Name.ToLowerInvariant();
return !blacklist.Any(e => lowerCaseFileName.Contains(e));
}

{
if (!Files.Any())
{
return null;
}
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);
return new Tuple<BitmapImage, string> (image, chosenGame.DisplayName);
}
}
}


2 of the 3 bugs from before are still present. I removed one of the bugs since it doesn't really

MAJOR: from what I can tell, it does not handle symbolic links well. I have several games symbolically linked from another drive, but they're not found.

MINOR: my blacklist is incomplete, but that's just a matter of adding the right items.

I have also noticed that it's quite slow compared to other similar software. For example, WinDirStat takes under 2 minutes to check my entire C: drive. This program isn't finished with that after 20 minutes AND makes the Runtime Broker use 4 GB of memory, which causes system warning for out of memory.

The first Major bug might be a limitation of GetFilesAsync. The Minor bug is just a matter of testing until a proper blacklist has been found.

the performance issues, however, are quite concerning. I am not sure if they are fixable though.

Special feedback I'm looking for on top of normal refactoring:

1. Have I implemented MVVM using Prism properly?
2. Are the performance issues solvable?
3. Is the major bug solvable?

<grv:GameRouletteView></grv:GameRouletteView>


The tag should be self-closing if there's nothing in it:

<grv:GameRouletteView />


You're using x:Name attributes, but the identifiers aren't used anywhere and can be discarded: that's a very often very good sign you're doing something right, when your XAML doesn't need named controls.

What I don't like though, is all these explicit margins: I don't know what your UI looks like, but it seems you could use better layouts; explicit margins everywhere are a sign that you're dragging-and-dropping your controls into the designer - that works, but you'll have a much better UI if you let WPF handle the layout; you're literally dumping your controls into that Grid with explicit positioning, as if drawing on a canvas.

You could start by defining rows and columns for your grid:

<Grid.RowDefinitions>
<RowDefinition Height="*" /> <!-- todo -->
</Grid.RowDefinitions>


And then use layout panels, for example StackPanel, wisely and as needed:

<StackPanel Grid.Row="1">
<!-- controls -->
</StackPanel>


Seeing a textbox's Width=171 in the markup, begs the question: why isn't it 170? With a structured layout, you get rid of arbitrary margins like 179,224,0,0, and let the controls take the space they're allowed to take - you can constrain grid rows/columns' MinHeight and/or MaxWidth to keep things under control, and the result is a view that adapts much better to resizing.

In other words, using explicit margins everywhere is the way of doing layout.

Code-wise, I don't like seeing a Tuple<T1,T2> in any API, even if it's internal. What you're returning from that ChooseGame method could have its own name:

internal class GameInfo
{
public string FileName { get; set; }
public BitmapImage Thumbnail { get; set; }
}


And then the signature and call sites become much more readable:

internal async Task<GameInfo> ChooseGame()


Tuple<BitmapImage, string> fileProperties = await Model.ChooseGame();
FileThumbnail = fileProperties.Item1;
Filename = fileProperties.Item2;


You could have that:

var info = await Model.ChooseGame();
FileThumbnail = info.Thumbnail;
Filename = info.FileName;


This is a problem:

Random generator = new Random();


It should be an instance-level field, not a new instance every time it's called.

Also, blacklist could be a private readonly IReadOnlyList<string> instead of a simple array.

Visibility should be bool IsVisible:

public Visibility ProgressRingVisibility
{
get
{
return progressRingVisibility;
}
set
{
SetProperty(ref progressRingVisibility, value);

}
}


I'm not sure why you're calling SetProperty here, why can't you just assign the value and OnPropertyChanged? Or is that some magic going on in BindableBase? I'm not sure I like that it needs that ref parameter, and SetProperty makes it look like you're assigning a DependencyProperty, which shouldn't be the case here.

The ViewModel exposing a bool Visibility will obviously break your binding:

Visibility="{Binding ProgressRingVisibility}"


The trick is to add a <UserControl.Resources> section where you give an x:Key to a BooleanToVisibilityConverter, like this:

<BooleanToVisibilityConverter x:Key="BoolToVisibility"/>


And then you update the binding with the converter:

Visibility="{Binding ProgressRingVisibility, Converter={StaticResource BoolToVisibility}}"


The concept of Visibility shouldn't be leaking into your ViewModel; it's really a presentation thing. Hence I'd probably expose it as some bool IsBusy or whatever fits the bill here.

• BindableBase is a class from Prism, a Microsoft-generated NuGet package that abstracts away all that OnPropertyChanged and handler boilerplate code for WPF binding. Also, it's a UWP project, and from what I've developed since the question, I seem to remember not coming across the BooleanToVisibilityConverter class. – Nzall Feb 16 '16 at 19:40
• At any rate, I've had my brother go over my MVVM implementation and apparently, the way I implemented it is not even close to true MVVM. what I currently have is basically the classic 3 layers model. I'm going to rewrite the entire project a 3rd time for TRUE MVVM implementation. For example, Model would be a file, and View Model would be the class that selects the file. – Nzall Feb 16 '16 at 19:42
• The converter is merely an implementation of IValueConverter` - make your own if it's not available! Also, design patterns are a mean of communication, not a coding standard. What matters is whether your code is robust, readable, maintainable and flexible, not whether it adheres to such or such blog's definition of what MVVM should be. – Mathieu Guindon Feb 16 '16 at 19:48