6
\$\begingroup\$

This is a sort of follow up to my previous question:

Counting the number of character occurrences

This time I have written code that still counts the number of different characters in a string, but with the added ability to find categories of characters. For example, finding how many numbers are in the string, how many punctuation characters are in the string.

Is there anything I should be doing differently? Any improvements? Also, I'm trying to learn better OOP design and software design and best practice, so any advice on that would also be helpful.

A couple of notes:

  • From what I've read immutable objects are preferred, so I created the "results" class to have private setters and return a read only dictionary to stop something else from changing it.
  • Is the way I've created the CharacterCountResult object from CharacterCount a good thing to do? As in, am I doing it correctly?

static void Main(string[] args)
{
    var i = CharacterCount.Count(@"Hello, World! £$% ^ powejdoiwr3u?!?!!/1;';'\\z\\]p[\][z]z\,.,/???");

    // Demonstrating some of the avaliable properties
    Console.WriteLine("Alphanumeric: {0}\nLowercase: {1}\nUppercase: {2}\nPunctuation: {3}\nDigits: {4}\nSymbols: {5}",
        i.LetterAndDigitCount, i.LowercaseCount, i.UppercaseCount, i.PunctuationCount, i.DigitCount, i.SymbolCount);

    foreach (var character in i.GetCharacterDictionary())
    {
        Console.WriteLine("{0} - {1}", character.Key, character.Value);
    }
}

This is the class that counts the characters in the string:

class CharacterCount
{
    public static CharacterCountResult Count(string stringToCount)
    {
        var tempDictionary = new Dictionary<char, uint>();

        uint controlCount = 0;
        uint highSurrogatecount = 0;
        uint lowSurrogateCount = 0;
        uint whiteSpaceCount = 0;
        uint symbolCount = 0;
        uint punctuationCount = 0;
        uint separatorCount = 0;
        uint letterCount = 0;
        uint digitCount = 0;
        uint numberCount = 0;
        uint letterAndDigitCount = 0;
        uint lowercaseCount = 0;
        uint upperCaseCount = 0;

        // Build dictionary of characters and occurrence of characters.
        foreach (var character in stringToCount)
        {
            if (!tempDictionary.ContainsKey(character))
            {
                tempDictionary.Add(character, 1);
            }
            else
            {
                tempDictionary[character]++;
            }
        }

        // Iterate over string and count various types of characters.
        foreach (var character in stringToCount)
        {
            if (char.IsNumber(character))
            {
                numberCount++;
            }

            if (char.IsPunctuation(character))
            {
                punctuationCount++;
            }

            if (char.IsSeparator(character))
            {
                separatorCount++;
            }

            if (char.IsSymbol(character))
            {
                symbolCount++;
            }

            if (char.IsUpper(character))
            {
                upperCaseCount++;
            }

            if (char.IsWhiteSpace(character))
            {
                whiteSpaceCount++;
            }


        }

        var result = new CharacterCountResult(controlCount, highSurrogatecount, lowSurrogateCount, whiteSpaceCount,
            symbolCount, punctuationCount, separatorCount, letterCount, digitCount, numberCount, letterAndDigitCount,
            lowercaseCount, upperCaseCount, tempDictionary);


        return result;
    }
}

And this class is the "result" of the character counting. It has properties which can be used to find the number of different types of characters as well as a method which returns a ReadOnlyDictionary<char, uint> that can be used to find the number of times each specific character occurs:

class CharacterCountResult
{
    // Unicode special characters.
    public uint ControlCount { get; private set; }
    public uint HighSurrogateCount { get; private set; }
    public uint LowSurrogateCount { get; private set; }

    // Textual special characters.
    public uint WhiteSpaceCount { get; private set; }
    public uint SymbolCount { get; private set; }
    public uint PunctuationCount { get; private set; }
    public uint SeparatorCount { get; private set; }

    //Letters, digits, numbers.
    public uint LetterCount { get; private set; }
    public uint DigitCount { get; private set; }
    public uint NumberCount { get; private set; }
    public uint LetterAndDigitCount { get; private set; }
    public uint LowercaseCount { get; private set; }
    public uint UppercaseCount { get; private set; }

