Link to home
Start Free TrialLog in
Avatar of stellyuk
stellyuk

asked on

problem with if statement

Hi there,
I have the following if statement that is supposed to check the dates, I just need to know if I have done this right??
            if (PayReceived < BikeRecieved)
            {
                PaymentPeriod = "A";
            }
            else if ((PayReceived > BikeRecieved) && (PayReceived <= firstDate))
            {
                PaymentPeriod = "B";
            }
            else if ((PayReceived > firstDate) && (PayReceived <= secondDate))
            {
                PaymentPeriod = "C";
            }
            else if ((PayReceived > secondDate) && (PayReceived <= thirdDate))
            {
                PaymentPeriod = "D";
            }
            else if ((PayReceived > thirdDate) && (PayReceived <= fourthDate))
            {
                PaymentPeriod = "E";
            }
            else if ((PayReceived > fourthDate) && (PayReceived <= fifthDate))
            {
                PaymentPeriod = "F";
            }
            else if ((PayReceived > fifthDate) && (PayReceived <= sixthDate))
            {
                PaymentPeriod = "G";
            }
            else if (PayReceived > sixthDate)
            {
                PaymentPeriod = "H";
            }
            else
            {
                PaymentPeriod = "null";
            }
Can you please let me know asap

Thanks,
Stelly
Avatar of lucky_james
lucky_james
Flag of India image

yes, seems to be right.......
let me know if you are facing any problems in it.........

Best Regards,
James
Looks fine. But long 'IF' like this imo is not a good thing.
Avatar of stellyuk
stellyuk

ASKER

Well bungHoc, if you have a better idea, please let me hear it.

Stelly
Actually the very first line should probably use <= instead of just <, because currently if PayReceived is exactly equal to BikeRecieved, the PaymentPeriod will be set to "null".

- Ben

alternative way to do it:

DateTime[] dates = new DateTime[] { BikeRecieved, firstDate, secondDate, thirdDate, fourthDate, fifthDate, sixthDate };

PaymentPeriod = "null";
if (PayReceived > BikeRecieved)
{
     for (int i=0; i<dates.Length-1; i++)
          if (PayReceived > dates[i] && PayReceived <= dates[i+1])
          {
                PaymentPeriod = ('A' + (char)i).ToString();
                break;
          }
}
else
     PaymentPeriod = "A";
sorry, it should be:
                PaymentPeriod = ('A' + (char)(i+1)).ToString();
Okay.. Jaime beat me to it already.
simplified version:


      DateTime[] dates = new DateTime[] { BikeRecieved, firstDate, secondDate, thirdDate, fourthDate, fifthDate, sixthDate };
 
     PaymentPeriod = "null";
     for (int i=0; i<dates.Length; i++)
          if (PayReceived <= dates[i])
          {
                PaymentPeriod = ('A' + (char)i).ToString();
                break; 
          }

Open in new window

ASKER CERTIFIED SOLUTION
Avatar of duckets
duckets
Flag of United Kingdom of Great Britain and Northern Ireland image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Heh, BTW, I also noticed that although you spelled ;Received' correctly in 'PayReceived', you spelled it incorrectly in 'BikeRecieved'! :-)
duckets,

I'm not 'by the book' dude. And converting to a loop with an array is not just refactoring for refactoring's sake. Putting them all into array reduce trouble for you if let's say you have A-Z categories.
>>whereas converting it to an array and looping through is much more complex, less readable....
This is usually solved by putting a clear comment about the purpose of the loop

>>and probably less efficient for the computer to execute
I don't know how, the numbers of IFs will be the same
bunHoc wrote:
>> Putting them all into array reduce trouble for you if let's say you have A-Z categories.

If stellyuk wanted to adapt his/her code so the number of categories could be variable, then sure, an array + loop might then be the best solution, but that's not what the original question was.

jaime_olivares wrote:
>> >> and probably less efficient for the computer to execute
>> I don't know how, the numbers of IFs will be the same

Because in addition to the IFs, you are also building the array, performing the evaluation for the loop, retrieving items from the array, computing the character code for the correct character and then converting it to a string.
>>Because in addition to the IFs, you are also building the array, performing the evaluation for the loop, retrieving items from the array, computing the character code for the correct character and then converting it to a string.

and how many microseconds do you estimate it will take?
Very few. But still - it is less efficient and less readable than stellyuk's original code.
>>But still - it is less efficient and less readable than stellyuk's original code.
Thanks for sharing your opinion.
Hey All,

Thanks for all your comments, but would this be better:

Let me know what you think...
Stelly
            else if ((DateTime.Compare(PayReceived,BikeRecieved) >0) && (DateTime.Compare(PayReceived,firstDate) <=0))
            {
                PaymentPeriod = "B";
            }
            else if ((DateTime.Compare(PayReceived,firstDate) >0) && (DateTime.Compare(PayReceived,secondDate) >=0))
            {
                PaymentPeriod = "C";
            }
            else if ((DateTime.Compare(PayReceived,secondDate) >0) && (DateTime.Compare(PayReceived,thirdDate) >=0))
            {
                PaymentPeriod = "D";
            }
            else if ((DateTime.Compare(PayReceived,thirdDate) >0) && (DateTime.Compare(PayReceived,fourthDate) >=0))
            {
                PaymentPeriod = "E";
            }
            else if ((DateTime.Compare(PayReceived,fourthDate) >0) && (DateTime.Compare(PayReceived,fifthDate) >=0))
            {
                PaymentPeriod = "F";
            }
            else if ((DateTime.Compare(PayReceived,fifthDate) >0) && (DateTime.Compare(PayReceived,sixthDate) >=0))
            {
                PaymentPeriod = "G";
            }
            else if (DateTime.Compare(PayReceived,sixthDate) >0)
            {
                PaymentPeriod = "H";
            }
            else
            {
                PaymentPeriod = "null";
            }

Open in new window

the DateTime.Compare() is unnecessary also a single comparison is enough in each if () statement.