views:

483

answers:

13

I have a class that defines a CallRate type. I need to add the ability to create multiple instances of my class by reading the data from a file.

I added a static method to my class CallRate that returns a List<CallRate>. Is it ok for a class to generate new instances of itself by calling one of its own constructors? It works, I just wonder if it's the proper thing to do.

Thank you, Paul

List<CallRates> cr = 
            CallRates.ProcessCallsFile(file);

edit: fixed typos.

+20  A: 

It is perfectly fine to get object(s) of its own from the static method.

e.g.

One of the dot net libraries does the same thing as you did,

XmlReadrer reader = XmlReader.Create(filepathString);
codemeit
Probably where I got the idea subconsciously...;) Thanks.
This is called the Factory pattern if I'm not mistaken and, as stated, is a perfectly viable method of accomplishing that task. I generally prefer to keep the factory separate from the class but that's largely up to personal preference.
Matthew Brubaker
There are two kinds of factory (in GoF book): Abstract Factory and Factory Method. The presented method is an example of Factory Method.
Leonardo Constantino
Yeah, the factory pattern indeed.
codemeit
+5  A: 

Sure that's fine, even encouraged in some instances. There are several design patterns that deal with object creation, and a few of them do just what you're describing.

Bill the Lizard
+2  A: 

Seems fine to me. In other languages you would probably write a function, but in a language like C#, static methods take up that role.

Bernard
+1  A: 

It is ok. What you just created is something like a simple factory method. You have a static method that creates a valid instance of a type. Actually your method doesn't even have to be static and you still have a valid code. There is a design pattern (Prototype) that creates a new valid object from an existing object. See details at http://www.dofactory.com/Patterns/PatternPrototype.aspx.

David Pokluda
I needed a good reason to start learning class design patterns. Thank you.
+1  A: 

Sure, for simple parsing (or similar) scenarios - I actually prefer the factory method be part of the class. Yes - it does break SRP, but it fulfills KISS - so I call it a net win. For larger apps, or more complicated parsing routines - it makes more sense to have it be an external factory class.

For your particular case, I'd probably prefer a method that took in an IEnumerable<string> instead of a filename - that'd still give you the parsing logic, but allow easy unit tests and "reuse". The caller can wrap the file into an IEnumerable easily enough.

Mark Brackett
+1  A: 

Factory methods are often a good design. When I write them in C#, I call them 'New', so that:

new MyClass()

becomes

MyClass.New()

Trivially it's implemented like this:

class MyClass
{
    public static MyClass New()
    {
        return new MyClass();
    }
}

Mostly I do this when there are additional conditions about whether to actually create the class or just return null, or whether to return MyClass or something derived from it.

Jay Bazuzi
Must be confusing to people who are used to VB.Net. In VB.Net, the constructor is called "New".
Kibbee
Maybe; or maybe it makes good sense - think of it as a static ctor vs. an instance ctor or something. I can't really say, since I haven't spent much time around VB.NET programmers.
Jay Bazuzi
A: 

Thanks all!

+1  A: 

I sometimes use public static methods as an alternative to constructor overloading.

Especially in situations where it is not nice to rely on parameter types alone to indicate what kind of object construction is intended.

Bjarke Ebert
+2  A: 

I'm a fan of having static methods return instances, as suggested plenty of times, above.

@Paul: don't forget to tick the comment above, which you find is the best answer.

Pure.Krome
Sorry! Everyone gave me some help so I gave all a tick up. I don't have the reputation to remove ticks by ticking down and just choosing one. boneHead = me;
I got it now... the tick the check mark... duh!
You remove "up-ticks" by pressing them a second time
Bjarke Ebert
+1  A: 

Just like to point out "generate new instances of itself by calling one of its own constructors"

It is not from the constructor, it is from the static method.

Yes but my static method is calling one of it's constructors... I've actually broken this out to be it's own class now. Thank you.
+1  A: 

I generally use this when I need instant implementations of a class. For example

    public class Car
    {
        public static Car RedExpensiveCar = new Car("Red", 250000);

        public Car()
        {

        }

        public Car(string color, int price)
        {
            Color = color;
            Price = price;
        }

        public string Color { get; set; }
        public int Price { get; set; }
    }

And with this, I don't need to remember or write constructor parameters in my code.

Car car = Car.RedExpensiveCar;
Note that in your example, Car car = Car.RedExpensiveCar is *not* creating a new Car. It will always return the same instance. Even more problematic, since you're using a public field, it could be assigned to: Car.RedExpensiveCar = new Car("Primer Brown", 75);
munificent
Generally better is public static Car RedExpensiveCar { get { return new Car("Red", 250000); } }
munificent
+3  A: 

I often use this pattern when I need to check the validity of parameters. It is strongly discouraged to throw an exception from a constructor. It's not so bad from a factory method, or you can choose to return null.

Brent Rockwood
I was just working on the possiblty of my class throwing errors back to itself... Thanks for the null suggestion.paul
A: 

It's perfectly acceptable to do this. When I do, I typically make the real constructors for the class private so that it's clear that the only way to construct instances is through the static method.

This is very useful in cases where "construction" may not always return a new instance. For example, you may want to return a previously cached object instead.

munificent