    private Dictionary<char, uint> _characterDictionary = new Dictionary<char, uint>();

    public CharacterCountResult(uint controlCount, uint highSurrogateCount, uint lowSurrogateCount,
        uint whiteSpaceCount, uint symbolCount, uint punctuationCount, uint separatorCount, uint letterCount,
        uint digitCount, uint numberCount, uint letterAndDigitCount, uint lowercaseCount, uint uppercaseCount,
        Dictionary<char, uint> characterDictionary)
    {
        ControlCount = controlCount;
        HighSurrogateCount = highSurrogateCount;
        LowSurrogateCount = lowSurrogateCount;
        WhiteSpaceCount = whiteSpaceCount;
        SymbolCount = symbolCount;
        PunctuationCount = punctuationCount;
        SeparatorCount = separatorCount;
        LetterCount = letterCount;
        DigitCount = digitCount;
        NumberCount = numberCount;
        LetterAndDigitCount = letterAndDigitCount;
        LowercaseCount = lowercaseCount;
        UppercaseCount = uppercaseCount;

        _characterDictionary = characterDictionary;
    }

    public ReadOnlyDictionary<char, uint> GetCharacterDictionary()
    {
        var readOnly = new ReadOnlyDictionary<char, uint>(_characterDictionary);

        return readOnly;
    }
}
\$\endgroup\$
7
\$\begingroup\$

The good:

  • Good naming, both in following conventions and in picking expressive names
  • Your approach is good procedurally- e.g. good use of the dictionary and in-build char methods.
  • Putting the results in their own class and making it immutable through the use of private setters and the ReadOnlyDictionary is smart.

In terms of areas for potential improvement, reviewing is made somewhat difficult because of the nature of the made-up requirements this code is fulfilling. To demonstrate why, here's my train of thought:

It's not very realistic that you always want to get all this information together, so the first thing might be to think how to extract each piece of information individually. The first step in doing this would be to extract out the individual counts into their own methods, like:

public int CountLetters(string stringToCount)
{
    var letterCount = 0;
    foreach(var character in stringToCount)
    {
        if (char.IsLetter(character))
        {
            letterCount++;
        }
    }
    return letterCount;
}

Unfortunately as you can see, this is pretty quickly going to end up with a load of really similar methods, all of which are frustratingly large. The solution to this is two-fold.

First, because the only difference between the methods is which char function we call, the use of Func can cut it down to just one method:

public int CountLetters(string stringToCount, Func<string, bool> predicate)
{
    var letterCount = 0;
    foreach(var character in stringToCount)
    {
        if (predicate(character))
        {
            letterCount++;
        }
    }
    return letterCount;
}

Second, we can use LINQ:

public int CountLetters(string stringToCount, Func<string, bool> predicate)
{
    return stringToCount.Count(predicate);
}

And now we find the method is so simple that it probably doesn't need to be its own method at all. For example, if in the middle of some other method I wanted to count how many characters in a string were letters I could just do:

var letterCount = myString.Count(char.IsLetter);

This goes back to what I was saying about this being difficult code to review, because it essentially exists to fulfill requirement which is already fulfilled so simply by the .NET framework.

However, if we go with the fiction that exposing the counts for all those different char methods for a single string is something that is done commonly throughout your program, then your approach is sensible. Using the LINQ-style counting above, you can remove the second foreach statement and all the ifs inside, replacing them with one-liners. You can also remove all the variable declarations and feed them right into the result constructor, since they are so simple:

