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

Categorize episode-file names - Follow up

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

Problem

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 ListOfFiles { get; private set; }

        public Category(Children file, string name)
        {
            ListOfFiles = new ObservableCollection();
            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; }
publ

Solution

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 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 GetTvTitles(string filePath)
    {
        return LoadXML(filePath, "item", "name");
    }

    private static List 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

```


anime




17938
721551383
ONA
Koyomimonogatari
ONA
2016-01-09


17937
1319318627
TV

Code Snippets

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);
}
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);
    }
}
<?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>
[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]);
    }
}
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);
    }
}

Context

StackExchange Code Review Q#117304, answer score: 2

Revisions (0)

No revisions yet.