Campbell Ritchie wrote:Are you using Date and Calendar still? Move onto the newer date classes.
You appear to be adding 0 to the months. Or have I read the code wrongly? Yes, I have got it wrong. You have an incorrect if because you can never get false out of it (line 6). You cannot tell from here what IDP is; it might for all we know be negative. And you have a date format which goes out of scope before you use it.
All things are lawful, but not all things are profitable.
Liutauras Vilda wrote:A bit off topic... Sorry for being picky.
1. Your method doing more things than method's name suggests it's suppose to do.
2. Variable "monthadd" should be named at least "monthAdd", and it is still sounds confusing what it adds.
3. Variables as "IDP", "IPV_DISQL_PERIOD" expected to be constants, is it true? In Java only constants suppose to be named in all upper cases.
4. Variable "cal" would clearer as "calendar", as "cal" could mean calculator also - ambiguous. "date1" is meaningless, the same as "cal", so when you compare if (date1 < cal) is not clear what actually is compared. Current date with something or...?
5. "IPVValidCheck" - would expect this as boolean, not String, probably because of not clear naming.
6. Lines 15, 16, 17 - badly indented code.
7. It doesn't contain "return" statement.
Liutauras Vilda wrote:I'm looking at this code without any understanding what it is meant to do. John, I think you should clear it up for yourself too.
1. What is the single purpose of this method? I noticed that method name changed from your previous code. And other problems too:
a) Variables calendar, date1, sdf are not used.
b) Still no return statement.
c) Line 8 using variable "IPV_DISQL_PERIOD", should be "idp" it seems.
d) Numbers: 97, 98, 99 are magic numbers without any meanings.
e) You have montAdd = idp = IPV_DISQL_PERIOD, you could omit 1 unnecessary declaration/initialisation.
f) Probably would work all this better in "switch" statement rather than multiple if's, else if's.
But I'd suggest at first clearly define what singular task your method suppose to achieve and start over without fixing current issues, there are a lot of them.
Great, so, get this method done first, to accomplish your stated task only.John Morgan wrote:1) The singular purpose is to take a date that is provide to me from a database, add a certain length of time and see of that date is before todays date.
Another method to create.John Morgan wrote:d) Regardless of the value of idp it will return IPVValidCheck string.
The only error that I get at the moment is "The operator < is undefined for the argument type(s) java.util.Calendar, java.util.Calendar" where I have (today < ipvEndDate)
All things are lawful, but not all things are profitable.