views:

170

answers:

5

I recently found out about auto-properties and like them quite a lot. At this moment I am trying to use them everywhere where I can. Not really to just be able to use them everywhere, but more to see how well they work in most situations.

Now I am making a singleton and thought:"Hey, let's try auto-properties here as well".

public class MySingleton
{
    public static MySingleton MySingleton { get; private set; }

    private MySingleton() {}

    static MySingleton() { MySingleton = new MySingleton(); }
}

So my question is: "Is it a good idea to implement a singleton like this?"

I am not asking whether a singleton in general is a good idea.

+1  A: 

I don't see any reason why this wouldn't be correct. After all, auto-properties are just syntactic sugar for accessors for a private (compiler-generated) backing field.

Philippe Leybaert
A: 

Sure, I don't see any problem with that.

Dave Markle
+3  A: 

See this on how to implement singleton correctly.

And you're aware that singletons are evil, right?

Anton Gogolev
+1 best way to implement singletons: use a DI container, and set the type to singleton scope.
Michael Meadows
"I am not asking whether a singleton in general is a good idea."
Jon Skeet
Question also got answered correctly, your blog got props.
Michael Meadows
Hmm, it seems that the article predates auto-properties. I don't see why the arguments presented there would argue against the use of an auto-property. Interesting though that you could also just define a public static readonly MySingleton instance = new MySingleton();
Matthijs Wessels
+12  A: 

I wouldn't personally do that. I don't like using automatically implemented properties with a private setter that you never call where really you want a read-only property backed by a read-only variable. It's only one more line of code to be more explicit about what you mean:

public class MySingleton
{
    private static readonly MySingleton mySingleton;
    public static MySingleton MySingleton { get { return mySingleton; } }

    private MySingleton() {}

    static MySingleton() { mySingleton = new MySingleton(); }
}

This way no-one's even tempted to change the singleton class to reassign either the property or the variable, because the compiler will stop them. They'd have to add a setter and/or make the variable non-readonly, which is a bigger change - one which hopefully they'd reconsider.

In other words:

  • Yes, it will work.
  • No, I don't think it's a good idea.
Jon Skeet
I strongly disagree. Auto-properties were invented to make code more succinct, which makes code more readable. The argument that it's a little harder for others to mess up your design is not very convincing IMHO.
Philippe Leybaert
@Philippe: Auto-properties were invented to make code more succinct *where the replacement code is equivalent to the original*. There's no such equivalence here: you're replacing a read-only property with a read-write one, even though it's got a private setter. This may seem trivial, but I rather like my code to express my intentions - and my intention here would be that the value can only be set once. The automatically implemented property expresses a different intention.
Jon Skeet
You have a point about the "intention" of the setter, but in the end, to the consumer of the class, there is no difference whatsoever. Having an auto-property with a private setter is excactly the same as having a read-only property with a private backing field. Sure, it compiles to different code, but it's essentially the same from the point of view of the consumer of the class. Now I have to slap myself, because I've never liked auto-properties and now I'm defending its use (but that's a topic for another question I guess)
Philippe Leybaert
@Philippe: So as long as the API is all right to the *consumer*, it doesn't matter what the implementation is like? What about the maintainer? I like my code to document intention to the next person who's going to look at the source code, not just consumers.
Jon Skeet
@Jon: In the case of a well-known pattern, the "intent" is usually very clear, so I don't think it's an issue here. My point really is: "Less code" is usually "Better code", as long as it results in better readability. But I don't want to start an endless argument. Your points are valid, but in this specific case, I think the OP's code is more readable.
Philippe Leybaert
+6  A: 

I would approach this from a slightly different direction than Jon. Regardless of whether the object is a singleton, is it logically best modeled as a property in the first place?

Properties are supposed to represent... properties. (Captain Obvious strikes again!) You know. Color. Length. Height. Name. Parent. All stuff that you would logically consider to be a property of a thing.

I cannot conceive of a property of an object which is logically a singleton. Maybe you've come up with a scenario that I haven't thought of; I haven't had any Diet Dr. Pepper yet this morning. But I am suspicious that this is an abuse of the model semantics.

Can you describe what the singleton is, and why you believe it to be a property of something?

All that said, I myself use this pattern frequently; usually like this:

class ImmutableStack<T>
{
    private static readonly ImmutableStack<T> emptystack = whatever;
    public static ImmutableStack<T> Empty { get { return emptystack; } }
    ...

Is "Empty" logically a property of an immutable stack? No. Here the compelling benefit of being able to say

var stack = ImmutableStack<int>.Empty;

trumps my desire for properties to logically be properties. Saying

var stack = ImmutableStack<int>.GetEmpty();

just seems weird.

Whether it is better in this case to have a readonly field and a regular property, or a static ctor and an autoprop, seems like less of an interesting question than whether to make it a property in the first place. In a "purist" mood I would likely side with Jon and make it a readonly field. But I also frequently use the pattern of making autoprops with private setters for logically immutable objects, just out of laziness.

How's that for taking all sides of a question?

Eric Lippert
Hmm, interesting. I never really considered properties in that manner. For me they have always just been a replacement for accessor methods. In a way I viewed accessor methods as a way to improve upon public fields (partially hide, put in interface, add some checking info). And then properties to be able to get a more intuitive way than getters and setters. So for me a property never has to logically be a property of the things the class represents. For me a property is just how a field looks from the outside.
Matthijs Wessels
Could you elaborate on the benefit of property-like syntax in your `ImmutableStack<int>.Empty` example? I'd like to better understand what benefit you achieve here, particularly since in LINQ `Enumerable.Empty<T>()` is similar to the type of syntax you avoid in that example.
LBushkin
@LBushkin -- I suppose "Empty" vs "Empty()" vs "GetEmpty()" are all pretty small differences. I think "compelling benefit" was an exaggeration! :-)
Eric Lippert