# Cleaning up conditional statements regarding new values

Please suggest ways of cleaning up this code.

Hashtable newValues = e.Values;

string NewPosition = null;
string NewFirst = null;
string NewLast = null;
string NewEmail = null;
int NewAppt = 0;

if (newValues["Position"] == null)
{
NewPosition = "";
}
else
{
NewPosition = newValues["Position"].ToString();
}

if (newValues["First Name"] == null)
{
NewFirst = "";
}
else
{
NewFirst = newValues["First Name"].ToString();
}

if (newValues["Last Name"] == null)
{
NewLast = "";
}
else
{
NewLast = newValues["Last Name"].ToString();
}

if (newValues["Email"] == null)
{
NewEmail = "";
}
else
{
NewEmail = newValues["Email"].ToString();
}

if (newValues["Appts"] == null)
{
NewAppt = 0;
}
else
{
NewAppt = 1;
}

• I would suggest rewording the question to only have a representative snippet of that pattern, rather than repeating it 5 times
– Rob H
Sep 25 '12 at 19:23
• I didn't know that was an option. Thanks @DaveZych. I will keep that in mind for future postings like this one.
– James Wilson
Sep 25 '12 at 20:02

You can use the null-coalescing operator ?? like this:

NewPosition = (newValues["Position"] ?? "").ToString();


The core of this approach is this expression:

newValues["Position"] ?? ""


The ?? operator evaluates the left expression first (i.e. newValues["Position"]), and uses it as the result if it's not null. If the first expression is null, the second expression is evaluated, and its result is returned as the overall result of the expression.

For the last one, use ?:

NewAppt = (newValues["Appts"] != null) ? 1 : 0;

• Does this also assign the value if it is not null? Could you explain how this works just so I understand it?
– James Wilson
Sep 25 '12 at 19:24
• And do I create the var first like string NewPosition; or Can I do a full assignment like string NewPosition = (newValues["Position"] ?? "").ToString();
– James Wilson
Sep 25 '12 at 19:25
• +1 Outstanding. Though I'd replace "" with string.Empty as a matter of habit. Sep 26 '12 at 14:46
• @AMissico It does make things harder to developers unfamiliar with the ?? operator. Developers who are familiar with their basic technology, on the other hand, tend to appreciate the beauty and the brevity of this approach. Oct 22 '13 at 19:35
• @AMissico Note that ?? can be replaced with + without changing the validity of your statement. Yet very few people in their right mind write AddOne(length) instead of length+1, and it does not make anyone's code any harder to "maintain, reuse, and read". Oct 22 '13 at 20:10

While these are all good suggestions, I would wrap the code into a method.

NewPosition = GatherValue("Position");
NewFirst = GatherValue("First Name");

...
string GatherValue(string name) {
return newValues[name] == null ? "" : newValues[name].ToString();
}

• Does this offer any benefit over the other option, or is it just personal preference?
– James Wilson
Sep 25 '12 at 19:32
• @James: Data container access method encapsulation/extraction. Sep 25 '12 at 20:01
• I would need to make a seperate method for the int portion I am assuming? Or modify the GatherValue to return a different value if the one passed in is Appts right?
– James Wilson
Sep 25 '12 at 20:04
• @James Wilson; Yes, create another method for the int. Sep 27 '12 at 0:58
• Personal preference based on a couple decades of experience. :O) Sep 27 '12 at 0:59

In case of Hashtable:

string value = ht["key"] as string ?? "";


But it's better to use generic thus type-safe Dictionary<string, string>:

string value = dic["key"] ?? "";


or Dictionary<string, objects>:

string value = ((string)dic["key"]) ?? "";


My preferred option is using the ternary operator when you are doing operations such as that.

Here is a brief example:

string NewPosition = newValues["Position"] != null ? newValues["Position"].ToString() : String.Empty;


The way it work is like this:

variable = boolean expression ? value that gets assigned when true : value that gets assigned when false.

You must be careful not to make your code difficult to read when using the ternary operator since it puts it in one line, however I find in most cases it makes it more readable.

This is a pattern that will reduce the lines, and IMO, make it easier to read.

Old:

if (newValues["Position"] == null)
{
NewPosition = "";
}
else
{
NewPosition = newValues["Position"].ToString();
}


New:

NewPosition = newValues["Position"] == null ? string.Empty : newValues["Position"].ToString();


As for the integer field:

NewAppt = newValues["Appts"] == null ? 0 : 1;

Other than null coalescing operator, you could also simply add empty string

NewFirst = newValues["First Name"]+"";


For the following reason:

String concatenation:

string operator +(string x, string y); string operator +(string x, object y); string operator +(object x, string y);

These overloads of the binary + operator perform string concatenation. If an operand of string concatenation is null, an empty string is substituted. Otherwise, any non-string argument is converted to its string representation by invoking the virtual ToString method inherited from type object. If ToString returns null, an empty string is substituted.

This will not work for integers/booleans/etc. Mostly just for strings or anything the .ToString() overload gives you that you actually want. For other cases i'd suggest null coalescing operator or some other approach like other answers provide here.

• This is an old hack that relies on the storage variable to be a string. It is not an improvement. Over the years, this trick has caused by problems in two projects. Sep 27 '12 at 1:03
• This makes is shorter at the expense of introducing a hack (relying on coincidence). Sometimes this is valid but in this case I would disagree. It is not needed.
– usr
Sep 30 '12 at 19:52

This code could be refactored like this:

HashTable newValues = e.Values;
string NewPosition = string.Empty;
string NewFirst = string.Empty;
string NewLast = string.Empty;
string NewEmail = string.Empty;
int NewAppt;

NewPosition = (newValues["Position"] ?? string.Empty).ToString();
NewFirst = (newValues["First Name"] ?? string.Empty).ToString();
NewLast = (newValues["Last Name"] ?? string.Empty).ToString();
NewEmail = (newValues["Email"] ?? string.Empty).ToString();
NewAppt = newValues["Appts"] == null ? 0 : 1


Things to note:

• you don't need to declare the int as 0 - it defaults to 0, NB: C# structs (like int) are not nullable.
• This will just result in null reference exceptions when the values are null Sep 25 '12 at 19:45
• Now it's just like several other existing answers... Sep 25 '12 at 19:53
• @Servy: Doesn't deserve downvote though imo. Sep 25 '12 at 20:00
• Why initialize the local string variables when they are instantly replaced? Extra coding and loss of performance. Sep 26 '12 at 14:48
• @Kryptonite Actually, as it seems these are all done as local variables in a method, they won't be null if you remove the initialization altogether. They'll be undefined until assigned, which the assignments immediately do. Sep 27 '12 at 18:36

Given you're using c# 4 I'd suggest to create a static extension method for your values.

public static class MyExtensions
{
public static string ToValueOrStringEmpty(this object myString)
{
return string.IsNullOrEmpty((string)myString)
? string.Empty
: (string)myString;
}
}


NewPosition = newValues["Position"].ToValueOrStringEmpty();