This is a follow up to this question.
I have implemented changes to the code as they were suggested in the previous question and have made a few changes of my own.
But it has been mentioned that the code snippet posted in the previous question was not sufficient to considerably affect code performance. So here, I'm posting all relevant code relating to the project.
I'd appreciate help with a few more things. They are after all the code (for a better context).
Category.cs
using System.Collections.ObjectModel;
namespace AnimeAssessor
{
public class Category
{
public string Name { get; private set; }
public long TotalSize { get; private set; }
public long AverageSize { get { return NumberOfFiles > 0 ? (TotalSize / NumberOfFiles) : 0; } }
public int NumberOfFiles { get { return ListOfFiles.Count > 0 ? ListOfFiles.Count : 0; } }
public ObservableCollection<Children> ListOfFiles { get; private set; }
public Category(Children file, string name)
{
ListOfFiles = new ObservableCollection<Children>();
ListOfFiles.Add(file);
// Assign totalSize
TotalSize += file.Size;
Name = name;
}
public void AddChildren(Children f, string name)
{
ListOfFiles.Add(f);
TotalSize += f.Size;
Name = name;
}
private string FindName()
{
return "name";
}
}
}
Children.cs
using System;
using System.IO;
namespace AnimeAssessor
{
public class Children
{
public char[] REMOVABLES = new char[] { '.', '_', '-', ' ', '^', '!', '@', '#', '$', '%', '&', '*', '~', '`', '?', '(', ')', '[', ']', '{', '}', '+' };
public FileInfo File { get; set; }
public string FileName { get; private set; }
public string FullPath { get; private set; }
public int WidthX { get; private set; }
public int HeightY { get; private set; }
public string[] Keywords { get; set; } // TODO
public string Format { get; private set; }
public long Size { get { return File.Length / 1000000; } }
public Children(FileInfo file)
{
File = file;
FileName = file.Name;
FullPath = file.FullName;
Keywords = file.Name.Split(REMOVABLES, StringSplitOptions.RemoveEmptyEntries);
Format = Keywords[Keywords.Length - 1];
}
}
}
MainWindow.xaml.cs
using System.Collections.Generic;
using System.Windows;
using System.Windows.Input;
using Gat.Controls;
using System.IO;
using System;
using System.Linq;
using System.Xml.Linq;
using System.ComponentModel;
using System.Text;
using System.Globalization;
namespace AnimeAssessor
{
/// <summary>
/// Interaction logic for MainWindow.xaml
/// </summary>
public partial class MainWindow : Window
{
// TODO Produce and suggest the top 5 matches?
// TODO Folder selection is not user-friendly
// TODO Pre-compile all patterns beforehand
// TODO Match directory names for all cases and avoid repetitions for files from the same directory
// TODO Filter out common encoder keywords
// TODO Filter by file-format
// Produce an output file or email it?
// Add matrix for Quality/Size ratio
OpenDialogView _openDialog;
List<DirectoryInfo> _dirList;
BackgroundWorker _AnalyzerThread;
// Analyzer Components
public static char[] _removablesNum = new char[] { '.', '_', '-', ' ', '^', '!', '@', '#', '$', '%', '&', '*', '~', '`', '?', '(', ')', '[', ']', '{', '}', '+', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9' };
public static char[] _removables = new char[] { '.', '_', '-', ' ', '^', '!', '@', '#', '$', '%', '&', '*', '~', '`', '?', '(', ')', '[', ']', '{', '}', '+' };
public static string _animeDBPath = "ANN_AnimeDB_20-12-2015.xml";
public string _parentPath, _outputPath;
public List<string> _titles;
public List<Children> _notSortedFiles;
public List<Category> _sortedFiles;
public MainWindow()
{
InitializeComponent();
InitObjects();
}
private void btn_Submit_Click(object sender, RoutedEventArgs e)
{
InitBackgroundWorker();
// Assigns handler methods
_AnalyzerThread.DoWork += AnalyzerThread_DoWork;
_AnalyzerThread.ProgressChanged += AnalyzerThread_ProgressChanged;
_AnalyzerThread.RunWorkerCompleted += AnalyzerThread_RunWorkerCompleted;
// Starts the scan
_AnalyzerThread.RunWorkerAsync();
}
private void btn_Clear_Click(object sender, RoutedEventArgs e)
{
txt_AddDir.Text = string.Empty;
_dirList.Clear();
list_Folders.Items.Refresh();
btn_Submit.IsEnabled = false;
}
private void btn_AddDir_PreviewKeyDown(object sender, KeyEventArgs e)
{
if (e.Key == Key.Return)
{
if (Directory.Exists(txt_AddDir.Text.Trim()))
{
DirectoryInfo dir = new DirectoryInfo(txt_AddDir.Text.Trim());
if (!_dirList.Contains(dir))
{
_dirList.Add(dir);
txt_AddDir.Text = string.Empty;
list_Folders.Items.Refresh();
btn_Submit.IsEnabled = true;
}
}
// TODO Display notification
e.Handled = true;
}
}
private void btn_AddDir_Click(object sender, RoutedEventArgs e)
{
string folderPath;
// If TextBox is not null or doesn't contain a valid path
if (Directory.Exists(txt_AddDir.Text.Trim()))
{
DirectoryInfo dir = new DirectoryInfo(txt_AddDir.Text.Trim());
if (!_dirList.Contains(dir))
{
_dirList.Add(dir);
txt_AddDir.Text = string.Empty;
btn_Submit.IsEnabled = true;
}
}
// TODO Display notification
else
{
folderPath = OpenDialog();
// If no path was selected, don't change the textBox value
if (!string.IsNullOrWhiteSpace(folderPath))
{
DirectoryInfo dir = new DirectoryInfo(folderPath);
if (!_dirList.Contains(dir))
{
_dirList.Add(dir);
// TextBox.Text was never changed so no need to reset it
btn_Submit.IsEnabled = true;
}
}
}
list_Folders.Items.Refresh();
}
private void InitObjects()
{
_dirList = new List<DirectoryInfo>();
list_Folders.ItemsSource = _dirList;
_notSortedFiles = new List<Children>();
_sortedFiles = new List<Category>();
}
#region BackgroundWorker Thread
private void InitBackgroundWorker()
{
_AnalyzerThread = new BackgroundWorker();
_AnalyzerThread.WorkerReportsProgress = true;
}
// Main worker thread
private void AnalyzerThread_DoWork(object sender, DoWorkEventArgs e)
{
RunAnalysis((BackgroundWorker)sender);
}
// This event will be called after each file's analysis
private void AnalyzerThread_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
pb_main.Value = e.ProgressPercentage;
// Update label with current values
if (e.ProgressPercentage >= 99)
{
lbl_scanProgress.Content = "Producing Results...";
}
else
{
lbl_scanProgress.Content = e.UserState + " files scanned";
}
}
// When thread finishes
private void AnalyzerThread_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
// Open the Result Window once the scan is complete
ScanResults resultWindow = new ScanResults(_sortedFiles, _notSortedFiles, _titles);
resultWindow.ShowDialog();
lbl_scanProgress.Content = string.Empty;
}
#endregion
private string OpenDialog()
{
// Don't reinitialize if the object already exists
// Won't be able to detect newly added drives/devices
if (_openDialog == null)
{
_openDialog = new OpenDialogView();
}
OpenDialogViewModel browseWindow = (OpenDialogViewModel)_openDialog.DataContext;
// Just disables file view, file view is limited though
browseWindow.IsDirectoryChooser = true;
browseWindow.Owner = this;
browseWindow.NameText = "Choose thy folder from the left pane";
// Add the selected directory to the list
if (browseWindow.Show() == true)
{
return browseWindow.SelectedFolder.Path;
}
else
{
return string.Empty;
}
}
#region Analyzer
private void RunAnalysis(BackgroundWorker backgroundWorker)
{
_titles = LoadXML(_animeDBPath, "item", "name");
List<FileInfo> allFiles = new List<FileInfo>();
try
{
// Find all directories
allFiles = _dirList.SelectMany(d => d.GetDirectories("*", SearchOption.AllDirectories))
.SelectMany(d => d.EnumerateFiles())
.ToList();
// Add the files in the parent directory as well
allFiles.AddRange(_dirList.SelectMany(d => d.EnumerateFiles()).ToList());
}
catch (UnauthorizedAccessException)
{
// Do nothing for now
}
_sortedFiles = SortFiles(allFiles, backgroundWorker);
}
private List<Category> SortFiles(List<FileInfo> allFiles, object backgroundWorker)
{
List<Category> categories = new List<Category>();
// Anonymous type creates a list of all titles split into substrings
var splitTitles = _titles.Select(t => new { titleName = t, Parts = SplitByRemovables(t) }).ToList();
int fileCount = 0;
foreach (FileInfo file in allFiles)
{
fileCount++;
string[] fileParts = SplitByRemovables(Path.GetFileNameWithoutExtension(file.Name));
string[] directoryParts = SplitByRemovables(file.Directory.Name);
// Anonymous type finds the best match for each file
// Finds the score for each title, orders it by descending based on the score
// and gets assigned the first title in the ordered list from splitTitles
var topTitle = splitTitles.Select(t => new { titleObj = t, Score = ScoreTitles(t.Parts, fileParts, directoryParts) })
.OrderByDescending(x => x.Score)
.First();
Children child = new Children(file);
// A score of 0 would indicate no matches
if(topTitle.Score > 0)
{
// Searches for a Category with the same name as the topTitle
// returns null if none is found
Category category = categories.FirstOrDefault(c => c.Name == topTitle.titleObj.titleName);
// Child assignment as per the returned value from above
if (category == null)
{
// Create a new category and add to the list
category = new Category(child, topTitle.titleObj.titleName);
categories.Add(category);
}
else
{
// Add to the existing category
category.AddChildren(child, topTitle.titleObj.titleName);
}
}
else
{
// Files without a score were not matched with any existing category
_notSortedFiles.Add(child);
}
// Update Progress
// Send percentComplete to the backgroundWorker and the current file number
int progressPercentage = 100 * fileCount / allFiles.Count;
// Only the ReportProgress method can update the UI
((BackgroundWorker)backgroundWorker).ReportProgress(progressPercentage, fileCount);
}
return categories;
}
private string[] SplitByRemovables(string value)
{
return value.Split(_removables, StringSplitOptions.RemoveEmptyEntries);
}
private int ScoreTitles(string[] titleParts, string[] fileParts, string[] directoryParts)
{
int score = 0;
score = fileParts.Intersect(titleParts).Count();
score += directoryParts.Intersect(titleParts).Count();
return score;
}
private List<string> LoadXML(string filePath, string descendant, string element)
{
return XDocument.Load(filePath)
.Root
.Descendants(descendant)
.Where(c => c.Element("type").Value == "TV")
.Select(c => c.Element(element).Value)
.OrderBy(v => v)
.Select(DeAccentTitles)
.ToList();
}
private string DeAccentTitles(string title)
{
char[] chars = title.Normalize(NormalizationForm.FormD)
.Where(c => CharUnicodeInfo.GetUnicodeCategory(c) != UnicodeCategory.NonSpacingMark)
.ToArray();
return new string(chars).Normalize(NormalizationForm.FormC);
}
#endregion
}
}
ScanResults.xaml.cs
using System.Collections.Generic;
using System.Windows;
namespace AnimeAssessor
{
/// <summary>
/// Interaction logic for ScanResults.xaml
/// </summary>
public partial class ScanResults : Window
{
List<Category> _SortedList;
List<Children> _UnsortedList;
List<string> _Categories;
public ScanResults(List<Category> sortedList, List<Children> unsortedList, List<string> categories)
{
InitializeComponent();
_SortedList = sortedList;
_UnsortedList = unsortedList;
_Categories = categories;
tree_sortedFiles.ItemsSource = _SortedList;
list_unsortedList.ItemsSource = _UnsortedList;
list_categoriesList.ItemsSource = _Categories;
}
}
}
MainWindow.xaml
<Window x:Class="AnimeAssessor.MainWindow"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
xmlns:local="clr-namespace:AnimeAssessor"
mc:Ignorable="d"
Title="Kagutsuchi - File Analysis Utility" Height="525" Width="525" ResizeMode="NoResize">
<Grid>
<Grid.RowDefinitions>
<RowDefinition Height="1*"/>
<RowDefinition Height="3*"/>
<RowDefinition Height="*"/>
</Grid.RowDefinitions>
<StackPanel Name="stack_AddDir"
Grid.Row="0"
Orientation="Horizontal"
HorizontalAlignment="Stretch"
Margin="25,65,25,5">
<Grid>
<Grid.ColumnDefinitions>
<ColumnDefinition Width="8*"/>
<ColumnDefinition Width="2*"/>
</Grid.ColumnDefinitions>
<TextBox Name="txt_AddDir"
Grid.Column="0"
HorizontalAlignment="Stretch"
Width="370"
Padding="4"
PreviewKeyDown="btn_AddDir_PreviewKeyDown"
HorizontalContentAlignment="Left"
VerticalContentAlignment="Center"
MaxLines="1"/>
<Button Name="btn_AddDir"
Grid.Column="1"
HorizontalAlignment="Right"
Margin="5,0,0,0"
Padding="10,0,10,1.5"
Content="Add Folder" Click="btn_AddDir_Click" />
</Grid>
</StackPanel>
<ListBox Name="list_Folders"
Grid.Row="1"
HorizontalAlignment="Stretch"
ScrollViewer.CanContentScroll="False"
Margin="25,5,25,5"/>
<StackPanel Name="stack_footer"
Grid.Row="2"
Margin="20,10,20,10"
Orientation="Vertical"
HorizontalAlignment="Stretch">
<DockPanel HorizontalAlignment="Stretch">
<Button Name="btn_Clear"
Content="Clear"
Margin="0,0,5,0"
Padding="30,5,30,5" Click="btn_Clear_Click"/>
<Button Name="btn_Submit"
Content="Scan"
IsEnabled="False"
Margin="5,0,0,0"
Padding="30,5,30,5" Click="btn_Submit_Click"/>
</DockPanel>
<ProgressBar Name="pb_main" Height="15" Margin="0,5,0,5"/>
<Label Name="lbl_scanProgress" Foreground="Red" FontWeight="Light" HorizontalAlignment="Right"/>
</StackPanel>
</Grid>
</Window>
ScanResults.xaml
<Window x:Class="AnimeAssessor.ScanResults"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
xmlns:local="clr-namespace:AnimeAssessor"
mc:Ignorable="d"
Title="Please confirm all predicted categories" Height="600" Width="600" ResizeMode="CanMinimize" WindowState="Maximized">
<Window.Resources>
<DataTemplate x:Key="unsortedListItem">
<!-- Item Grid -->
<Grid>
<StackPanel Orientation="Horizontal"
Margin="5">
<TextBlock Text="{Binding FileName}" />
<TextBlock Text=" | Size: "/>
<TextBlock Text="{Binding Size}" />
<TextBlock Text=" MB" />
<ToolTipService.ToolTip>
<StackPanel>
<TextBlock Text="{Binding FullPath}" />
</StackPanel>
</ToolTipService.ToolTip>
</StackPanel>
</Grid>
</DataTemplate>
<DataTemplate x:Key="titleListItem">
<!-- Item Grid -->
<Grid>
<StackPanel Orientation="Horizontal"
Margin="5">
<TextBlock Text="{Binding}" />
</StackPanel>
</Grid>
</DataTemplate>
</Window.Resources>
<Grid Background="LightGray">
<Grid.RowDefinitions>
<RowDefinition Height="1*"/>
<RowDefinition Height="10*"/>
<RowDefinition Height="1*"/>
</Grid.RowDefinitions>
<Label VerticalAlignment="Bottom"
HorizontalContentAlignment="Center"
FontWeight="Bold"
Background="GhostWhite"
Content="Drag and Drop Files from one List to another to sort them."/>
<Grid Name="grid_AllLists"
Grid.Row="1">
<Grid.ColumnDefinitions>
<ColumnDefinition Width="3*"/>
<ColumnDefinition Width="2*"/>
<ColumnDefinition Width="2*"/>
</Grid.ColumnDefinitions>
<Grid Margin="5"
Grid.Column="0"
HorizontalAlignment="Stretch">
<Grid.RowDefinitions>
<RowDefinition Height="Auto"/>
<RowDefinition Height="*"/>
</Grid.RowDefinitions>
<Label HorizontalContentAlignment="Center"
Grid.Row="0"
FontWeight="Light"
Content="SORTED FILES"/>
<TreeView Name="tree_sortedFiles"
Grid.Row="1"
Background="White"
HorizontalAlignment="Stretch"
VerticalAlignment="Stretch"
ScrollViewer.CanContentScroll="False">
<TreeView.Resources>
<!-- TreeView header template-->
<HierarchicalDataTemplate
DataType="{x:Type local:Category}"
ItemsSource="{Binding ListOfFiles}">
<TextBlock Name="tree_headerText" Foreground="Black" FontWeight="Light" Margin="7">
<TextBlock Text="{Binding Name}" FontWeight="SemiBold" Foreground="DarkBlue" />
<TextBlock Text=" | Avg. Size: " />
<TextBlock Text="{Binding AverageSize}" />
<TextBlock Text=" MB" />
</TextBlock>
</HierarchicalDataTemplate>
<!-- TreeView nodes template-->
<DataTemplate DataType="{x:Type local:Children}">
<StackPanel Orientation="Horizontal">
<TextBlock Name="tree_headerText" Margin="3" Foreground="DarkGreen" FontWeight="Light">
<TextBlock Text="{Binding FileName}" Foreground="Blue" FontWeight="Normal" />
<TextBlock Text=" | Size: " />
<TextBlock Text="{Binding Size}" />
<TextBlock Text=" MB" />
</TextBlock>
<ToolTipService.ToolTip>
<StackPanel>
<TextBlock Text="{Binding FullPath}" />
</StackPanel>
</ToolTipService.ToolTip>
</StackPanel>
</DataTemplate>
</TreeView.Resources>
</TreeView>
</Grid>
<Grid Margin="5"
Grid.Column="1"
HorizontalAlignment="Stretch">
<Grid.RowDefinitions>
<RowDefinition Height="Auto"/>
<RowDefinition Height="*"/>
</Grid.RowDefinitions>
<Label HorizontalContentAlignment="Center"
FontWeight="Light"
Grid.Row="0"
Content="UNSORTED FILES"/>
<ListBox Name="list_unsortedList"
Grid.Row="1"
ScrollViewer.CanContentScroll="False"
HorizontalAlignment="Stretch"
Background="GhostWhite"
Foreground="Red"
FontWeight="Light"
ItemTemplate="{DynamicResource unsortedListItem}"/>
</Grid>
<Grid Margin="5"
Grid.Column="2"
HorizontalAlignment="Stretch">
<Grid.RowDefinitions>
<RowDefinition Height="Auto"/>
<RowDefinition Height="*"/>
</Grid.RowDefinitions>
<Label HorizontalContentAlignment="Center"
FontWeight="Light"
Grid.Row="0"
Content="CATEGORIES"/>
<ListBox Name="list_categoriesList"
Grid.Row="1"
ScrollViewer.CanContentScroll="False"
HorizontalAlignment="Stretch"
Background="GhostWhite"
FontWeight="Light"
ItemTemplate="{DynamicResource titleListItem}"/>
</Grid>
</Grid>
</Grid>
</Window>
Sample File names:
ShingekinoKyojinOVA-01(480p)[Hatsuyuki-Kaitou][D8E8CC75].mkv -- Expected Category "Shingeki no Kyojin" (Hi10)_Gosick_-_22_The_Christmas_Carol_Adorns_the_Happiness_by_the_Window_(BD_720p)_(Broken).mkv -- Expected Category "Gosick" Manyuu.Hiken-chou.04.HD.BD.Kira.ACB.mkv -- Expected Category "Manyu Hiken-cho" Commie_Steins Gate 01 Prologue to the Beginning and End.mkv -- Expected Category "Steins Gate" Commie_Steins_Gate_02_BD_720p_AnimeKens.com.mkv -- Expected Category "Steins Gate"
Extract from the source XML file:
<report>
<args>
<type>anime</type>
<name></name>
<search></search>
</args>
<item>
<id>17938</id>
<gid>721551383</gid>
<type>ONA</type>
<name>Koyomimonogatari</name>
<precision>ONA</precision>
<vintage>2016年01月09日</vintage>
</item>
<item>
<id>17937</id>
<gid>1319318627</gid>
<type>TV</type>
<name>Qualidea Code</name>
<precision>TV</precision>
</item>
</report>
Github Repository:
You can find the project files here
Note:
- I'm using the WPF Open Dialog 1.1.1 NuGet package by Christoph Gattnar for handling the folder-selection operation.
I've added a default manifest file to require Administrator Access when the app runs, this avoids
UnauthorizedAccessException
. The following piece of code was added into the manifest file to make this happen:<requestedExecutionLevel level="requireAdministrator" uiAccess="false" />
Issues:
- How can I check matches with the directory name but only do this once for all the files existing in that directory? There are many ways to do it, but which is the most scalable and efficient?
- Will it be faster to pre-compile the Regex pattern for all the 3000~ title-names during
LoadXML
? Because otherwise the same title is being matched at least twice. Will it be faster than the currently implementedIntersect
method?
-
\$\begingroup\$ You don't by chance have this code hosted somewhere do you? \$\endgroup\$Robert Snyder– Robert Snyder2016年01月20日 04:36:27 +00:00Commented Jan 20, 2016 at 4:36
-
\$\begingroup\$ @RobertSnyder It is on Github \$\endgroup\$Paras– Paras2016年01月20日 05:33:14 +00:00Commented Jan 20, 2016 at 5:33
-
1\$\begingroup\$ great. I started looking at your code last night (after pasting all the classes in) and got it up and running. It might take me sometime to write up a code review, but you are next on my list. Since I'm currently working my suggestion right now is to think about encapsulation, and using a testing framework. Those two will answer your questions directly, but I'll show how in my review. \$\endgroup\$Robert Snyder– Robert Snyder2016年01月20日 13:39:24 +00:00Commented Jan 20, 2016 at 13:39
-
\$\begingroup\$ Interesting, I'll post a link to the project's github as well. \$\endgroup\$Paras– Paras2016年01月20日 17:13:49 +00:00Commented Jan 20, 2016 at 17:13
-
\$\begingroup\$ Why does adding children to a category change the category's name? \$\endgroup\$Jeroen Vannevel– Jeroen Vannevel2016年01月20日 17:34:06 +00:00Commented Jan 20, 2016 at 17:34
1 Answer 1
Good Names
A hard task, but a worthwhile one, is giving names to everything. The hard part is that the names need to show what the purpose of it being there is. When we look at the properties of Category
those are all good names. They explictly describe what each thing is. But the name Category
and Children
are not so nice. Category
has me to believe that this is going to represent different a single Category of a movie. When I compare that thought with the Properties in Category
my perception of it changes. I no longer think of it as a Category
I think of it as FileCollectionStatistics
. I recommend that you rethink the name of those two classes as they don't represent what they really are.
Single Responsibility Principle
This principle states that an object should be responsible for one thing, and one thing only. You can search for it and you'll find a never ending supply of articles, papers, and videos on it. I find the reason for so much information is because of the lack of people who use it. It is common for new programmers to break this principle. Lets consider a good example: Category
(or what i'd rather call FileCollectionStatistics
) It shows some basic statistics of a list of Files. It does a good job of this, and the code is easily testable. Lets look at a not so good example: MainWindow
. Usually I'll find Main_____
anything to break the SRP. Your main window does so many things I can't even keep it all straight, yet there are some very obvious things that could be pulled into another class that ONLY does that one thing. I'll point out the most obvious one the Analyzer
section. You conveniantly told me in your code that MainWindow
was doing too much by your Regions. When I look at the methods inside the Analyzer
section I can see that all of those can be moved out into a new class somewhat easily. Moving code out of one class into another is called encapsulation, and is a way of fixing violations of the SRP. I'll do a very easy one for you here.
private void RunAnalysis(BackgroundWorker backgroundWorker)
{
_titles = LoadXML(_animeDBPath, "item", "name");
//...removed extra code for clarity
}
private List<string> LoadXML(string filePath, string descendant, string element)
{
return XDocument.Load(filePath)
.Root
.Descendants(descendant)
.Where(c => c.Element("type").Value == "TV")
.Select(c => c.Element(element).Value)
.OrderBy(v => v)
.Select(DeAccentTitles)
.ToList();
}
private string DeAccentTitles(string title)
{
char[] chars = title.Normalize(NormalizationForm.FormD)
.Where(c => CharUnicodeInfo.GetUnicodeCategory(c) != UnicodeCategory.NonSpacingMark)
.ToArray();
return new string(chars).Normalize(NormalizationForm.FormC);
}
You get back a list of strings after loading some xml into memory and picking out values that you want. Well Lets pull out this very small bit of code into a class called AnimeTvRepository
since MainWindow
should not care how it gets its titles. So after a few tweaks the new class could look like this.
public class AnimeTvRepository
{
public List<string> GetTvTitles(string filePath)
{
return LoadXML(filePath, "item", "name");
}
private static List<string> LoadXML(string filePath, string descendant, string element)
{
return XDocument.Load(filePath)
.Root
.Descendants(descendant)
.Where(c => c.Element("type").Value == "TV")
.Select(c => c.Element(element).Value)
.OrderBy(v => v)
.Select(DeAccentTitles)
.ToList();
}
private static string DeAccentTitles(string title)
{
char[] chars = title.Normalize(NormalizationForm.FormD)
.Where(c => CharUnicodeInfo.GetUnicodeCategory(c) != UnicodeCategory.NonSpacingMark)
.ToArray();
return new string(chars).Normalize(NormalizationForm.FormC);
}
}
More importantly I am able to also test this in isolation. I don't have to load up the program at all to see if my TvRepository is correct. This leads me to my final point
Unit Testing
Unit testing is a fast and easy way to check if your code is doing what you want it to do. I'm not going to tell you how to unit test because of the abundance of information available on how to unit test. (Plus as I write this I'm also doing it in code and can create another pull request) Since AnimeTvRepository
only needs a file path I can make a very simple xml file with no more than 10 items in it. Once that works as needed I could do a much larger test check the one xml file you have in your project ANN_AnimeDB_20-12-2015.xml. So I create a TestDb.xml file and put 10 items into it from your bigger xml
<?xml version="1.0" encoding="utf-8" ?>
<report>
<args>
<type>anime</type>
<name></name>
<search></search>
</args>
<item>
<id>17938</id>
<gid>721551383</gid>
<type>ONA</type>
<name>Koyomimonogatari</name>
<precision>ONA</precision>
<vintage>2016年01月09日</vintage>
</item>
<item>
<id>17937</id>
<gid>1319318627</gid>
<type>TV</type>
<name>Qualidea Code</name>
<precision>TV</precision>
</item>
<item>
<id>17936</id>
<gid>542632259</gid>
<type>OAV</type>
<name>To Love-Ru -Trouble- Darkness</name>
<precision>OAV 3</precision>
<vintage>2016年07月04日</vintage>
</item>
<item>
<id>17925</id>
<gid>1248384627</gid>
<type>OAV</type>
<name>Fairy in the Forest</name>
<precision>OAV</precision>
<vintage>2001年06月15日</vintage>
</item>
<item>
<id>17924</id>
<gid>798219104</gid>
<type>OAV</type>
<name>My Private Tutor</name>
<precision>OAV</precision>
<vintage>2002年05月17日</vintage>
</item>
<item>
<id>17921</id>
<gid>2494305591</gid>
<type>TV</type>
<name>Ohenro</name>
<precision>TV</precision>
<vintage>2014年05月03日 to 2015年03月28日</vintage>
</item>
<item>
<id>17920</id>
<gid>509269950</gid>
<type>TV</type>
<name>Ketsuekigata-kun!</name>
<precision>TV 4</precision>
<vintage>2016年01月11日</vintage>
</item>
<item>
<id>17911</id>
<gid>2399511937</gid>
<type>TV</type>
<name>Tabi Machi Late Show</name>
<precision>TV</precision>
<vintage>2016年01月08日</vintage>
</item>
<item>
<id>17910</id>
<gid>1711432611</gid>
<type>TV</type>
<name>Kono Danshi, Mahō ga Oshigoto Desu.</name>
<precision>TV</precision>
<vintage>2016年02月05日</vintage>
</item>
</report>
from there I can start writing my test for the AnimeTvRepository
Since the code is already in place this type of test is more of an charactization test (a test that shows how something currently works so that functionality is not lost when refactoring). The series of tests could be bunch more comprehensive but for this I think it would best if you looked into more than me.
[TestClass]
public class AnimeTvRepositoryTests
{
[TestMethod]
public void OnlyTvItemsAreReturned()
{
var target = new AnimeTvRepository();
var titles = target.GetTvTitles("TestDb.xml");
Assert.AreEqual(5, titles.Count);
Assert.AreEqual("Ketsuekigata-kun!", titles[0]);
Assert.AreEqual("Kono Danshi, Maho ga Oshigoto Desu.", titles[1]);
Assert.AreEqual("Ohenro", titles[2]);
Assert.AreEqual("Qualidea Code", titles[3]);
Assert.AreEqual("Tabi Machi Late Show", titles[4]);
}
}
That is what my test looks like. With that in place I can feel confident in modifing the repository code without breaking anything. So here is what my slightly revised AnimeTvRepository
looks like
public class AnimeTvRepository
{
public List<string> GetTvTitles(string filePath)
{
return LoadXML(filePath, "item", "name").OrderBy(x=> x).ToList();
}
private static IEnumerable<string> LoadXML(string filePath, string descendant, string element)
{
return XElement.Load(filePath)
.Elements(descendant)
.Where(c => c.Element("type").Value == "TV")
.Select(c => DeAccentTitles(c.Element(element).Value));
}
private static string DeAccentTitles(string title)
{
char[] chars = title.Normalize(NormalizationForm.FormD)
.Where(c => CharUnicodeInfo.GetUnicodeCategory(c) != UnicodeCategory.NonSpacingMark)
.ToArray();
return new string(chars).Normalize(NormalizationForm.FormC);
}
}
There are plenty of other suggestions, but I will leave those to other reviewers. Good luck!