Simple question about stack variable

[DBC User]

I have a method, in which I populate a local variable from a singleton
method value like the following

int age = SingletonAge.GetInstance().CurrentAge;

then I use age in two places in the method. Out of cuiosity, which is
a correct way to do this? If it is a singleton variable, then is it
better to use the property directly instead of assigning the value to
local stack variable?

Thanks.

 

[Marc Gravell]

As always, it depends ;-p

If you expect the value to change, and you need to spot the change,
then you have to read it twice.
Beyond that - be careful of premature optimisation; is this code used
in a tight loop? Is there a reason to suspect a performance issue? If
neither, then my suggestion is to worry about the *real* problems
first, then come back and tidy this if you get bored.

If it is a trivial implementation (i.e. simply goes to a field without
any locking etc) then in all likelihood the JIT will inline it, so it
won't make much difference either way. But if the behavior and
performance is the same either way, then go with whichever makes the
code clearer. If either the GetInstance or CurrentAge are very
expensive, then definitely read once. Benchmarking (or knowledge of
the implementation) might tell you this.

Wrapping the singleton instance-fetch/instance-method into a single
static property might help readability (depending on the real
context), so you could just use:
int age = SingletonAge.Current;

Marc

 

[DBC User]

As always, it depends ;-p

If you expect the value to change, and you need to spot the change,
then you have to read it twice.
Beyond that - be careful of premature optimisation; is this code used
in a tight loop? Is there a reason to suspect a performance issue? If
neither, then my suggestion is to worry about the *real* problems
first, then come back and tidy this if you get bored.

If it is a trivial implementation (i.e. simply goes to a field without
any locking etc) then in all likelihood the JIT will inline it, so it
won't make much difference either way. But if the behavior and
performance is the same either way, then go with whichever makes the
code clearer. If either the GetInstance or CurrentAge are very
expensive, then definitely read once. Benchmarking (or knowledge of
the implementation) might tell you this.

Wrapping the singleton instance-fetch/instance-method into a single
static property might help readability (depending on the real
context), so you could just use:
int age = SingletonAge.Current;

Marc

Hi Marc,

Thank you for the insight, I am kind of lost in the last paragraph of
yours. Could you kind enough to expand that one for me?

 

[Marc Gravell]

I guess I mean that if you have a class like below:

public sealed class SingletonAge
{
public int CurrentAge { get { ... } }
private SingletonAge() {}
public static SingletonAge GetCurrent() { ... }
}

then you could add a static property like below:
public static int Current {
get { return GetCurrent().CurrentAge; }
}

and now calling code can just ask for SingletonAge.

As a stylistic note, it occurs also that with a singleton it is rarely
an expensive operation to get the "current", since this (by
definition) is actually the "only", so in this case a property might
be more suitable. There is an expectation that properties are less
expensive than methods, so this might make the "is this a problem"
clearer - and in this case I wouldn't bother with a static wrapper,
but would just expect the caller to use the static properties:

public sealed class SomeClass
{
private SomeClass() { }
private static readonly SomeClass _singleton = new SomeClass();
public static SomeClass Singleton { get { return _singleton; } }

public int Age {
get { ... }
}
}
// caller does something with SomeClass.Singleton.Age

If the GetCurrent() is more complicated, i.e. there are multiple
instances detected perhaps on a thread property, then it simply isn't
a singleton. It would be OK for the static-property "get" to check if
one exists, and if not create it - but if so it would have to be
synchronized. There is a trick for getting around this if you need
[ask if interested] - but in most cases initializing with the type is
fine and this trick is overkill.

Marc

 

[DBC User]

I guess I mean that if you have a class like below:

public sealed class SingletonAge
{
public int CurrentAge { get { ... } }
private SingletonAge() {}
public static SingletonAge GetCurrent() { ... }

}

then you could add a static property like below:
public static int Current {
get { return GetCurrent().CurrentAge; }
}

and now calling code can just ask for SingletonAge.

As a stylistic note, it occurs also that with a singleton it is rarely
an expensive operation to get the "current", since this (by
definition) is actually the "only", so in this case a property might
be more suitable. There is an expectation that properties are less
expensive than methods, so this might make the "is this a problem"
clearer - and in this case I wouldn't bother with a static wrapper,
but would just expect the caller to use the static properties:

public sealed class SomeClass
{
private SomeClass() { }
private static readonly SomeClass _singleton = new SomeClass();
public static SomeClass Singleton { get { return _singleton; } }

public int Age {
get { ... }
}}

// caller does something with SomeClass.Singleton.Age

If the GetCurrent() is more complicated, i.e. there are multiple
instances detected perhaps on a thread property, then it simply isn't
a singleton. It would be OK for the static-property "get" to check if
one exists, and if not create it - but if so it would have to be
synchronized. There is a trick for getting around this if you need
[ask if interested] - but in most cases initializing with the type is
fine and this trick is overkill.

Marc

Thank you for the good info.