Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a lot of variables that I need to check for exceptions and output the empty field in case of a null returned value (I am reading a calender list from Sharepoint and my program is supposed to send email notifications if some of the conditions are met).

I surrounded my variables with a try-catch with a generic output "variable not found". Here is an example;

try
{
var name = item["Name"].ToString(); 
var dueDate= item["Due Date"].ToString();
.
.//more variables
.   
var Title= item["Title"].ToString();    
}
catch (Exception ex)
                        {
                            Console.WriteLine();
                            Console.WriteLine(ex.Message); //generic message for now
                            Console.WriteLine();
                        }

Is there is a way to handle this better? I know that going to each individual line and adding an exception is an option but it would save me a lot of time if I can just output something like the variable name.

share|improve this question
    
What is the type if item? –  ANeves Nov 21 '12 at 14:26
    
@ANeves SPListItem. –  Ammar Ahmed Nov 21 '12 at 16:09
    
Where do these values come from? Are they settings of some sort? –  Leonid Nov 24 '12 at 4:26
add comment

5 Answers 5

up vote 7 down vote accepted

Refactor into a method:

var name = this.GetVariable("Name"); 
var dueDate= this.GetVariable("Due Date");
.
.//more variables
.   
var Title= this.GetVariable("Title");    

private string GetVariable(string name)
{
    try
    {
        return item[name].ToString(); 
    }
    catch (Exception ex)
                        {
                            Console.WriteLine();
                            Console.WriteLine(name + " not found.");
                            Console.WriteLine();
                            return null;
                        }
}
share|improve this answer
    
Nicely done. +1 –  Ross Patterson Nov 25 '12 at 22:47
add comment

Create a class that wraps the information you need. Within the class you would have a method that retrieves the information, or even retrieves the information and strongly types it.

public sealed class ValueConverter
{
    private readonly IDictionary<string, dynamic> _item;

    public ValueConverter(IDictionary<string, dynamic> item)
    {
        _item = item;
    }

    public T ConvertTo<T>(string fieldName)
    {
        var type = typeof(T);

        var value = _item[fieldName];

        if (value == null)
        {
            throw new ApplicationException(string.Format("Field '{0}' is empty.", fieldName));
        }

        return Convert.ChangeType(value, type);
    }
}

I used IDictionary<string, dynamic> because I don't know what 'item' is.

you would then do something like this:

var converter = new ValueConverter(item);

var name = converter.ConvertTo<string>("name"); 
var dueDate= = converter.ConvertTo<DateTime>("Due Date");
.
.//more variables
.   
var Title = converter.ConvertTo<string>("Title"); 
share|improve this answer
add comment

I prefer a more functional approach to the problem, using a little LINQ. I'm assuming that "item" is a Dictionary. To see how the code behaves, add or remove items from this Dictionary, or play with the values by using null and empty strings. Whitespace left as an exercise to the reader ;)

var required = new [] { "Name", "Due Date", "Title" };
var items = new Dictionary<string, string>{
    {"Name", "Joe"},
    {"Title", "a title"},
    {"Due Date", "11/23/2012"},
    {"Foo", "A non required field"}
};

var filledOutValues = items.Where(i => !String.IsNullOrEmpty(i.Value));
var missingFields = required.Except(filledOutValues.Select(i => i.Key));

if(missingFields.Any()) {
    var missing = missingFields.Aggregate((acc, m) => acc + ", " + m);
    Console.WriteLine("Missing Fields: " + missing);
}
else {
    //Rock On!
}
share|improve this answer
    
Although .Count() == 0 is probably quicker I would personally still prefer to get rid of the isValid and use missingFields.Any() –  dreza Nov 24 '12 at 6:18
    
Great suggestion! Much more readable. I updated the code sample. Thanks! –  M Falanga Nov 28 '12 at 14:35
add comment

This is my two attempts at keeping it simple. I normally try not to rely on the exceptions to detect errors of validation. Therefore one answer with exception and one without.

Also you might want to return a string for the error message from the method instead of writing directly at the console.

        public string ExecuteUsingTry()
    {
        var empty = string.Empty;
        string[] objectsInTheCollection = { "Name", "Due Date", "Title" };
        var item = new Dictionary<string, object>();

        foreach (var key in objectsInTheCollection)
        {
            try
            {
                item[key].ToString();
            }
            catch (Exception ex)
            {
                empty += "\n";
                empty += ex.Message;
                empty += "\n";
            }
        }

        return empty;
    }

Example number two

    public string ExecuteUsingContains()
    {
        var empty = string.Empty;
        string[] objectsInTheCollection = { "Name", "Due Date", "Title" };
        var item = new Dictionary<string, object>();

        foreach (var key in objectsInTheCollection)
        {
            if (!item.ContainsKey(key))
            {
                empty += "\n";
                empty += "Error related to" + key;
                empty += "\n";
            }
            else
            {
              // Your Processing
                item[key].ToString();
            }
        }
        return empty;
    }
share|improve this answer
    
+1 I like the approach without exceptions because the scenario doesn't sound very exceptional and I think it's much better to read here. –  jpfollenius Nov 25 '12 at 12:22
    
smasher I like the second one more also is very predictable what the need is. Also should mention that you might want to use a string.format in those examples when creating empty I suffered a bit of tunnel vision when writing the samples. –  Avatare Mar 24 '13 at 16:53
add comment

If you actually just need the empty string instead of null, and you don't care about alerting the user that the value was null, just do:

var name = (item["Name"] ?? "").ToString(); 
var dueDate= (item["Due Date"] ??).ToString();
...

Alternatively, you could even make a small helper method to do this for you:

public static string GetValue(object o)
{
    if (o == null) return "";
    else return o.ToString();
}

and call it as:

var name = GetValue(item["Name"]); 
var dueDate= GetValue(item["Due Date"]);

You won't get any exceptions with either of these ways - just empty strings. You should avoid throwing (or deliberately expecting) an exception whenever possible. See this answer and some of the other answers on that page - exceptions should be for an unexpected scenario, not normal control flow.


In the case where the column doesn't even exist when it has no data (which indicates an issue with the generation of the data), you can modify it as follows:

public static string GetValue(DataRow row, string column)
{
    if (!row.Table.Columns.Contains(column) || row[column] == null) return "";
    else return row[column].ToString();
}
share|improve this answer
    
What if item doesn't have ["name"]? Exception does jump out. –  ANeves Nov 21 '12 at 14:27
    
@ANeves - That's an issue with the process which gets the data - if it's not returned in a standard format, then you're going to have a lot more issues with processing it. However, edited to handle that scenario. –  Bobson Nov 21 '12 at 14:52
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.