Why does the source code for the Guid constructor contain the line "this = Guid.Empty"?

Question

If you look a the source code for the constructor of Guid(string) in the .NET 4.5.2 source code it is as follows:

public Guid(String g)
{
    if (g==null) {
        throw new ArgumentNullException("g");
    }
    Contract.EndContractBlock();
    this = Guid.Empty;

    GuidResult result = new GuidResult();
    result.Init(GuidParseThrowStyle.All);
    if (TryParseGuid(g, GuidStyles.Any, ref result)) {
        this = result.parsedGuid;
    }
    else {
        throw result.GetGuidParseException();
    }
}

The question is what is the purpose of the line this = Guid.Empty;?

From what I can see if string g can successfully be parsed in the TryParseGuid method then this will be assigned. If it can't then an exception will be thrown.

Suppose you wrote:

var guid = new Guid("invalidguid");

This would cause an exception and the value of guid would be undefined I would assume. So why the need to assign this to Guid.Empty?

Default empty value. simple as that. It probably is a bit of an oversight because it is meaningless as it is doing a TryParse and assigning it if successful otherwise throws an exception. But again probably, when you look at it, even if the exception is thrown, the default value will be Guid.Empty on the line #6. Now the consumer can still catch an exception and continue to use this particular "instance" of the Guid class...but the value would be empty by default. — Ahmed ilyas, Apr 08 '15 at 05:05
@HighCore - I have not looked at the code ATM however it IS possible to assign it to this if of course, it is of the correct type. — Ahmed ilyas, Apr 08 '15 at 05:11
@HighCore Guid is a struct, it is perfectly leagal to assign this inside of one. http://ideone.com/yBmCGm — Scott Chamberlain, Apr 08 '15 at 05:17
@Ahmedilyas, the value won't be empty, because in `try { g = new Guid("blah"); }...` if an exception is thrown in Guid constructor, `g` won't be assigned, as there are 2 operations - instantiation and assignment and assignment won't execute — d_z, Apr 08 '15 at 05:42

score 7 · Answer 1 · answered Apr 08 '15 at 05:19

7

This is more a matter of style than anything else - functionally it's superfluous and the compiler may even optimize away the assignment to Guid.Empty in the generated code.

Defensive coding recommends that variable should always have an initial value explicitly assigned. Why? Because it reduces ambiguity, especially for those unfamiliar with the details of a given programming language/platform. For example one might ask:

What is the default value of a Guid?
Should it be a new Guid?
Should it be all zero's?
Should it be some other indeterminate state?

In stating:

Guid id = Guid.Empty;

All these questions are essentially answered by reading the code and without having to resort to reading the documentation and/or source. Further, it's explicit that the variable id starts out being empty which indicates to the reader that it'll have its value set later in the code.

answered Apr 08 '15 at 05:19

Rich Turner

10,800
1
51
68

1

Are you saying that this statement `this = Guid.Empty;` in the constructor makes understanding of the code easier? Not for me. For me it raises a question "how is it gonna be used when an exception is raised". For variables I agree you should initialize, but in this case it's confusing. – d_z Apr 08 '15 at 05:36
1

If it's for defensive coding practices, why isn't `this = Guid.Empty;` placed at the top before the null check on the string? Also none of the other constructors have this. Why would that be? – neodymium Apr 08 '15 at 07:32
1

