too many conditions [closed]

I have this piece of code I developed but it doesn't look efficient to me, can anyone improve it please.

 private void AddToDataBase(SPItemEventProperties properties)
{
DataAccess DA = new DataAccess(ConnectionString());

DetailsRow result = DA.GetDetails(properties.WebUrl);

if (result != null && string.IsNullOrEmpty(result.URL))
{
DA.DeleteDetails(properties.WebUrl);
}

if (bool.Parse(properties.ListItem.Properties["Regular Customer"].ToString()) == true)
{
DA.InsertDetails(properties.Name, properties.WebUrl, DateTime.Now);
}
else
{
if (NumberOfInvoices(properties) > 0)
{
DA.InsertDetails(properties.Name, properties.WebUrl, DateTime.Now);
}

else
{
DeleteInvoiceDocument(properties);
}
}
}

• Hello TimeToThine. How do you suppose anyone can answer that without knowing what your conditions are? Your question is at risk of being closed as "not a real question" if you don't include this critical information. By the way, your assumption is probably wrong - even if the code is different, it can have a big influence on the possible refactorings, so you should show that too. – Adam Sep 6 '12 at 9:08
• basically my conditions are just checking few results returned by a DataTable and it checks if they are null or not + few other validations, I want this code to be reviewed based on structure of it as I got 1 empty "else" statement in there which doesn't look great to me to be honest :) – Muhammad Raja Sep 6 '12 at 9:13

Since I don't have the SharePoint libraries I wasn't able to test my solution, but it should work.

1. Use proper indentation and formatting.
2. == true is redundant and can be removed.
3. Instead of the following:

bool.Parse(properties.ListItem.Properties["Regular Customer"].ToString())


just convert to bool (casting won't work because of SharePoint):

Convert.ToBoolean(properties.ListItem.Properties["Regular Customer"])

4. Reduce nesting by using else if instead of else { if (...) }

5. Use || instead of two seperate conditions with the same associated code to avoid redundancy.

Result of refactoring

private void AddToDataBase(SPItemEventProperties properties)
{
DataAccess dataAccess = new DataAccess(ConnectionString());
DetailsRow result = dataAccess.GetDetails(properties.WebUrl);
bool isRegular = Convert.ToBoolean(properties.ListItem.Properties["Regular Customer"])

if (result != null && string.IsNullOrEmpty(result.URL))
{
dataAccess.DeleteDetails(properties.WebUrl);
}

if (isRegular || NumberOfInvoices(properties) > 0)
{
dataAccess.InsertDetails(properties.Name, properties.WebUrl, DateTime.Now);
}
else
{
DeleteInvoiceDocument(properties);
}
}

• my code is ignoring rest of code after this line "bool isRegularCustomer = (bool)properties.ListItem.Properties["Regular Customer"];" tried debugging but when it comes to this line it simply refreshes SharePoint page and there's nothing left to process – Muhammad Raja Sep 6 '12 at 11:13
• the cast to bool seems to be throwing an exception. try using the old version isRegularCustomer = bool.Parse(...ToString()) instead. – Adam Sep 6 '12 at 11:14
• its something related to sharepoint I guess, I tried it so many times before as you suggested but never worked, so used to prase it – Muhammad Raja Sep 6 '12 at 11:19
• hm... according to this comment on a related StackOverflow post, Convert.ToBoolean should also work. – Adam Sep 6 '12 at 11:23
• I prefer it because we can leave out the call to ToString(). There are some subtle differences, but the main reason for me is that Convert.ToBoolean will work with any compatible type, not just with string. – Adam Sep 6 '12 at 11:28