PDA

View Full Version : Intro java programming.. need a check on my logic (else/if statements)




Whorehay
Feb 26, 2009, 11:15 PM
Hey. I'm taking an intro to java programming course, and it's pretty interesting. I've run into something weird. I just need a check if my logic is okay.

So first I state that "if day1 is greater than 1 and less than 17th, then print out January day1 is Winter Recess!"

Then when I say "else if day1 is less than 31, then January day1 is not a school holiday," is my code a logical error? I ask because the numbers in the first statement are also less than 31.

The output comes out correctly, so I'm just wondering.

output:
"January 17 is Winter Recess!
BUILD SUCCESSFUL (total time: 6 seconds)"

switch ( month1 ) {
case 1:
if ( (day1 >= 1) && (day1 <= 17) )
System.out.println( "January " + (day1) +
" is Winter Recess!");
else if (day1 == 19)
System.out.println( "January " + (day1) +
" is Martin Luther King's Birthday." );
else if (day1 <= 31)
System.out.println( "January " + (day1) +
" is not a school holiday." );
else
System.out.println( "You entered an invalid date. " +
"Please try again.");



yg17
Feb 26, 2009, 11:22 PM
No, logically it's fine because when Java runs through an if/else statement, once it matches a condition, it runs that, then jumps out of the if/else statement. So, if day1 is 5, it matches the first condition (day1 >= 1 && day1 <= 17), it'll run that, and even though it still matches the day1 <= 31 condition, it's never run because after running the first condition, it goes to whatever's below the if/else statement.

Also, on a sidenote, for readability's sake, I strongly recommend using curly brackets { } around your if blocks, even if there's only one line of code and you can get away without them. It makes it easier to read, and if you go back and expand the block to more than one line of code and forget to add those curly brackets, it's not going to work as expected.

Whorehay
Feb 26, 2009, 11:38 PM
Hey, thanks for the explanation. It made sense intuitively in my head, but I wasn't sure. Haha.

Also, thanks for the tip. Do you mean brackets around just the if statement or all the if/else if/else statements?

if {
}

or if {
else if
else
}

or something else?

yg17
Feb 26, 2009, 11:41 PM
No problem. Brackets around each block:


if (blah == blah) {
do stuff
} else if (asdf == asdf) {
do other stuff
} else {
do something else
}

Gelfin
Feb 26, 2009, 11:47 PM
No problem. Brackets around each block:

Watch, I'm totally going to start a religious war:

Do this:if
{
}
not this:if {
}

:D

Whorehay
Feb 27, 2009, 12:02 AM
All done :) just went back and modified everything. Thanks for the help!

Whorehay
Feb 27, 2009, 12:03 AM
Watch, I'm totally going to start a religious war:

Do this:if
{
}
not this:if {
}

:D

I'll do that for the next assignment :)

Guiyon
Feb 27, 2009, 12:51 AM
Watch, I'm totally going to start a religious war:

Do this:if
{
}not this:if {
}


This is an affront to The One True Brace Style! Go peddle your filth elsewhere.

Zortrium
Feb 27, 2009, 03:34 PM
Watch, I'm totally going to start a religious war:

Do this:if
{
}
not this:if {
}

:D

Seconded. Also, use nested ternary operators wherever possible to cut down on the number of lines of code.

Okay, maybe not the second idea. But definitely the first.

Whorehay
Feb 27, 2009, 09:20 PM
Seconded. Also, use nested ternary operators wherever possible to cut down on the number of lines of code.

Okay, maybe not the second idea. But definitely the first.

It's not due for another week, so I'm open to your thoughts on the nested operators? I've been trying to reason a way to cut it down (it's pretty long and repetitive), but I can't think of a way to put it all together. I have 12 of those switch statements each with their own else/if statements, which are all pretty repetitive.

The program is supposed to tell you if an entered day is a school holiday or not.

lee1210
Feb 27, 2009, 10:09 PM
Well... since this is a school project i don't feel like i should post most of what i wrote for this... but i'll try to give some hints:

1) Do all of your error handling first, so you can use default: or nothing at all to indicate that a date is not a holiday, instead of expressly handling an error case and a "not holiday" case. For example:

java.util.GregorianCalendar cal = new java.util.GregorianCalendar();
boolean isLeapYear = cal.isLeapYear(cal.YEAR);
int monthDays[] = {31,isLeapYear?29:28,31,30,31,30,31,31,30,31,30,31};
if(month < 1 || month > 12) {
System.out.println("Invalid month entered, try again.");
return;
} else if(monthDays[month-1] < day || day < 1) {
System.out.println("Month entered does not have the number of days entered, try again.");
return;
}