"Defensive coding recommends that variable should always have an initial value explicitly assigned. " This is not exactly defensive because it turns off the ability of the compiler to warn you of using a bogus values. This practice *increases* risk and increases clutter. – usr Apr 08 '15 at 14:39
@neodymium Having the assignment before the if statement would only be `defensive` against someone who would use an object whose constructor threw an `ArgumentNullException`. There's no help for those people. The idea is to do error checking before you do any real work which could be long-running. In this case the assignment gets optimized out - but we do it just to be consistent. [This question talks about it](http://programmers.stackexchange.com/questions/18454/should-i-return-from-a-function-early-or-use-an-if-statement). Be sure to read the linked articles/questions as well. – Shaz Apr 08 '15 at 15:25
@neodymium The reason this variable assignment isn't performed at the top of the function is that the code above it is API contract checking code that validates the incoming `g` parameter is not invalid. Assigning a value to `this` is not part of that contract validation. – Rich Turner Apr 08 '15 at 19:01
@d_z I think you're getting wrapped around the assignment to `this`. Note that a `Guid` is a value-type (i.e. Struct), not a reference-type. The value of a `Guid` is a Guid and `Guid.Empty` is a Guid of all-zero's. – Rich Turner Apr 08 '15 at 19:04
@usr using uninitialized variables in C# is a compiler error. Also, assigning an initial value **is** defensive because you then can't run into undefined behavior. Try this in, for example, C; if you don't initialize your variables, they will contain whatever value happens to be left over in that location on the stack from a previous function call. This can cause all kinds of weird and unexpected side-effects. – Tom Lint Aug 11 '16 at 09:21
@TomLint there is no undefined behavior caused by uninitialized values in C#. Even in C, I'm not sure what's worse: Risking silently wrong computations or risking UB. UB has sanitiziers at least that can detect uninitialized values in many cases at runtime. Setting an init value silences *all* such warnings (incl. compiler warnings). This is not clearly a safer approach. – usr Aug 11 '16 at 11:36
@usr "there is no undefined behavior caused by uninitialized values in C#" Of course there isn't; you're required to initialize all your value types, and references default to null in C#. "Even in C, I'm not sure what's worse: Risking silently wrong computations or risking UB." Neither are acceptable to me, which is why I set the compiler to error on uninitialized variables, and always assign the default value I need in the function. "Setting an init value silences _all_ such warnings" Since you're manually setting the initial value, I don't see the problem. – Tom Lint Mar 01 '17 at 08:56
@TomLint well, you must pick a value to set and depending on what the function does later that value might be the wrong one. There's *never* a safe default value. That's what I mean by silent wrong computations. – usr Mar 03 '17 at 16:54
@usr That makes no sense. You base your initial value on what's needed. If you're writing function A and know it's safe to use 0, then you initialize to 0. Sometimes you need -1 as a starting point, and you initialize to -1. From what I can gather from your replies, you appear to be thinking about using a general default value across *all* code, which is not what I mean. – Tom Lint Mar 28 '17 at 07:57

Damien_The_Unbeliever · Answer 2 · 2015-04-08T16:39:45.697

I would guess that this is a historical artifact.

In the 3.5 version, the GUID is constructed in-place, with all of the text parsing all included within the constructor method itself.

I would guess that at one point the developers decided to refactor all of the parsing code out into helper methods¹, at which point they'd have hit a compiler error because you have to definitely assign this before calling instance methods.

The GuidResult helper structure seems to have been introduced at some other point in time, at which point these parsing methods could then become static and work against the GuidResult rather than the actual Guid currently being constructed - at which point everything becomes simplified again and that definite assignment isn't required.

This is what reflector decompiles the 3.5 version to:

