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 enum
s. 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 CharacterCount
s (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
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