2 Replies - 6531 Views - Last Post: 21 January 2013 - 06:00 AM

#1 Xna4life  Icon User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 78
  • Joined: 21-February 12

Refactor if statemets

Posted 21 January 2013 - 05:25 AM

Hey guys...So atm im importing an excel doc into my project. I'm then using if statements to display the appropriate error message if any columns in the spreadsheet are empty. My code is working but it just doesn't look tidy. Is there anyway to refactor all the if statements into a loop to display the error message, possibly another method which checks all?? checkIfColumnIsEmpty is a bool method which returns true if the column is empty

//if either column 0 and 1 are empty && column 2,3,4 and 5 are not
if (!checkIfColumnisEmpty(r.ItemArray[0]) || !checkIfColumnisEmpty(r.ItemArray[1])
&& checkIfColumnisEmpty(r.ItemArray[2]) && checkIfColumnisEmpty(r.ItemArray[3])
&& checkIfColumnisEmpty(r.ItemArray[4]) && checkIfColumnisEmpty(r.ItemArray[5]))
{
if (checkIfColumnisEmpty(r.ItemArray[0]) && !checkIfColumnisEmpty(r.ItemArray[1]))
{
throw new ImportBOQException("Error importing document: First column is empty");
}
else if (!checkIfColumnisEmpty(r.ItemArray[0]) && checkIfColumnisEmpty(r.ItemArray[1]))
{
throw new ImportBOQException("Error importing document: Second column is empty");
}

else if (!checkIfColumnisEmpty(r.ItemArray[0]) && !checkIfColumnisEmpty(r.ItemArray[1]))
{
//all columns are valid so...
Column0inSpreadsheet = r.ItemArray[0] as string;
Column1inSpreadsheet = r.ItemArray[1] as string;

//Other code which performs other operations, once the level as reached this far
}
} 

//if column 0 and 1 are NOT empty && Either column 2,3,4 or 5 is empty
else if (checkIfColumnisEmpty(r.ItemArray[0]) && checkIfColumnisEmpty(r.ItemArray[1])
|| !checkIfColumnisEmpty(r.ItemArray[2]) || !checkIfColumnisEmpty(r.ItemArray[3])
|| !checkIfColumnisEmpty(r.ItemArray[4]) || !checkIfColumnisEmpty(r.ItemArray[5]))
{
if (checkIfColumnisEmpty(r.ItemArray[2]))
{
throw new ImportBOQException("Error importing document: Third column is empty");
}
else if (checkIfColumnisEmpty(r.ItemArray[3]))
{
throw new ImportBOQException("Error importing document: Fourth column is empty");
}
else if (checkIfColumnisEmpty(r.ItemArray[4]))
{
throw new ImportBOQException("Error importing document: Fifth column is empty");
}
else if (checkIfColumnisEmpty(r.ItemArray[5]))
{
throw new ImportBOQException("Error importing document: Sixth column is empty");
}
else
//all columns are valid so...
{ Column2inSpreadsheet = (r.ItemArray[2]) as string;
Column3inSpreadsheet = (r.ItemArray[3]) as string;
Column4inSpreadsheet = (r.ItemArray[4]) as string;
Column5inSpreadsheet = (r.ItemArray[5]) as string;

//Other code which performs other operations, once the level as reached this far
}
}
else
//other errors ot related to empty colums
{
throw new Exception("Error Uploading");
}
}


Is This A Good Question/Topic? 0
  • +

Replies To: Refactor if statemets

#2 andrewsw  Icon User is online

  • Fire giant boob nipple gun!
  • member icon

Reputation: 2879
  • View blog
  • Posts: 9,552
  • Joined: 12-December 12

Re: Refactor if statemets

Posted 21 January 2013 - 05:44 AM

The first thing that I would do is to use six boolean values, or an array of these, to store the functions' return value for each of the columns. This will make it easier to work with, easier to read!, and you wouldn't be re-checking the same column(s) over and over.

Then I would check firstly if all of them are empty, or none of them are.

You might post your is-empty function as well.

This post has been edited by andrewsw: 21 January 2013 - 05:47 AM

Was This Post Helpful? 0
  • +
  • -

#3 andrewsw  Icon User is online

  • Fire giant boob nipple gun!
  • member icon

Reputation: 2879
  • View blog
  • Posts: 9,552
  • Joined: 12-December 12

Re: Refactor if statemets

Posted 21 January 2013 - 06:00 AM

Something like

Dim colEmpty(6) As Boolean
Dim x As Integer
Dim countEmpty As Integer = 0

For x = 0 To 5
    colEmpty(x) = checkIfColumnisEmpty(r.ItemArray[x])
    If colEmpty(x) Then
        countEmpty += 1
    End If
Next x
'countEmpty == 6 means they are all empty


Oops, you are using C#! Never mind, you get the idea.

This post has been edited by andrewsw: 21 January 2013 - 06:07 AM

Was This Post Helpful? 0
  • +
  • -

Page 1 of 1