public Guid(string g)
{
    if (g == null)
    {
        throw new ArgumentNullException("g");
    }
    int startIndex = 0;
    int parsePos = 0;
    try
    {
        int num2;
        long num3;
        if (g.IndexOf('-', 0) >= 0)
        {
            string str = g.Trim();
            if (str[0] == '{')
            {
                if ((str.Length != 0x26) || (str[0x25] != '}'))
                {
                    throw new FormatException(Environment.GetResourceString("Format_GuidInvLen"));
                }
                startIndex = 1;
            }
            else if (str[0] == '(')
            {
                if ((str.Length != 0x26) || (str[0x25] != ')'))
                {
                    throw new FormatException(Environment.GetResourceString("Format_GuidInvLen"));
                }
                startIndex = 1;
            }
            else if (str.Length != 0x24)
            {
                throw new FormatException(Environment.GetResourceString("Format_GuidInvLen"));
            }
            if (((str[8 + startIndex] != '-') || (str[13 + startIndex] != '-')) || ((str[0x12 + startIndex] != '-') || (str[0x17 + startIndex] != '-')))
            {
                throw new FormatException(Environment.GetResourceString("Format_GuidDashes"));
            }
            parsePos = startIndex;
            this._a = TryParse(str, ref parsePos, 8);
            parsePos++;
            this._b = (short) TryParse(str, ref parsePos, 4);
            parsePos++;
            this._c = (short) TryParse(str, ref parsePos, 4);
            parsePos++;
            num2 = TryParse(str, ref parsePos, 4);
            parsePos++;
            startIndex = parsePos;
            num3 = ParseNumbers.StringToLong(str, 0x10, 0x2000, ref parsePos);
            if ((parsePos - startIndex) != 12)
            {
                throw new FormatException(string.Format(CultureInfo.CurrentCulture, Environment.GetResourceString("Format_GuidInvLen"), new object[0]));
            }
            this._d = (byte) (num2 >> 8);
            this._e = (byte) num2;
            num2 = (int) (num3 >> 0x20);
            this._f = (byte) (num2 >> 8);
            this._g = (byte) num2;
            num2 = (int) num3;
            this._h = (byte) (num2 >> 0x18);
            this._i = (byte) (num2 >> 0x10);
            this._j = (byte) (num2 >> 8);
            this._k = (byte) num2;
        }
        else if (g.IndexOf('{', 0) >= 0)
        {
            int num5 = 0;
            int length = 0;
            g = EatAllWhitespace(g);
            if (g[0] != '{')
            {
                throw new FormatException(Environment.GetResourceString("Format_GuidBrace"));
            }
            if (!IsHexPrefix(g, 1))
            {
                throw new FormatException(string.Format(CultureInfo.CurrentCulture, Environment.GetResourceString("Format_GuidHexPrefix"), new object[] { "{0xdddddddd, etc}" }));
            }
            num5 = 3;
            length = g.IndexOf(',', num5) - num5;
            if (length <= 0)
            {
                throw new FormatException(Environment.GetResourceString("Format_GuidComma"));
            }
            this._a = ParseNumbers.StringToInt(g.Substring(num5, length), 0x10, 0x1000);
            if (!IsHexPrefix(g, (num5 + length) + 1))
            {
                throw new FormatException(string.Format(CultureInfo.CurrentCulture, Environment.GetResourceString("Format_GuidHexPrefix"), new object[] { "{0xdddddddd, 0xdddd, etc}" }));
            }
            num5 = (num5 + length) + 3;
            length = g.IndexOf(',', num5) - num5;
            if (length <= 0)
            {
                throw new FormatException(Environment.GetResourceString("Format_GuidComma"));
            }
            this._b = (short) ParseNumbers.StringToInt(g.Substring(num5, length), 0x10, 0x1000);
            if (!IsHexPrefix(g, (num5 + length) + 1))
            {
                throw new FormatException(string.Format(CultureInfo.CurrentCulture, Environment.GetResourceString("Format_GuidHexPrefix"), new object[] { "{0xdddddddd, 0xdddd, 0xdddd, etc}" }));
            }
            num5 = (num5 + length) + 3;
            length = g.IndexOf(',', num5) - num5;
            if (length <= 0)
            {
                throw new FormatException(Environment.GetResourceString("Format_GuidComma"));
            }
            this._c = (short) ParseNumbers.StringToInt(g.Substring(num5, length), 0x10, 0x1000);
            if ((g.Length <= ((num5 + length) + 1)) || (g[(num5 + length) + 1] != '{'))
            {
                throw new FormatException(Environment.GetResourceString("Format_GuidBrace"));
            }
            length++;
            byte[] buffer = new byte[8];
            for (int i = 0; i < 8; i++)
            {
                if (!IsHexPrefix(g, (num5 + length) + 1))
                {
                    throw new FormatException(string.Format(CultureInfo.CurrentCulture, Environment.GetResourceString("Format_GuidHexPrefix"), new object[] { "{... { ... 0xdd, ...}}" }));
                }
                num5 = (num5 + length) + 3;
                if (i < 7)
                {
                    length = g.IndexOf(',', num5) - num5;
                    if (length <= 0)
                    {
                        throw new FormatException(Environment.GetResourceString("Format_GuidComma"));
                    }
                }
                else
                {
                    length = g.IndexOf('}', num5) - num5;
                    if (length <= 0)
                    {
                        throw new FormatException(Environment.GetResourceString("Format_GuidBraceAfterLastNumber"));
                    }
                }
                uint num8 = (uint) Convert.ToInt32(g.Substring(num5, length), 0x10);
                if (num8 > 0xff)
                {
                    throw new FormatException(Environment.GetResourceString("Overflow_Byte"));
                }
                buffer[i] = (byte) num8;
            }
            this._d = buffer[0];
            this._e = buffer[1];
            this._f = buffer[2];
            this._g = buffer[3];
            this._h = buffer[4];
            this._i = buffer[5];
            this._j = buffer[6];
            this._k = buffer[7];
            if ((((num5 + length) + 1) >= g.Length) || (g[(num5 + length) + 1] != '}'))
            {
                throw new FormatException(Environment.GetResourceString("Format_GuidEndBrace"));
            }
            if (((num5 + length) + 1) != (g.Length - 1))
            {
                throw new FormatException(Environment.GetResourceString("Format_ExtraJunkAtEnd"));
            }
        }
        else
        {
            string s = g.Trim();
            if (s.Length != 0x20)
            {
                throw new FormatException(Environment.GetResourceString("Format_GuidInvLen"));
            }
            for (int j = 0; j < s.Length; j++)
            {
                char c = s[j];
                if ((c < '0') || (c > '9'))
                {
                    char ch2 = char.ToUpper(c, CultureInfo.InvariantCulture);
                    if ((ch2 < 'A') || (ch2 > 'F'))
                    {
                        throw new FormatException(Environment.GetResourceString("Format_GuidInvalidChar"));
                    }
                }
            }
            this._a = ParseNumbers.StringToInt(s.Substring(startIndex, 8), 0x10, 0x1000);
            startIndex += 8;
            this._b = (short) ParseNumbers.StringToInt(s.Substring(startIndex, 4), 0x10, 0x1000);
            startIndex += 4;
            this._c = (short) ParseNumbers.StringToInt(s.Substring(startIndex, 4), 0x10, 0x1000);
            startIndex += 4;
            num2 = (short) ParseNumbers.StringToInt(s.Substring(startIndex, 4), 0x10, 0x1000);
            startIndex += 4;
            parsePos = startIndex;
            num3 = ParseNumbers.StringToLong(s, 0x10, startIndex, ref parsePos);
            if ((parsePos - startIndex) != 12)
            {
                throw new FormatException(string.Format(CultureInfo.CurrentCulture, Environment.GetResourceString("Format_GuidInvLen"), new object[0]));
            }
            this._d = (byte) (num2 >> 8);
            this._e = (byte) num2;
            num2 = (int) (num3 >> 0x20);
            this._f = (byte) (num2 >> 8);
            this._g = (byte) num2;
            num2 = (int) num3;
            this._h = (byte) (num2 >> 0x18);
            this._i = (byte) (num2 >> 0x10);
            this._j = (byte) (num2 >> 8);
            this._k = (byte) num2;
        }
    }
    catch (IndexOutOfRangeException)
    {
        throw new FormatException(Environment.GetResourceString("Format_GuidUnrecognized"));
    }
}

