Valid way to set readonly field outside of constructor

I have a constructor that performs initialization on the switch as follows:

class Foo { public readonly int Bar; public readonly object Baz; public Foo(int bar, string baz) { this.Bar = bar; switch (bar) { case 1: // Boatload of initialization code this.Bar = /* value based upon initialization code */ this.Baz = /* different value based upon initialization code */ case 2: // Different boatload of initialization code this.Bar = /* value based upon initialization code */ this.Baz = /* different value based upon initialization code */ case 3: // Yet another... this.Bar = /* value based upon initialization code */ this.Baz = /* different value based upon initialization code */ default: // handle unexpected value } } } 

I am still doing this, but as soon as this is done, it will easily be a few hundred lines. I am not a fan of creating such a constructor, but I don’t understand how to safely circumvent this language function (and bypassing in general is what I do not want to do). Maybe there should be a hint that there is something fundamentally wrong with what I'm trying to do, but I'm not sure.

Basically, I want to perform complex initialization in my own immutable type. What is the best way to do this? Is the gazillion string constructor terrible in this case?

Update: Just for the sake of clarification, I want to maintain immutability in a class that would initialize instances in a complex way in the best way. I am writing a class representing a randomly generated token, FormatToken , which is usually a character.

Complex initialization parses the format string (note: I'm not trying to parse a regular expression to generate a random string, I don't want to spend my next 20 lives doing this :)). Initially, I wrote something that would accept input through a constructor parameter, e.g.

 + /// Format tokens + /// c{l} Lowercase Roman character in the ASCII range. + /// v{L} Uppercase Roman character in the ASCII range. + /// c Roman character in the ASCII range. + /// d Decimal. + /// d{0-9} Decimal with optional range, both minimum and maximum inclusive. var rand = new RandomString("c{l}C{L}ddd{0-4}d{5-9}"); rand.Value == /* could equal "fz8318" or "dP8945", but not "f92781". 

The class that ultimately raised this question was what each of these tokens represents. The initialization issue is related to the possibility of supporting various formats (ASCII characters, Roman alphabet, decimals, characters, etc.)

This is the actual code in question:

 internal class FormatToken { public TokenType Specifier { get; private set; } public object Parameter { get; private set; } public FormatToken(TokenType _specifier, string _parameter) { // discussion of this constructor at // http://stackoverflow.com/questions/19288131/acceptable-way-to-set-readonly-field-outside-of-a-constructor/ Specifier = _specifier; _init(_specifier, _parameter); } private void _init(TokenType _specifier, string _parameter) { switch (_specifier) { case TokenType.Decimal: _initDecimalToken(_parameter); break; case TokenType.Literal: Parameter = _parameter; break; case TokenType.Roman: case TokenType.LowerRoman: case TokenType.UpperRoman: _initRomanToken(_specifier, _parameter); break; default: throw new ArgumentOutOfRangeException("Unexpected value of TokenType."); } } 

I used readonly initially because I misunderstood the reason for using it. Just removing readonly and replacing it with an auto-property (i.e. { get; private set; } will take care of my immutability.

This question has become more about initialization tasks and less about FormatToken . Perhaps “How to do complex, possibly unknown initialization” is now the best question. Now it’s quite obvious to me that having a giant switch is a bad idea. The factory sample is certainly intriguing for what I am doing, and I think it answers the question I have. I just want to give him a couple more days.

Thank you so much for your thoughts! I leave the original code example here to save the answers.

+7
constructor c # switch-statement readonly
source share
9 answers

You can use the static factory method in the Foo class in combination with a private constructor. The factory method should be responsible for ensuring that your large switch determines the required Bar and Baz values, and then simply passes the calculated values ​​to the private constructor.

This, of course, does not eliminate the giant switch, but completely transfers it from the constructor, in which we are usually told that doing big calculations is not good.

So you get something like

 class Foo { public readonly int Bar; public readonly object Baz; private Foo(int bar, string baz) { this.Bar = bar; this.Bas = baz; } public static Foo CreateFoo(int bar, string baz) { int tbar; string tbaz; switch (bar) { case 1: // Boatload of initialization code tbar = /* value based upon initialization code */ tbaz = /* different value based upon initialization code */ case 2: // Different boatload of initialization code tbar = /* value based upon initialization code */ tbaz = /* different value based upon initialization code */ //... default: // handle unexpected value } return new Foo(tbar, tbaz); } } 
+7
source share

You can use auto properties :

public int Bar { get; private set; } public int Bar { get; private set; } . You are already capitalizing on Bar , as if this property. Other classes can get Bar , but only your class can install Bar because of its private set; installer private set; .

However, you can set the Bar value several times for each object.

You can set automatic properties in methods (but you cannot use readonly ) if you create a path to the Micha constructor ( https://stackoverflow.com/a/340947/ ).

+5
source share

If there is something fundamentally wrong, it is difficult to say without additional information, but I do not look completely wrong (with the facts shown). I would do every case when I used the method myself or, possibly, with my own objects (depending on the contents of the form). Of course, you cannot use readonly for this, but Properties with public int Bar { get; private set; } public int Bar { get; private set; } public int Bar { get; private set; } and public object Baz { get; private set; } public object Baz { get; private set; } public object Baz { get; private set; } .

 public Foo(int bar, string baz) { this.Bar = bar; switch (bar) { case 1: methodFoo(); case 2: methodBar(); case 3: methodFooBar(); default: ExceptionHandling(); } 
+2
source share

I'd rather go with the Nahum answer, since one of the principles of Open / closed SOLID will not be implemented with Switch statements if you want to extend the behavior, which is one part. Another part of the answer is how to solve this problem. This can be done by going with the inheritance method and using the Factory method ( http://en.wikipedia.org/wiki/Factory_method_pattern ) to create the appropriate instance and perform lazy initialization ( http://en.wikipedia.org/wiki/ Lazy_initialization ) members.

  class FooFactory { static Foo CreateFoo(int bar,string baz) { if(baz == "a") return new Foo1(bar,baz); else if(baz == "b") return new Foo2(bar,baz); ........ } } abstract class Foo { public int bar{get;protected set;} public string baz{get;protected set;} //this method will be overriden by all the derived class to do //the initialization abstract void Initialize(); } 

Let Foo1 and Foo2 be derived from Foo and override the Initialize method to provide an appropriate implementation. Since we need to initialize for other Foo working methods first, we can have the bool variable set to true in the Initalize method, and in other methods we can check if this is set to true, otherwise we can throw an exception indicating the object needs to be initialized. by calling the Initialize method.

Now the client code will look something like this.

  Foo obj = FooFactory.CreateFoo(1,"a"); obj.Initialize(); //now we can do any operation with Foo object. 

The problem that arises if we use the static method inside the class is that these methods cannot use access instance members if necessary. Thus, here instead of static methods inside one class, we can allocate it as a Factory method for creating an instance (but yes, although Singleton works this way, I emphasize this behavior more for the current behavior mentioned here because it gets access to other relevant static methods to do their job).

+2
source share

I may miss this one, but what do you think of:

 class Foo { public readonly int Bar; public readonly object Baz; public Foo(int bar, string baz) { this.Bar = GetInitBar(bar); } private int GetInitBar(int bar) { int result; switch (bar) { case 1: // Boatload of initialization code result = /* value based upon initialization code */ result = /* different value based upon initialization code */ case 2: // Different boatload of initialization code result = /* value based upon initialization code */ result = /* different value based upon initialization code */ case 3: // Yet another... result = /* value based upon initialization code */ result = /* different value based upon initialization code */ default: // handle unexpected value } return result; } } 
+1
source share

I think Thomas's approach is the simplest and supports the constructor API that jdphenix already has.

An alternative approach is to use Lazy to actually delay the installation until the values ​​are used. I like to use Lazy when constructors are not extremely trivial because 1) the setup logic for variables that are never used is never executed and 2) it ensures that object creation is never unexpectedly slow.

In this case, I don’t think the installation logic is complicated or slow, advantage 1 is really noticeable, as the class is becoming more and more complex.

 class Foo { public readonly Lazy<int> Bar; public readonly Lazy<object> Baz; public Foo(int bar, string baz) { this.Bar = new Lazy<int>(() => this.InitBar(bar)); this.Baz = new Lazy<object>(() => this.InitBaz(bar)); } private int InitBar(int bar) { switch (bar) { case 1: // Bar for case 1 case 2: // Bar for case 2 case 3: // etc.. default: } } private object InitBaz(int bar) { switch (bar) { case 1: // Baz for case 1 case 2: // Baz for case 2 case 3: // etc.. default: } } } 
+1
source share

Following Rasmusswarm and John Skeet:

 class Foo { public readonly int Bar; public readonly object Baz; private Foo(int bar, string baz) { this.Bar = bar; this.Baz = baz; } private static Foo _initDecimalToken(string _parameter) { int calculatedint = 0; string calculatedstring = _parameter; //do calculations return new Foo(calculatedint, calculatedstring); } private static Foo _initRomanToken(int bar, string _parameter) { int calculatedint = 0; string calculatedstring = _parameter; //do calculations return new Foo(calculatedint, calculatedstring); } public static Foo CreateFoo(int bar, string baz) { switch (bar) { case 1: return _initDecimalToken(baz); case 2: return _initRomanToken(bar, baz); default: // handle unexpected value... return null; } } } 

If you want to keep Foo lightweight, you can put the static build functions in a separate class (e.g. FooMaker.)

0
source share

You might want to use the readonly field, which preserves the changed structure. What for? Let it be reduced to the necessary:

  • You want to mutate and mix around values ​​during construction. In particular, you want to use regular encapsulation and code reuse methods, such as simple old method calls when building your values.
  • After building, you want the value to be fixed.

Structures are just a bag of values; therefore, they easily allow mutation and encapsulation of this mutation during construction. However, since they are simply value, they use any warehouse semantics that their container provides. In particular, once you store struct (value) in your readonly field, the value cannot be changed (outside the constructor). Even native struct methods cannot mutate unread fields if the structure itself is stored in a readonly field.

For example (can be used in LINQpad):

 void Main() { MyImmutable o = new MyImmutable(new MyMutable { Message = "hello!", A = 2}); Console.WriteLine(o.Value.A);//prints 3 o.Value.IncrementA(); //compiles & runs, but mutates a copy Console.WriteLine(o.Value.A);//prints 3 (prints 4 when Value isn't readonly) //o.Value.B = 42; //this would cause a compiler error. //Consume(ref o.Value.B); //this also causes a compiler error. } struct MyMutable { public string Message; public int A, B, C, D; //avoid mutating members such as the following: public void IncrementA() { A++; } //safe, valid, but really confusing... } class MyImmutable{ public readonly MyMutable Value; public MyImmutable(MyMutable val) { this.Value=val; Value.IncrementA(); } } void Consume(ref int variable){} 

The advantage of this method is that you can have many fields and a well-laid out logic of mutations, but nevertheless it is easy to correct the value after it is completed. It also makes copies and copies with minor variations very easy:

 var v2 = o.Value; v2.D = 42; var d = new MyImmutable(v2); 

The disadvantage is that C #'s volatile structures are unusual, and sometimes surprising. If your initialization logic becomes complex, you will work with parameters and return values ​​with copy semantics and be different enough to accidentally introduce errors. In particular, behavior of type IncrementA() (which changes the behavior depending on whether the structure is in a mutable or immutable context) may be subtle and unexpected. To stay healthy, keep simple structures: avoid methods and properties and never mutate the contents of a structure in a member.

0
source share

All Articles