return new CharacterCountResult(
    stringToCount.Count(char.IsControl), 
    //etc...

Going forward, my suggestion is that you'll learn more about design by tackling problems that are a little more realistic than this one, and they will probably garner more informative reviews, too.

\$\endgroup\$
1
\$\begingroup\$

I'm trying to learn better OOP design and software design

EDIT

This pretty much supercedes my original post, which remains below has been deleted.

Strive For Structure

"Structure" is any Class or Class composite that helps organize and use the data. The next two lines of code will replace over half the original code. I kid you not.

    public enum CharGroup
    {
        Number,
        Punctuation,
        Separator,
        Digit,
        Symbol,
        Upper,
        WhiteSpace,
        Undefined
    }

 protected SortedDictionary<CharGroup, int> GroupDictionary { get; set; }

Yes, I'm counting the enum as one line for dramatic effect and because it is.

enum points to ponder

  • Notice the Undefined. I always put that in my enums. You'll see how handy it is below.
  • type safety. Compared to "Number", a plain old string for example
  • Intellisense
  • Clearly and completely defines all of the possible values. Well, you should do this. I have not done so here for brevity.
  • Avoids all the inherent (potential) problems with strings
    • Cannot be null or "empty"
    • cannot be misspelled
    • No gotchas with UPPER or lower case.

One Class

Once you see how the GroupDictionary eliminates all that code then I hope you will see that this naturally fits in with the CharacterDictionary. It's a common thing to see, breaking out code into arbitrary classes in order to corral the clutter. Our new structure will do that for us.

using System;
using System.Collections.Generic;
using System.Text;

namespace CharacterCounterImproved
{
    public class CharacterCounter
    {
        protected SortedDictionary<char, int> CharacterDictionary { get; set; }
        protected SortedDictionary<CharGroup, int> GroupDictionary { get; set; }
        protected string RawString { get; set; }

        public CharacterCounter(string targetString)
        {
            CharacterDictionary = new SortedDictionary<char, int>();
            GroupDictionary = new SortedDictionary<CharGroup, int>();
            RawString = targetString ?? string.Empty;

            BuildCharacterDictionary();
            BuildGroupDictionary();
        }

        protected void BuildCharacterDictionary()
        {
            CharacterDictionary.Clear();

            foreach (Char guy in RawString)
            {
                if (!CharacterDictionary.ContainsKey(guy))
                    CharacterDictionary.Add(guy, 1);
                else
                    CharacterDictionary[guy]++;
            }
        }

        protected void BuildGroupDictionary()
        {
            GroupDictionary.Clear();

            foreach (Char guy in CharacterDictionary.Keys)
            {
                if (Char.IsNumber(guy))
                    AddToGroup(CharGroup.Number, CharacterDictionary[guy]);
                else
                    if (Char.IsWhiteSpace(guy))
                        AddToGroup(CharGroup.WhiteSpace, CharacterDictionary[guy]);
                    else
                        if (Char.IsNumber(guy))
                            AddToGroup(CharGroup.Number, CharacterDictionary[guy]);
                        else
                            if (Char.IsUpper(guy))
                                AddToGroup(CharGroup.Upper, CharacterDictionary[guy]);
                            else
                                if (Char.IsSymbol(guy))
                                    AddToGroup(CharGroup.Symbol, CharacterDictionary[guy]);
                                else
                                    if (Char.IsSeparator(guy))
                                        AddToGroup(CharGroup.Separator, CharacterDictionary[guy]);
                                    else
                                        if (Char.IsPunctuation(guy))
                                            AddToGroup(CharGroup.Punctuation, CharacterDictionary[guy]);
                                        else
                                            AddToGroup(CharGroup.Undefined, CharacterDictionary[guy]);
            }
        }

        protected void AddToGroup(CharGroup thisGroup, int count)
        {
            if (!GroupDictionary.ContainsKey(thisGroup))
                GroupDictionary.Add(thisGroup, count);
            else
                GroupDictionary[thisGroup] += count;
        }

        public override string ToString()
        {
            StringBuilder me = new StringBuilder();
            me.AppendLine("THE STRING: " + RawString);

            me.AppendLine();

            foreach (var entry in CharacterDictionary.Keys)
                me.AppendLine(entry + " : " + CharacterDictionary[entry]);

            me.AppendLine();

            foreach (var entry in GroupDictionary.Keys)
                me.AppendLine(entry + " : " + GroupDictionary[entry]);

            return me.ToString();
        }

        public int CharacterCount(char thisChar)
        {
            int count = 0;

            if (CharacterDictionary.ContainsKey(thisChar))
                count = CharacterDictionary[thisChar];

            return count;
        }

        public int CharacterGroupCount(CharGroup thisGroup)
        {
            int count = 0;

            if (GroupDictionary.ContainsKey(thisGroup))
                count = GroupDictionary[thisGroup];

            return count;
        }

        public string CharacterGroupCounts()
        {
           // http://stackoverflow.com/questions/482729/c-sharp-iterating-through-an-enum-indexing-a-system-array
            StringBuilder me = new StringBuilder();
            Array values = Enum.GetValues(typeof(CharGroup));
            string groupName = string.Empty;

            foreach (CharGroup group in values)
            {
                groupName = Enum.GetName(typeof(CharGroup), group);

                if(GroupDictionary.ContainsKey(group))
                    me.AppendFormat("{0} : {1}\n", groupName, GroupDictionary[group]);
            }

            return me.ToString();
        }

    } // class


        public enum CharGroup
        {
            Number,
            Punctuation,
            Separator,
            Digit,
            Symbol,
            Upper,
            WhiteSpace,
            Undefined
        }

}

Points to Ponter

  • Do not expose the Dictionarys. Instead we expose methods that speak in terms of what the class is supposed to give/do for us.
  • override ToString. Again, we do not want to expose our internal structure. We want to expose our functionality.
  • Single Responsibility Principle
    • Makes code re-usable. Without overriding ToString every client has to write it.
    • The CharacterCounter class is responsible for all "counting" - individual characters and their grouping. If other classes were needed to help in the overall "counting functionality" then make a composite.

The Main() Event

class Program
  {
    static void Main(string[] args)
    {
        string HewohWorld = @"Hello, World! £$% ^ powejdoiwr3u?!?!!/1;';'\\z\\]p[\][z]z\,.,/???";
        CharacterCounter HelloWorldCounter = new CharacterCounter(HewohWorld);

        Console.WriteLine("ToString() ...");
        Console.WriteLine(HelloWorldCounter); 
        Console.WriteLine();
        Console.WriteLine("CharacerGroupCounts() ...");
        Console.WriteLine(HelloWorldCounter.CharacterGroupCounts());

    }
}

Points to Ponder:

  • CharacterCounter provides methods for the various output bits. So the client does not need to know its structure.
  • Good encapsulation, the Single Responsibility Principle, etc. make the concerns about immutable objects moot
    • There is no need to give a client reference to your internal structure. If you want to output the internal key, value pairs then make a simple Data Transfer Class. Write a CharacterCounter method to fill it from its internal structure.

EDIT 2

  • remove all of my original post
  • Answer the OP's question posed in a comment

How do you want the CharacterCount to expose its results. Just put them all as public properties on itself?

  • A Data Transfer Object - DTO
    • Everyone says "DTO" but I say "DTC" - class. It is simply good OO design to have classes that are named, describe and define things in terms of the problem domain.

.

// THE root, basic DTO. 
// Notice how we can still give simple functionality to such a simple thing.
// Notice how we accidentally (yeah, right) defined default values for the 
//        entire CharacterCount-using universe. That is code re-use.
public class CharacterCount {
    public Char Character { get; set; }
    public int  Count     { get; set; }

    public CharacterCount() {
        Character = Char.MinValue; // there is no "Char.Empty"
        Count = 0;  // unnecessary, but explicitly initializing ALL properties is a good thing
                    // the poor maintenance programmer now knows that you intend
                    // zero as the default... Oh, I know what that is! Self documentation! cool.
    }

    public override string ToString() {
        return string.Format( "{0} : {1}", Character, Count);
    }
}

Obviously the clients will want all the CharacterCounts (OMG, I just used a class name in a proper sentence. The power of Self Documentation!).

Sticking with the OO way, we make a strongly typed collection. Think Single Responsibility, encapsulation, domain centric (my words) design.

public class CharacterCountCollection : List<CharacterCount> {

     public override string ToString() {
         StringBuilder me = new StringBuilder();

         foreach( var countObj in this)
             me.AppendLine(countObj.ToString());

         return me.ToString();
     }
}

// update CharacterCounter class; Single Responsibility Principle is always in effect.
// Note: immutability is a non-issue here. We're not giving reference to the 
//       internal objects. We make new ones to give out - which is how one 
//       implements immutability.
public class CharacterCounter {
    public CharacterCountCollection GetCountData() {
        CharacterCountCollection me = new CharacterCountCollection();

        CharacterCount countMeIn;
        foreach(var charCountKey in CharacterDictionary.Keys) {
            countMeIn = new CharacterCount();
            countMeIn.Character = charCountKey;
            countMeIn.Count = CharacterDictionary[charCountKey];
            me.Add(countMeIn);
        }
        return me;
    }
}

//  Here's how the main() might use that.

string HelloWorld = "...our original string ";

CharacterCounter countDracula = new CharacterCounter(HelloWorld);

Console.WriteLine("Let's count boys and girls!");
foreach(var charCount in countDracula.GetCountData()) 
    Console.WriteLine("Character: {0}, We counted {1} - ha ha!", charCount.Character, charCount.Count);

Ask not what you can do for your base class, Ask what your base class can do for you

We just did that by inheriting List. There are some cool Find() overloads. And every class inherits object, and we have some great power just waiting to be unleashed.

Fetching a specific CharacterCount

Let's say our CharacterCount clients want to be able to find a specific character count object.

// override the inherited Equals(object x) method
// but also use the Char.Equals(Char x) method
public class CharacterCount() {

    // I declare: CharacterCount objects are equal if their Character Properties are Equal.

    // this innocuous override is unleashing some freaky awesomeness
    // as you'll see soon
    public override bool Equals(object that) {
        // we don't want just any "object"
        if(that == null) return false;
        if(! that is CharacterCount) return false;
        return this.Character.Equals(that.Character);
    }
}

// now we can find things in our strongly typed collection!

public class CharacterCountCollection {
    public CharacterCount GetCountOf (Char thisGuy) {
        CharacterCount me = new CharacterCount();
        me.Character = thisGuy;

        // here is the poster child for "ask not..."
        me = this.Find(x => x.Equals(me)); 

        // if not found, returns "the default value" for the type.
        // CharacterCount's default constructor defines this for us!

        // if not found insert the target Char. The count defaulted to zero
        me.Character = me.Character == Char.MinValue ? thisGuy : me.Character;
        return me;  
    }
}

In the client

public static void main(string[] args) {
    string HelloWorld = "...our original string ";
    CharacterCounter countDracula = new CharacterCounter(HelloWorld);

    // output a single CharacterCount object
    Console.WriteLine(countDracula.GetCountOf('c'));

    // output the CharacterCount object
    Console.WriteLine(countDracula);

    // or, if we want the data so we can fuss with that...
    CharacerCountCollection countChocula = countDracula.GetCountData();

    // output the count data object
    Console.WriteLine(countChocula);
}

Object Oriented Awesomeness sysnopsis

  • Symptoms of our OO design

    • The code in main() is dead simple. So much so it seems like magic.
    • CharacterCounterCollection is dead simple. So much so that some of it seems like magic.
      • ONE line of code finds a given Char. Contemplate on this while re-reading the code. Freaky. Awesome.
    • CharacterCount is dead simple
      • 2 properties and 2 overridden methods
    • Every method in every class is simple, and short.
    • Good OO design manages complexity.
    • Good OO design makes short methods naturally. This is because of our fanatic adherence to the single responsibility principle.
  • FUNDAMENTAL OO polymorphism in action.

    • Console.Writeline implicitly calls ToString() so our override gets called
    • Every class calls Equals on the objects used. Nowhere do we rifle through that class's internals to figure out how things are equal. We just ask if it's Equal()
  • Maximize coherence and minimize coupling

    • Our count results are decoupled and distinct from the counting - separate classes for the DTO and the counter-grunt-work.
    • The client main() cannot manipulate CharacterCounter internal structure
    • Classes operate only on other class's public methods.
    • Define classes for things in your problem domain, even the small stuff
  • Structure is the key

    • Okay, "structure" is really design.
    • Designing data structure bottom up works well
    • CharacterCount data class almost doesn't seem worth the effort. But it's structure and API (public methods) had a profound influence on all the other classes.
  • Single Responsibility

    • It is obvious that a character and its count is an atomic, fundamental thing in the problem domain.
    • Each class knows for itself how it is equal to other objects of its type.
    • Each class exposes its functionality and hides its details.

    • Keeping our OO design coherent

    • When adding the GetCountOF() functionality we made each class responsible for doing its part.
  • Learn, and trust, OO fundamentals. Inheritance and polymorphism are the 2 pillars of the whole idea of "object oriented."

    • For the inexperienced the proliferation of classes here may seem like a WTF. The before and after code result is self evident.
    • By starting with classes for the simple things in our domain, all the SOLID and other OO principles tend to take care of themselves. Once we get some design in place then principles help us analyze, refine, and improve.
    • Start with design, not principles. Specifically, "I've read immutable objects ... with private setters are preferred." That resulted in a steaming pile of failure from the object oriented perspective. QED.

end EDIT 2

EDIT 3

@PeterKiss answer makes us realize that we should add "character category", our clients would like to know that.

Add the category property

public class CharacterCount() {
    ...

    public CharGroup Category { get; set; }

    public CharacterCount() {
        ....
        Category = CharGroup.Undefined;
    }

     public override string ToString() {
        return string.Format( "{0} : {1} : {2} ", Character, Count, Category);
    }
}
  • No changes needed in any other existing code to have "Category" show in output. This is the power of putting code in the proper classes.
  • We're not populating Category yet, of course.

Refactor CharacterCounter class

//CharacterCounter.BuildGroupDictionary() needs to know about `Category` 
//and now we have a 2nd need. Let's make a method for that.

protected CharGroup GetCharGroup(Char guy) {
    CharGroup myGroup = CharGroup.Undefined; 

    if (Char.IsNumber(guy))      myGroup = CharGroup.Number;
    else
    if (Char.IsWhiteSpace(guy))  myGroup = CharGroup.WhiteSpace;
    else
    if (Char.IsNumber(guy))      myGroup = CharGroup.Number;
    else
    if (Char.IsUpper(guy))       myGroup = CharGroup.Upper;
    else
    if (Char.IsSymbol(guy))      myGroup = CharGroup.Symbol;
    else
    if (Char.IsSeparator(guy))   myGroup = CharGroup.Separator;
    else
    if (Char.IsPunctuation(guy)) myGroup = CharGroup.Punctuation;
    else
       myGroup = CharGroup.Undefined;

    return myGroup;
}

// refactor GetCountData()

public CharacterCountCollection GetCountData() {
    CharacterCountCollection me = new CharacterCountCollection();

    CharacterCount countMeIn;
    foreach(var charCountKey in CharacterDictionary.Keys) {
        countMeIn = new CharacterCount();
        countMeIn.Character = charCountKey;
        countMeIn.Count = CharacterDictionary[charCountKey];
        countMeIn.Category = GetCharGroup(charCountKey);   // <=== added this line
        me.Add(countMeIn);
    }
    return me;
}

// refactor BuildGroupDictionary()

protected void BuildGroupDictionary()
{
    GroupDictionary.Clear();

    foreach (Char guy in CharacterDictionary.Keys)
        AddToGroup (GetCharGroup(guy), CharacterDictionary[guy]);
 }
  • All outside code is oblivious to the refactoring - a good sign.
  • If we had written Unit Tests we would know instantly if refactoring broke something.
  • Adding Category - 1 new method containing existing (refactored) code, 3 new lines, 3 modified lines The code is manifesting traits of maintainability. Reasonable design, taking advantage of inheritance and polymorphism, and refactoring.

end EDIT 3

EDIT 4

@PeterKiss answer mentions sorting. So let's see how that works here.

Sorting CharacterCountCollection

CharacterCounter has internal SortedDictionary objects but its clients don't know that and it certainly is NOT DOCUMENTED. Further it would just be good to have default sort behavior for our DTOs.

Implementing IComparable is the secret. We're taking advantage of our structure, inheritance and built-in .net framework goodness.

public class CharacterCount : IComparable<Char> {
    // all existing code as is

    public int CompareTo(Char that) {
        // a Char cannot be null
        if(this.Character > that.Character) return 1;
        if(this.Character < that.Character) return -1;
        return 0;  // they're equal
    }
}

Now CharacterCount knows how to compare itself to other CharacterCount objects. so ...

public static void main(string[] args) {
    CharacterCounter countDracula         = new CharacterCounter(HelloWorldString);
    CharacterCountCollection countResults = countDracula.GetCountData();

    countResults.Sort();
}

CharacterCountCollection IS A List<T> so a client could do all kinds of sorting dynamically using LINQ. Call the above, then, "default sorting."

Future OO Refactoring

I think CharacterCounter and CharacterCountCollection should merge into one.

  • CharacterCounter needs to know intimate details to build the collection. Of course this knowledge must be somewhere, but if a class is "functionality (methods) and its state (properties), together", then I think all the CharacterCounter functionality should be in the collection. So we're going full circle back to one class.
    • The collection could very well contain a character counter object, but the client interacts only with CharacterCountCollection.
    • Currently we have 2 different classes that both emit "character count data" and/or its formatted output. Merging into one would be more coherent from the clients perspective.
    • GetCountData() will still not give references to internal stuff

end EDIT 4

end EDIT

\$\endgroup\$
1
\$\begingroup\$

Learning OOP

If you want to learn OOP checkout the SOLID principles

  • Single responsibility principle
  • Open/closed principle
  • Liskov substitution principle
  • Interface segregation principle
  • Dependency inversion principle

If I take a look at your approach and at the provided answers I don't see these things in the solutions I only see code which are as flexible as cast iron.

What we will need

  • Something which could count things based on rules
  • The rules themselves

Rules

We want to check the characters' categories (and name those categories) so create an abstract rule:

public abstract class CharacterCategory
{
    public virtual string Name { get { return GetType().Name; } }

    public abstract bool Match(char c);
}

Nothing fancy the rule name will be the descendant implementation class name (but in can be overridden) and we providing a way to check for category matching by the Match(char) method.

We don't implement any rule yet.

CharacterCategorizer

We have to have something which can apply our rules to sort the characters in a string somehow so define a CharacterCategorizer which is requiring some rules to work.

Only one method will be in the categorizer: Categorize(string)

public class CharacterCategorizer
{
    private readonly Dictionary<string, CharacterCategory> _categories;

    public CharacterCategorizer(IEnumerable<CharacterCategory> categories)
    {
        if (categories == null) throw new ArgumentNullException("categories");

        _categories = categories.ToDictionary(x => x.Name);
    }

    public CharacterCategorizeResult Categoryze(string source)
    {
        if (source == null) throw new ArgumentNullException("source");

        var counts = _categories.Values.ToDictionary(x => x.Name, x => new CharacterCategoryCountHelper { Count = 0, Chars = new List<char>() });

        var uncategorized = new List<char>();

        foreach (var c in source)
        {
            var characterCategories = _categories.Values.Where(characterCategory => characterCategory.Match(c)).ToList();

            if (characterCategories.Count > 0)
            {
                foreach (var match in characterCategories)
                {
                    counts[match.Name].Count++;
                    counts[match.Name].Chars.Add(c);
                }
            }
            else
            {
                uncategorized.Add(c);
            }
        }

        return new CharacterCategorizeResult(
            counts.Select(count => new CategorizedCharacters(count.Key, count.Value.Count, count.Value.Chars.AsReadOnly())),
            uncategorized.AsReadOnly());
    }
}

What we do here is initializing the ruleset in the constructor and in the Categorize(string) method we are preparing to categorize the string's characters so:

  • We need a temporary count storage (dictionary of CharacterCategoryCountHelper based on the provided rules)

    internal class CharacterCategoryCountHelper
    {
        public int Count { get; set; }
    
        public List<char> Chars { get; set; }
    }
    
  • Initialize a bucket for the uncategorized characters (just a List<char>; nothing fancy)

In the foreach loop we try to apply the rules for the current char with in mind: a character can be in multiple categories! If we found matches we summarize it in the temporary counter by the rules' names.

At the end of the method we summarize our result with another helper class (notice the ReadOnlyList<T> in the return statement):

public class CharacterCategorizeResult
{
    public IEnumerable<CategorizedCharacters> CategorizedCharacters { get; private set; }
    public IEnumerable<char> UncategorizedCharacters { get; private set; }

    public CharacterCategorizeResult(IEnumerable<CategorizedCharacters> categorizedCharacters, IEnumerable<char> uncategorizedCharacters)
    {
        CategorizedCharacters = categorizedCharacters;
        UncategorizedCharacters = uncategorizedCharacters;
    }
}

(You can use HashSet<char> anywhere where you want to keep track the characters if you don't want to have duplicates.)

Implementing rules

We can have simple rules like this:

public class NumberCharacterCategory : CharacterCategory
{
    public override bool Match(char c)
    {
        return char.IsNumber(c);
    }
}

Derive from the CharacterCategory and implement the Match(char). It's easy and understandable.

But we can do a little magic too!

Collect the rules from the char type

Build a static factory to find all "Is" static methods from the char type which can be used for categorize.

public static class DynamicCharacterCategoryFactory
{
    public static IEnumerable<CharacterCategory> GetCategories()
    {
        var source = typeof (Char).GetMethods(BindingFlags.Public | BindingFlags.Static)
            .Where(x => x.Name.StartsWith("Is"));

        foreach (var methodInfo in source)
        {
            var parameters = methodInfo.GetParameters();

            if (parameters.Length == 1 && parameters[0].ParameterType == typeof(char))
            {
                yield return new DynamicCharacterCategory(methodInfo);
            }
        }
    }
}

Simple reflection:

  1. Collect the public static methods
  2. Which have only one parameter with parameter type char and the name starts with "Is"

We will need a powerful rule to use the collected methods:

public class DynamicCharacterCategory : CharacterCategory
{
    private readonly string _name;
    private readonly Func<char, bool> _func;

    public DynamicCharacterCategory(MethodInfo methodInfo)
    {
        _name = methodInfo.Name;
        _func = methodInfo.CreateDelegate(typeof(Func<char, bool>)) as Func<char, bool>;
    }

    public override string Name
    {
        get { return _name; }
    }

    public override bool Match(char c)
    {
        return _func(c);
    }
}

What we do here is naming our rule (it's important, remember!) and creating a delegate (char type as parameter and bool as return parameter) from the provided MethodInfo to have a fast execution.

Pay attention this dynamic stuff can be evil if something is happening with the .NET framework and the full static implementation of the factory can have problems in the future! (But for testing it is perfect and fast.)

Usage

class Program
{
    static void Main()
    {
        var cc = new CharacterCategorizer(DynamicCharacterCategoryFactory.GetCategories());

        var sourceString = @"Hello, World! £$% ^ powejdoiwr3u?!?!!/1;';'\\z\\]p[\][z]z\,.,/???";

        Console.WriteLine(sourceString);

        var res = cc.Categoryze(sourceString);

        foreach (var categorizedCharacterse in res.CategorizedCharacters)
        {
            Console.WriteLine("{0}: {1}", categorizedCharacterse.Category, categorizedCharacterse.MatchCount);
        }

        Console.WriteLine();

        Console.WriteLine("Uncategorized characters:");

        foreach (var uc in res.UncategorizedCharacters)
        {
            Console.WriteLine(uc);
        }

        Console.WriteLine();

        Console.ReadLine();
    }
}

If you learn the SOLID principles you will see that this code can be easily maintained, extended, tested because no hard coded rules (like the NumberCount property) or any other stuff (methods like CountLetters or nested if statements) in the code.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.