• Post Reply Bookmark Topic Watch Topic
  • New Topic

Is it safe to ignore message " c"  RSS feed

 
Stijn Rensen
Ranch Hand
Posts: 48
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I just wrote a method to test if an entered date excists (ie determining that feb-30-2014 is false)

The return value is a boolean defined within the method. This boolean only records whether the date is correct (true) or not (false).

But as the boolean is not used to set other values I get the warning in eclipse: "The value of the local variable bestaat is not used"

Now I'm wondering if this is a warning I can always ignore.

Reason to know: the warning might distract me during debuging as it might serve as a false clue to the real issue.

Cheers,

Stijn
 
Darryl Burke
Bartender
Posts: 5167
11
Java Netbeans IDE Opera
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Well, if you're not using the variable then why declare it and assign it a value?

Without seeing code, preferably in the form of an SSCCE, that's all I can say.
 
Stijn Rensen
Ranch Hand
Posts: 48
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks for your reply Darryl.

But the fact is that I am using the boolean to store a statement. I seem to get the warning message because I did not use the boolean to set another variable.

Below the code with some dutch in it.

bestaatDatum = doesDateExcist
dag = day
maand = month
jaar = year
bestaat = excists
isSchrikkelJaar = a method that checks if Feb has 29 days in a given year

 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Your code has bugs.

Even though the message didn't really make sense in light of the true problem, you would be better off not ignoring this kind of warning message from Eclipse. At the very least, you can eliminate redundant code.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The reason Eclipse says that bestaat is not being used is because you're only ever assigning a value to it by using "=" instead of "==". This is why you should avoid doing return someBoolean == true because it's easy to make a mistake and use "=" instead. Plus, it's redundant. The buggy code becomes an assignment statement that resolves to the value being assigned. It will compile but it probably isn't what you wanted to do. Since your code consistently does the wrong thing, you end up not ever reading back the value you assigned to bestaat.

In this case, Eclipse wasn't being a nag like you thought it was; it was actually trying to be helpful but it just didn't go far enough to tell you what the real problem was.

The problem probably would have been caught by an automated style checker or static analyzer.

Edit #2: Logic-wise, the code is not actually "doing the wrong thing" as I said above. Because of the way you've written it, the code will still produce the desired results (except for the boolean operator precedence bug). Nonetheless, the code is redundant and using return someBoolean == true /* or false */ should be avoided. The Eclipse warning message will go away if you just do return false or return true instead of return bestaat=false and return bestaat=true, respectively, and then remove the redundant boolean bestaat=true; declaration/initialization.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You have another bug. Review how Java operator precedence works: && has higher precedence over ||.
Therefore: (a && b || c) is evaluated as ((a && b) || c), not (a && (b || c))
 
Stijn Rensen
Ranch Hand
Posts: 48
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks Junilu for your reply. Though It was hard to get introduced to the Harsch world of bugs like this ;).

I have altered the code based on your feedback:



But I wouldn't go as far as that the original had bugs. As the code performed as planned.

Based on the conditional statements ensures that if it doesn't meet any of the if statements the date is correct. And as soon as it meets the first statement it returns a value skipping the rest of the code.
(ie return true on correct dates and false on incorrect dates). (for the record ;) )

However you did teach me the following:
- My first code isn't the prefered way of coding
- I should use return only in one place (end of method)
- I should return a boolean like this: return bestaat;

Thanks for the learning experience
 
Jesper de Jong
Java Cowboy
Sheriff
Posts: 16060
88
Android IntelliJ IDE Java Scala Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stijn Rensen wrote:But I wouldn't go as far as that the original had bugs. As the code performed as planned.

Note that

ALWAYS returns true, regardless of what you passed to the method, or what the rest of the code in the method does. So, if your plan was to always return true, then it performed as planned, but I highly doubt that that was really what you wanted.

And even in the new version, have you thought about what will be returned if you call bestaatDatum(0, 10, 2014);? Is 0 October 2014 a valid date?
 
Stijn Rensen
Ranch Hand
Posts: 48
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
wow you are quick and good in your replies. Making my previous reply redundant at least. ;)

These replies are very educative for me.

By the way I understand the operator precedence. And the line



does what I want to do. ie: It checks if the date is 31 (or higher..) and the month is any of the months that has less than 31 days.

 
Jesper de Jong
Java Cowboy
Sheriff
Posts: 16060
88
Android IntelliJ IDE Java Scala Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stijn Rensen wrote:By the way I understand the operator precedence. And the line