If you get past that bit of code (don't use that bit of code, please. It was meant for demonstration), you know you have a valid month and a valid day in that month.

2) It might be easiest to do a single switch for the months, then instead of nesting a switch inside of that, just use an if-then-else(if) block to check for a holiday. There aren't that many holidays in a month, so it should be pretty easy. For something like june, you might not even have to check the day.

3) Instead of doing all of that printing in your switch, if-then-else blocks, etc. you could just store if the day is a holiday or not, and if so, which. Then at the end you can print if it's a holiday and which. You could also leverage your switch to set the month string, or you could just make an array with month names that you index into. If you initialize(initialise? can't tell where you're from. Don't hear winter recess where I live, at least) these right, you don't even have to worry about doing anything if it's not a holiday.

4) Your logic might be a little verbose. The only recourse i'd see would be to have a list of lists of holiday days for each month, or a giant list of lists of booleans that details if each day of the year is a holiday, then you'd just index into that array with the month and day, and you'd know right away. That would require a lot of lines of literal true/falses, so that wouldn't ultimately save code. You'd also have to store some sort of data structure with the name of the holiday that would be easily accessible. I'd say something that allows more sparseness, so maybe a hashmap<java.lang.Integer,java.lang.String> whose key is month*32+day. I am not recommending this method, but it would allow all of the logic to be encoded in "fixed" data structures, so the code that's actually executing would look like:

java.lang.String holidayName = "not a holiday.";
if(isHoliday[month-1][day-1]) {
holidayName = holidayList.get(month*32+day);
}
System.out.println(monthNames[month-1] + " " + day + " is " + holidayName);


That looks nice and tidy, but there'd be a few hundred lines of initialization to get there. Even if you just made a hashset<Integer> of the days that were holidays, it would still take a lot of setup.

Good luck. Post here again as you run in to other problems.

-Lee

P.S. Braces belong on the line with the construct that warrants them! Death to the next-line heathens! How do you people write an else?

if(...)
{
...
}
else
{
...
}

Disgusting! And if it's not like that, it's inconsistent. How can you live with yourselves?

Gelfin
Feb 27, 2009, 10:35 PM
It's not due for another week, so I'm open to your thoughts on the nested operators? I've been trying to reason a way to cut it down (it's pretty long and repetitive), but I can't think of a way to put it all together. I have 12 of those switch statements each with their own else/if statements, which are all pretty repetitive.

The program is supposed to tell you if an entered day is a school holiday or not.

Ignore this if it is more advanced than where you are in your class, but in general it is a far better strategy to separate your code from your data.

In this case, your data includes the set of dates that are holidays.

All those conditionals are a consequence of the fact that your data is represented entirely in the structure of the code. An additional downside is that the solution is specific to a given year. If the holidays change next year, you have to modify the program.

If you were doing this for real, you would want information about holidays to be in a form you could load from a file, for instance. That's probably beyond the scope of the assignment, but once you've separated the data, you end up with a much smaller piece of code that tests the input date against each element in the holiday data in a loop.

How do you people write an else?

if(...)
{
...
}
else
{
...
}

Disgusting! And if it's not like that, it's inconsistent. How can you live with yourselves?

Exactly like that, and I suppose it is disgusting so long as you experience violent nausea at the prospect of being able to clearly see the delimiters of a block in a balanced and symmetrical way. It's just more readable. Whitespace is free, so don't be stingy, infidel dog. :p

Whorehay
Feb 28, 2009, 12:19 AM
... all the amazing information was here

I gotta say, this is my first programming course and it's only been 4 weeks since I've been introduced to programming (this is the 3rd weekly assignment-- we just learned about switch statements). So yeah, I have absolutely no idea what you just said. Haha. I'm going to have to reread it a couple of times and see if I can at least see the logic in it.

But thanks for posting! I do appreciate it. I'm trying to learn as much as possible so that's why I'm working ahead and asking questions. I'll refer back to what you said maybe in a couple of weeks!

lee1210
Feb 28, 2009, 12:39 AM
without knowing the level you're at, it was hard to say how to handle the problem. The top things I would do would be:
Error check first, so you don't need to do so in your "big switch". It will just be used to generate the possible holiday name.

What is the MINIMUM information you need to print? That's to say, if you just have to print the month and day, the word "is", then "not a holiday" or a holiday name, what do you need to keep track of? I would say just the last part of the sentence. So what would you need to store in the "big switch" to keep track of that?

You have a numerical month and need to print the name of the month. You could set the name in the "big switch" or use an array of month names and just access the month-1 element when printing the name.

Good luck. Don't worry if it doesn't all click yet, or if you feel your solution is suboptimal. Just work with what you know, and ask when you have questions.

-Lee

craig1410
Feb 28, 2009, 06:25 AM
Watch, I'm totally going to start a religious war:

Do this:if
{
}
not this:if {
}

:D

