Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Acceptable way to set readonly field outside of a constructor

I have a constructor that performs initialization on a switch like this:

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'm still implementing this, but once done it will easily be a few hundred lines. I'm not a fan of having a constructor this large, but I"m at a loss as to how to either safely bypass this language feature (and bypassing at all is something I don't want to do). Maybe should should be a hint that there's 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 custom immutable type. What's the best way to do this? Is a gazillion line count constructor a terrible thing in this case?

Update: Just for clarification, what I am wanting to do is maintain immutability in a class that would have instances initialized in a complex manner in the best manner possible. I am writing a class that represents a randomly generated token, FormatToken, which would usually be a character.

The complex initialization is parsing a format string (note, I am not trying to parse a regexp to generate a random string, I don't feel like spending my next 20 lifetimes doing this :) ). I was initially writing up something that would accept input through a constructor parameter, such as

+        /// 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 spawned this question was that which represents each of those tokens. The initialization question comes from being able to support various formats (ASCII characters, roman alphabet, decimals, symbols, 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 to use it. Simply removing readonly and replacing with an auto-property (i.e. { get; private set; } would take care of my immutability concern.

This question has become more a question about the initialization tasks and less about the immutability of FormatToken. Perhaps 'How to perform complex, possibly unknown initialization' is a better question title now. It's now completely obvious to me that having a giant switch is a bad idea. The factory pattern is certainly intriguing for what I'm doing, and I think answers the question I have. I just want to give it a couple of more days.

Thank you so much for your thoughts so far! I'm leaving the initial example code in here to keep the answers making sense.

like image 363
jdphenix Avatar asked Oct 10 '13 05:10

jdphenix


2 Answers

Perhaps I miss the point, 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;
    }
}
like image 126
Thomas Avatar answered Oct 11 '22 13:10

Thomas


You could use a static factory method on the Foo class in combination with a private constructor. The factory method should be responsible of doing your large switch figuring out the desired values of Bar and Baz, and then simply passing the computed values to the private constructor.

This, of course, does not get rid of the giant switch, but it moves it completely out of the constructor, in which we usually are told that it is bad to do large computations.

This way you end up with 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);
    }
}
like image 31
rasmusgreve Avatar answered Oct 11 '22 13:10

rasmusgreve