So obviously between 3.5 and 4.5.2, the helper methods have been introduced. From there, it's supposition that the helper methods were introduced first (as instance methods, and so requiring definite assignment) and then the helper struct (GuidResult) was introduced afterwards.

If you copy all of the 4.5.2 code into a new project, remove the line you ask about and compile, everything is still fine.

¹Probably to support the introduction of Parse/TryParse methods on Guid which seem to have appeared in the .NET 4.0 time frame.

score 1 · Answer 3 · answered Apr 08 '15 at 06:45

In essence you answered your own question to an extent.

If an error is thrown and the line this = Gui.Empty is not there it could result in an undefined value beign assigned to the variable. Thus even though you have an exception the variable itself is undefined.

There can be use cases where just that is a bad case. For example if the variable is a global one, or one that needs to be used even if the gui generation failed or it needs to at least be checked for a value, .... . Thus even though an exception is thrown it is better to have a defined value (in this case 0s) than an undefined value at all which could lead to erratic behaviour if someone NEEDS a value even if the constructor fails for some reason.

And that is the main reason for such a construct to exist (at least I hope that microsoft had THAT reason as honestly aside from that I can't think of a logical reason for doing that).

score 0 · Answer 4 · answered Apr 08 '15 at 14:21

I think I've got a satisfying explanation of that line.

In a struct parameterized constructor you have to assign all the fields, otherwise you get something like this "Error 1 Field 'My.Guid._j' must be fully assigned before control is returned to the caller C:\Work\Projects\Guid\Guid\Program.cs 153 16 Guid"

AFAIU, the reason why you have to do that is that when you call a parameterized struct constructor you override default CLR struct initialization behaviour which just zeroes allocated memory instead of initializing every field. (I might be wrong though)

Normally you'd call a parameterless constructor with :this() from any parameterized constructor to initialize all members and that would work here as well. But that adds an extra call and extra memory allocations, so for performance optimization they replaced a call to default constructor with assignment to a static readonly field Guid.Empty.

score -1 · Answer 5 · answered Apr 08 '15 at 05:14

-1

Guid.Empty will assign all zeros. If you don't want to assign all zeros, you may use Guid.NewGuid() method. Here the default is being assigned.

answered Apr 08 '15 at 05:14

Shyju

203
1
5

2

Not sure how this answers the question because `this=Guild.Empty` is technically pointless - default value of struct is already "all zeros" even before that assignment. – Alexei Levenkov Apr 08 '15 at 05:24
@Shyju, I can't see how it answers the question. And how can you use Guid.NewGuid() in Guid source code unless you work in MS? – d_z Apr 08 '15 at 05:24
@ d_z, I tried to explain what does that line means. Not interested in any controversy. I just took a step ahead and explained, if tomorrow MS think, why can't we assign a non-zero, they will have to generate some Guid using NewGuid method, isn't it? – Shyju Apr 08 '15 at 05:41

Why does the source code for the Guid constructor contain the line "this = Guid.Empty"?

5 Answers5