Well you asked for it...:p

I prefer setting out my if statements like this:

if (condition) {
// do stuff
} else {
// do other stuff
}


instead of:


if (condition)
{
// do stuff
}
else
{
// do other stuff
}


I also do this for "for" blocks and try-catch blocks and some others. I think this is much neater and easier to follow by eye when scanning over code. It's all subjective of course but this is the standard my employer (an IT services company) favours for all Java projects. Then again it was me who established the coding standard... :D

This is also the preferred coding standard according to Sun who invented Java. See here:

http://java.sun.com/docs/codeconv/html/CodeConventions.doc6.html#449


Cheers,
Craig.

Cromulent
Feb 28, 2009, 07:05 AM
Watch, I'm totally going to start a religious war:

Do this:if
{
}not this:if {
}:D

Exactly! Put the brackets on new lines so you can see clearly where blocks start and stop.

craig1410
Feb 28, 2009, 08:04 AM
Exactly! Put the brackets on new lines so you can see clearly where blocks start and stop.

I've never had any trouble finding the start and end of my blocks - that is why we use indentation...

Anyway, regardless of whether you use the Sun Java standard convention or whether you use some other style, the important thing is to be consistent within your team or organisation. In fact, if you move between organisations and/or teams you need to be prepared to adopt whatever standard is in use. This is made really easy using IDE's like Eclipse where you can define your standard and the IDE will maintain it for you. In fact, even the Eclipse standard built-in coding standard keeps the first brace on the same line as the if statement. Eclipse is one of the most popular Java IDE's so it looks increasingly that the "norm" is to use the Sun Java standard. Anyway, each to their own, back to the original topic of the thread.

Cheers,
Craig.

Cromulent
Feb 28, 2009, 09:57 AM
I've never had any trouble finding the start and end of my blocks - that is why we use indentation...

True but indentation does not fix everything. If we use an example previously posted:

if(month < 1 || month > 12) {
System.out.println("Invalid month entered, try again.");
return;
} else if(monthDays[month-1] < day || day < 1) {
System.out.println("Month entered does not have the number of days entered, try again.");
return;
}and then convert it to the brackets on new lines it instantly becomes much, much easier to read:

if(month < 1 || month > 12)
{
System.out.println("Invalid month entered, try again.");
return;
}
else if(monthDays[month-1] < day || day < 1)
{
System.out.println("Month entered does not have the number of days entered, try again.");
return;
}all the brackets line up nicely meaning you can see where one block starts and ends. It especially helps in nested if/else statements:

if(blah == blah2)
{
if(meh == meh2)
{
// do stuff
}
else if(meh2 == meh3)
{
// do stuff
}
}

I'll stop posting off topic stuff now :).

craig1410
Feb 28, 2009, 11:16 AM
True but indentation does not fix everything. If we use an example previously posted:

if(month < 1 || month > 12) {
System.out.println("Invalid month entered, try again.");
return;
} else if(monthDays[month-1] < day || day < 1) {
System.out.println("Month entered does not have the number of days entered, try again.");
return;
}and then convert it to the brackets on new lines it instantly becomes much, much easier to read:

if(month < 1 || month > 12)
{
System.out.println("Invalid month entered, try again.");
return;
}
else if(monthDays[month-1] < day || day < 1)
{
System.out.println("Month entered does not have the number of days entered, try again.");
return;
}all the brackets line up nicely meaning you can see where one block starts and ends. It especially helps in nested if/else statements:

if(blah == blah2)
{
if(meh == meh2)
{
// do stuff
}
else if(meh2 == meh3)
{
// do stuff
}
}

I'll stop posting off topic stuff now :).

I wasn't going to respond but I should make a minor correction to your examples above. From your first example I take it you meant:


if (month < 1 || month > 12) {
System.out.println("Invalid month entered, try again.");
return;
} else if (monthDays[month-1] < day || day < 1) {
System.out.println("Month entered does not have the number of days entered, try again.");
return;
}


This is just as easy to read as your non-standard formatting (IMHO of course :p) Btw, did you mean to use so many tabs/spaces in your second example or was this just for effect?

And an example of nesting would be:


if (blah == blah2) {
if (meh == meh2) {
// do stuff
} else if (meh2 == meh3) {
// do stuff
}
}


As you can see, each block is nested consistently and clearly and it has the advantage of being 7 lines long instead of 11 lines long without loss of readability. I like to keep my functions/methods to a small size so that I can see the whole function/method without scrolling around. Not always possible but adding 50% extra whitespace makes it that much harder to achieve.

There is no right and wrong as long as you are consistent but starting with the coding standard laid down by the organisation which invented the language can't be a bad place to start... The fact that Sun went to the trouble of creating a document on coding standards suggests that they have at least put some thought into it.

Over and out! :)