locked
Code Review on checking for null RRS feed

  • Question

  • I wanted to get opinions on how I handled this particular code dealing with first the OrderItems which is under the covers List<OrderItem> and how I handled the checking if the list is empty or not so I don't get an array out of bounds exception in the rest of the properties.

            protected OrderItem FirstItem
            {
                get { return oitems[0]; }
            }
    
            protected int FirstItemStoreID
            {
                get { return FirstItem != null ? oitems[0].StoreID : 0;}
            }
    
            protected string FirstItemStoreName
            {
                get { return FirstItem != null ? Catalog.Catalog.Default.GetStore(FirstItem.StoreID).DisplayName : string.Empty; }
            }
    
            protected string FirstItemDeptName
            {
                get { return FirstItem != null ? GetDept(FirstItem.DeptID).Name : string.Empty; }
            }
    
            protected Product FirstItemProduct
            {
                get { return GetProduct(FirstItem.ProductID); }
            }
    
            protected string FirstItemProductImageUrl
            {
                get { return FirstItemProduct != null ? ImgUrl(FirstItemProduct.Image, false) : string.Empty; }
            }
    I don't want to just check for oitems[0].Count > 0  every time in all the rest of the properties that want to use that OrderITem. and so I thought the way I did this was cleaner..check the property for != null ?  I could be wrong but is this a good check for nulls or empty list in this case for each of those properties?
    C# Web Developer
    Monday, November 23, 2009 10:54 PM

Answers

  • one option is when u declare a variable u can assign its default value, say like below

    string strName = string.Empty;
    int iDepartmentID =0;

    if u do like the above, u don't have to check for is null, as there will be a default value when something is not assigned...  any way we cant use a variable without declaring it, in that place u need to do this tune up... then it will work fine in all the place's

    hope this helped u and my explanation 



    Narayanan Dayalan - Zeetaa Business Solutions ------- Please "Mark As Answer", if my answer works well with ur Query
    Tuesday, November 24, 2009 2:33 AM
  • Hi NoEgo,

    Its a matter of style. As you said, there are plenty of ways to write code. But what ever you have written looks cleaner.

    Only problem is for the property FirstItem, you are not checking whether oitems is null or not. If its null, you will get an exception. The rest of your code is pretty Ok. Even in the view of Code Readability, it seems fine.

    Run you code against StyleCop :-), just a suggestion.
    Thanks,
    A.m.a.L
    Dot Net Goodies
    Don't hate the hacker, hate the code
    Wednesday, November 25, 2009 7:37 AM

All replies

  • one option is when u declare a variable u can assign its default value, say like below

    string strName = string.Empty;
    int iDepartmentID =0;

    if u do like the above, u don't have to check for is null, as there will be a default value when something is not assigned...  any way we cant use a variable without declaring it, in that place u need to do this tune up... then it will work fine in all the place's

    hope this helped u and my explanation 



    Narayanan Dayalan - Zeetaa Business Solutions ------- Please "Mark As Answer", if my answer works well with ur Query
    Tuesday, November 24, 2009 2:33 AM
  • Hi NoEgo,

    Its a matter of style. As you said, there are plenty of ways to write code. But what ever you have written looks cleaner.

    Only problem is for the property FirstItem, you are not checking whether oitems is null or not. If its null, you will get an exception. The rest of your code is pretty Ok. Even in the view of Code Readability, it seems fine.

    Run you code against StyleCop :-), just a suggestion.
    Thanks,
    A.m.a.L
    Dot Net Goodies
    Don't hate the hacker, hate the code
    Wednesday, November 25, 2009 7:37 AM