does what I want to do. ie: It checks if the date is 31 (or higher..) and the month is any of the months that has less than 31 days.


Really? Have you tried bestaatDatum(1, 6, 2014); (1 June 2014)? Or any other valid date in June, September or November?

The line above means the same as:

But what you actually wanted is:
 
Stijn Rensen
Ranch Hand
Posts: 48
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks for the tip on the days (and months) smaller than 1. Updated on that.

I understand that return = true will always return true. And I understand the risk of using return this way.

However in this original (not so strong code) I placed the return statements in different spots. And as return also performs like a breaker for the rest of code it ignores all other return statements ones it reads out the first return statement. Ie when the first if statement is true it returns false and terminates the method.

by the way all versions of the code so far have returned false on the date Feb 30, 2014.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stijn Rensen wrote:Thanks Junilu for your reply.

You're welcome


But I wouldn't go as far as that the original had bugs. As the code performed as planned.

Your modified code still has the operator precedence bug. But you're right, it would have performed as desired and I noted that with an edit of my previous response.

Based on the conditional statements ensures that if it doesn't meet any of the if statements the date is correct. And as soon as it meets the first statement it returns a value skipping the rest of the code.
(ie return true on correct dates and false on incorrect dates). (for the record ;) )

However you did teach me the following:
- My first code isn't the prefered way of coding
- I should use return only in one place (end of method)
- I should return a boolean like this: return bestaat;

I actually don't mind returning from more than one place in a method if I'm using the coding pattern called "Introduce Guard Clause". This is essentially what you're doing with the "pre-checks". If you just return false in the guard clauses, then in the end just return true when all checks pass, you can eliminate the use of the temporary variable bestaat altogether.

The way you've written it now, you still evaluate all the remaining validity checks unnecessarily even if you've already found the date to be invalid. If you use an if-else-if-else construct instead, you will avoid doing unnecessary checks. I still would prefer just removing the redundant bestaat variable.

Thanks for the learning experience

Not a problem, that's why we spend time volunteering here.
 
Stijn Rensen
Ranch Hand
Posts: 48
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
.... :/ Poor testing on my part. Put in "random" dates as a test. ie dates of me and my partner and such. Were all dates in April...

Updated the code:

 
Stijn Rensen
Ranch Hand
Posts: 48
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Will go and google on "Introduce Guard Clause".

And again thanks for all the replies. as a complete noob (just finished chapter 3 of 9 of the book: Jave the Basics) I know I'm very suspect to getting into "bad" or "incorrect" habits.

Reading this replies really makes me aware of what I actually just coded.

Regards,

Stijn
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stijn Rensen wrote:
Updated the code:


You're still comparing a boolean variable with a boolean literal here: isSchrikkelJaar(jaar) == false

Prefer renaming the method and using this expression instead: !isSchrikkel(jaar) -- I'm assuming this translates to !isLeap(year)

My most preferred way would be to define two methods isLeap and isNotLeap and use isLeap(year) or isNotLeap(year) as appropriate where


Reason for edits: Even I get confused with isSchrikkelJaar(jaar) == false. It can get a bit non-intuitive: the expression false == false is true.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stijn Rensen wrote:by the way all versions of the code so far have returned false on the date Feb 30, 2014.

But your code is still buggy because I'm pretty sure it will say that Feb 30, 2016 and Feb 31, 2016 are valid. While 2016 is a leap year, Feb 30 and Feb 31 are still not valid dates.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stijn, if you want to learn a better way of testing your code, you should learn to use JUnit. With it you can write tests like:

@Test public void Feb_29_of_a_leap_year_should_be_valid() {...}
@Test public void Feb_30_of_a_leap_year_should_be_invalid() {...}
@Test public void Feb_31_of_a_leap_year_should_be_invalid() {...}
@Test public void should_be_invalid_if_long_month_and_day_is_greater_than_31() {...}
@Test public void should_be_invalid_if_not_long_month_and_day_is_greater_than_30() {...}
etc.

Automated tests like these help you document your code and think through your design and implementation. It may seem advanced for you right now but I don't think it's beyond a beginner's comprehension and I think it's within your capabilities to learn it. It will certainly benefit you in the long run. I also recommend looking into the practice of Test-Driven Development and Behavior-Driven Development once you're more familiar with Java. There's a promo going on right now for the "BDD in Action" book in the Design forum, if you're interested.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!