Which Constructor is Better?

[Jasmine2501]

This is kind of for my own entertainment, but I was about to say "the second thing is better" to someone, but now that I think about it I'm not sure... Option 1

public class MyClass {
private System.String SettingValue = "";
private System.String SETTING_KEY = "SomeSettingKey";

public MyClass() {
this.Server = System.Configuration.ConfigurationManager.AppSettings[this.SETTING_KEY];
}
}

Option 2

public class MyClass {
private System.String SettingValue =
System.Configuration.ConfigurationManager.AppSettings["SomeSettingKey"];

public MyClass() { }

}

My reasoning is that the setting key is only ever used in one place so creating a constant is unnecessary overhead, and secondly, there is no reason to create an empty string, then discard that string to the garbage heap, then set it to the value you really wanted... just set it to the correct value on instantiation.

[Matt T Heffron]

Jasmine, The use of a named constant is preferred, even if it will be used only once. Will the value from the AppSettings be the same for every instance of MyClass? If so, then Option 2a ;) with the addition of static readonly:

public class MyClass {
private const string SettingKey = "SomeSettingKey";
private static readonly System.String SettingValue =
System.Configuration.ConfigurationManager.AppSettings[SettingKey];
}

If you need the SettingValue to match the AppSettings value when each instance is created, then I'd scope as much as possible within the constructor:

public class MyClass {
private readonly System.String SettingValue; // readonly if it never changes during the "life" of this instance
public MyClass() {
const string SettingKey = "SomeSettingKey";
this.SettingValue = System.Configuration.ConfigurationManager.AppSettings[SettingKey];
}
}

[Jasmine2501]

If I had given you the real name of the class in the code I'm working with it would be obvious, but yeah the setting value and the key can not change for the life of the application. In fact, I think the class might actually be a singleton for our app but I need to discuss that first with the previous developer. Once it's read, it's always the same, so I think the "static readonly" is the right modifier for it. Thanks for the help, that makes a lot of sense :) Also, I know it's excessively trivial, but my objection to the named constant is - wouldn't that take a memory location we don't need to use? The compiler doesn't know that it will only be used once, so it's going to stick that in a memory location, right? So then, what we have is two instances of the string in memory, right? Once in the code and once in the heap? If you do this a thousand times, you waste a K of memory. I know, I know, who cares about a lousy 1K of memory... (programmers not giving a crap about saving 1K of memory when they can is probably why my favorite pinball game no longer works well on my tablet, and I find it annoying. I find the bloat in modern software really annoying, even when it doesn't cause bugs or performance issues)

[PIEBALDconsult]

I'd probably use a const, but have you considered using an enumeration?

Jasmine2501 wrote:

wouldn't that take a memory location we don't need to use?

Possibly, but that kind of thinking can lead to defines -- at one place I worked (in C) the standard was to put such values in defines to "save space". :rolleyes:

[V 0]

It's probably personal. I prefer option 2, because it does not allow for confusion and keys rarely or never change name.

V.
(MQOTD Rules and previous Solutions )

[Orjan Westin]

There are a number of issues here, when speaking about a generic answer rather than one strictly limited to the examples given, with one potentially important one so far not mentioned: the assignment will happen at different times. In your first example, SettingValue is assigned after all base constructors have been executed. In your second example, which uses field initialisation, SettingValue is assigned before any base constructors have been executed. This may affect what exceptions are thrown on construction errors. For instance, a ConfigurationErrorsException will be thrown if the value is not found. In the first example, you can catch this in the constructor and set a default, or give a more detailed exception. In the second example, you can't, and will have to rely on it being caught outside the class. And if a base constructor would also throw an exception, in your first example this is what will be thrown, while in the second example it's the ConfigurationErrorsException that will be thrown.

[Richard Deeming]

Orjan Westin wrote:

a ConfigurationErrorsException will be thrown if the value is not found

Not in the AppSettings section; if the specified key doesn't exist, it will return null. However, you can get a ConfigurationErrorsException if the configuration file is corrupt.

---

"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer

[Orjan Westin]

Oh, I wasn't aware of that. Thanks.

[Jasmine2501]

But that doesn't really save space, right? Using #define puts the literal value into your final code, right? So, if it's used in multiple places, you're actually wasting memory (code size) with #define, but only if the value is used more than once.

[Jasmine2501]

Thanks! This is what I'm looking for. I didn't think about the order of things, that might be important. Also thanks for giving me the names of the processes, I had never heard "field initialization" before.

[jschell]

Option 1 because if it fails then the cause of the failure is less likely to be confusing. And yes the code can fail.

[Jasmine2501]

Any code can fail, but if this fails, it's a fatal error and if the application blows up, that's fine.

[Matt T Heffron]

The const for the string is really the better way. Remember that in c#, strings are invariant. So the compiler can and will automatically use the same actual string no matter how often it is referenced. Referencing a string const multiple times, or using the identical string literal multiple times is the same. Only one string will be stored in the program and all of the references to it will be to the exact same object. So why did I say the const for string is better? For maintainability. The name of the const can (should) be based on the functional use of the value, and can be updated in a single location, guaranteed to affect all uses. With string literals it is easy to miss one :)

[Jasmine2501]

Yeah I agree, I just thought since the string was being created as an object, it's going to get stored in memory somewhere, in addition to the place where it's stored in the code.

[Matt T Heffron]

But no matter how you code it, const or literal, it must be an object at run-time! The compiler arranges for it to always be the same object. This is from the c# spec document: For instance, the output produced by

class Test
{
static void Main() {
object a = "hello";
object b = "hello";
System.Console.WriteLine(a == b);
}
}

is True because the two literals refer to the same string instance.

[Jasmine2501]

Well no, if you stick it in there as a literal, it's only stored once, in the code. Unless you're saying... Console.Writeln("Hello World"); ...creates a string object in memory? Your IF is true above because you used the equivalence operator, which compares the values, not the pointers.

[Matt T Heffron]

Yes. Exactly. The receiving method must get a string object. That is all it knows how to deal with! See: http://msdn.microsoft.com/en-us/library/system.string.intern.aspx[^]

[Matt T Heffron]

Regarding == of the strings, the same result (true) is displayed if the comparison is changed to: object.ReferenceEquals(a,b) They really are the same object.