tags:

views:

94

answers:

2

Boy, I am sure am messing around with a lot of casting and such in this code below. It seems like there should be a smoother way. I'm basically trying to use a builder method (CreateNewPattern) to handle creating new objects of the passed sub-class type (by the CreateNewCircularPattern and CreateNewLinePattern methods). I presently only have two sub-classed types CircularHolePattern and SingleLineHolePattern that inherit from HolePattern, but I expect to have more as my app grows.

Is this a place for using a delegate or a lambda? It know nothing about them, so please be as specific as possible with and code suggestions.

private CircularHolePattern CreateNewCircularPattern()
{
   var CreatedPattern = CreateNewPattern(typeof(CircularHolePattern));
   return (CircularHolePattern)CreatedPattern;
}


private SingleLineHolePattern CreateNewLinePattern()
{
   var CreatedPattern=CreateNewPattern(typeof(SingleLineHolePattern));
   return (SingleLineHolePattern)CreatedPattern;
}

private HolePattern CreateNewPattern(Type PatternTypeToCreate)
{
    var NewHolePattern = (HolePattern)Activator.CreateInstance(PatternTypeToCreate);
    NewHolePattern.PatternName = "Pattern #" + (HolePatterns.Count + 1).ToString();
    this.AddPattern(NewHolePattern);
    this.SetActivePattern(NewHolePattern);
    return NewHolePattern;
}
+6  A: 

I suspect you want generics:

private T CreateNewPattern<T>() where T : HolePattern, new()
{
    var newHolePattern = new T();
    newHolePattern.PatternName = "Pattern #" +
        (HolePatterns.Count + 1).ToString();
    this.AddPattern(newHolePattern);
    this.SetActivePattern(newHolePattern);
    return newHolePattern;          
}

private SingleLineHolePattern CreateNewLinePattern() {
    return CreateNewPattern<SingleLineHolePattern>();
}

private CircularHolePattern CreateNewCircularPattern() {
    return CreateNewPattern<CircularHolePattern>();
}

The T is the generic-type-argument; the type we want to create. The where says "it must be HolePattern or a sub-type, and it must have a public parameterless constructor" - this lets us use new T() to create a new instance of it, and access all members of HolePattern against such instances (such as PatternName). This also allows us to call the methods that accept a HolePattern as an argument.

Marc Gravell
I can certainly follow what generics is doing here. However, let me ask, respectfully, because I am a newbie to C# (coming from FoxPro mostly), since I want to develop good practices and an undertanding that I can own with conviction, so, what in the world have I really gained with this approach?
MattSlay
One thing is type safety; you can't call `CreateNewPattern<int>`, but you can call `CreateNewPattern(typeof(int))`; it knows that T is a HolePattern with a usable constructor. A second is you no longer have any casting - the compiler can prove that you are doing things right.
Marc Gravell
Depending on your point of view, you have also removed all useful functionality from 2 methods: CreateNewLinePattern/CreateNewCircularPattern - you could even remove them completely and have the caller use CreateNewPattern<CircularHolePattern>() etc; less code to maintain.
Marc Gravell
A: 

For one, you could reduce the Create...Pattern methods to

private CircularHolePattern CreateNewCircularPattern()
{
   return CreateNewPattern(typeof(CircularHolePattern));
}

Another suggestion might be to only work in abstractions. For example, only return HolePattern types from the Create...Pattern methods instead of their concrete types such as CircularHolePattern. You are casting them down to HolePattern in any case.

So, CreateNewCircularPattern becomes

private HolePattern CreateNewCircularPattern()
{
   return CreateNewPattern(typeof(CircularHolePattern));
}
sduplooy