Want to win a PS4? Go Premium and enter to win our High-Tech Treats giveaway. Enter to Win

x
?
Solved

problem with if statement

Posted on 2007-12-04
18
Medium Priority
?
170 Views
Last Modified: 2010-04-15
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
0
Comment
Question by:stellyuk
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 7
  • 5
  • 3
  • +2
18 Comments
 
LVL 7

Expert Comment

by:lucky_james
ID: 20402111
yes, seems to be right.......
let me know if you are facing any problems in it.........

Best Regards,
James
0
 
LVL 7

Expert Comment

by:bungHoc
ID: 20402119
Looks fine. But long 'IF' like this imo is not a good thing.
0
 

Author Comment

by:stellyuk
ID: 20402145
Well bungHoc, if you have a better idea, please let me hear it.

Stelly
0
NEW Veeam Agent for Microsoft Windows

Backup and recover physical and cloud-based servers and workstations, as well as endpoint devices that belong to remote users. Avoid downtime and data loss quickly and easily for Windows-based physical or public cloud-based workloads!

 
LVL 2

Expert Comment

by:duckets
ID: 20402151
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

0
 
LVL 55

Expert Comment

by:Jaime Olivares
ID: 20402154
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";
0
 
LVL 55

Expert Comment

by:Jaime Olivares
ID: 20402158
sorry, it should be:
                PaymentPeriod = ('A' + (char)(i+1)).ToString();
0
 
LVL 7

Expert Comment

by:bungHoc
ID: 20402163
Okay.. Jaime beat me to it already.
0
 
LVL 55

Expert Comment

by:Jaime Olivares
ID: 20402196
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

0
 
LVL 2

Accepted Solution

by:
duckets earned 2000 total points
ID: 20402301

I would argue that it is fine to leave it as an 'if' statement.

Yes everyone knows, as a broad and general rule, long 'if' statements are a Bad Thing - but only when they could be better implemented in another way. I would say this particular case is an exception to the rule, as the contents of each 'if' or 'elseif' clause are extremely simple, and therefore very readable.

The purpose and function of the original code is understandable at a quick glance, whereas converting it to an array and looping through is much more complex, less readable, and probably less efficient for the computer to execute. Converting it to a loop with an array is just refactoring for refactoring's sake.

As a side note, if the dates are listed in chronological order, you can safely remove the first of each pair of comparisons in each condition. You can also remove the 'null' statement because (with the fix to the first line) it will never be reached. This will leave you with the following neat, simple code.

if (PayReceived <= BikeRecieved)
{
	PaymentPeriod = "A";
}
else if (PayReceived <= firstDate)
{
	PaymentPeriod = "B";
}
else if (PayReceived <= secondDate)
{
	PaymentPeriod = "C";
}
else if (PayReceived <= thirdDate)
{
	PaymentPeriod = "D";
}
else if (PayReceived <= fourthDate)
{
	PaymentPeriod = "E";
}
else if (PayReceived <= fifthDate)
{
	PaymentPeriod = "F";
}
else if (PayReceived <= sixthDate)
{
	PaymentPeriod = "G";
}
else 
{
	PaymentPeriod = "H";
}

Open in new window

0
 
LVL 2

Expert Comment

by:duckets
ID: 20402317
Heh, BTW, I also noticed that although you spelled ;Received' correctly in 'PayReceived', you spelled it incorrectly in 'BikeRecieved'! :-)
0
 
LVL 7

Expert Comment

by:bungHoc
ID: 20402415
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.
0
 
LVL 55

Expert Comment

by:Jaime Olivares
ID: 20402489
>>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
0
 
LVL 2

Expert Comment

by:duckets
ID: 20402542
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.
0
 
LVL 55

Expert Comment

by:Jaime Olivares
ID: 20402561
>>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?
0
 
LVL 2

Expert Comment

by:duckets
ID: 20402598
Very few. But still - it is less efficient and less readable than stellyuk's original code.
0
 
LVL 55

Expert Comment

by:Jaime Olivares
ID: 20402614
>>But still - it is less efficient and less readable than stellyuk's original code.
Thanks for sharing your opinion.
0
 

Author Comment

by:stellyuk
ID: 20403279
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

0
 
LVL 55

Expert Comment

by:Jaime Olivares
ID: 20403352
the DateTime.Compare() is unnecessary also a single comparison is enough in each if () statement.
0

Featured Post

Nothing ever in the clear!

This technical paper will help you implement VMware’s VM encryption as well as implement Veeam encryption which together will achieve the nothing ever in the clear goal. If a bad guy steals VMs, backups or traffic they get nothing.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Wouldn’t it be nice if you could test whether an element is contained in an array by using a Contains method just like the one available on List objects? Wouldn’t it be good if you could write code like this? (CODE) In .NET 3.5, this is possible…
Today I had a very interesting conundrum that had to get solved quickly. Needless to say, it wasn't resolved quickly because when we needed it we were very rushed, but as soon as the conference call was over and I took a step back I saw the correct …
Want to learn how to record your desktop screen without having to use an outside camera. Click on this video and learn how to use the cool google extension called "Screencastify"! Step 1: Open a new google tab Step 2: Go to the left hand upper corn…
This lesson discusses how to use a Mainform + Subforms in Microsoft Access to find and enter data for payments on orders. The sample data comes from a custom shop that builds and sells movable storage structures that are delivered to your property. …

610 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question