6

I'm a newbie in java. I'm writing a class where the constructor must check the price parameter and ensure it is not a negative number. And if it is negative, it must set the price to zero. I get a stackoverflow error when I check price. Can i get help with what I did wrong?

public class Book
{
    private String title;
    private String author;
    private String isbn;
    private int pages;
    private boolean pback;
    private double price;

    /**
     * Constructor for objects of class Book
     */
    public Book(String bookTitle, String bookAuthor, String bookCode, int bookPages, boolean paperback, double bookRetail)
    {
        title = bookTitle;
        author = bookAuthor;
        isbn = bookCode;
        pages = bookPages;
        pback = paperback;
        price = bookRetail;
    }

    /**
     * @returns title
     */

    public String gettitle()
    {
        return title;
    }

   /**
    * @returns author
    */

    public String getauthor()
    {
        return author;
    }

   /**
    * @returns ISBN#
    */

   public String getisbn()
    {
        return isbn; 
    }

   /**
    * @return number of pages
    */ 

   public int getpages()
    {
        return pages;
    }

   /**
    * @return is book paperback
    */ 

   public boolean getpback()
    {
        return pback;
    }

   /**
    * @return retail price
    */ 

   public double getprice() 
   {
       if(getprice() < 0)
       {
           return 0;
       } 
       else
       {
           return price;
       }

    }
}
Improve this question
3
  • 4
    +1 for self-referential stackoverflow on stackoverflow! Commented Feb 6, 2010 at 4:25
  • now some might realise what the site's name means... Commented Feb 6, 2010 at 12:05
  • So meta! I wonder whether he got here by googling for "stackoverflow". :-) Commented Feb 23, 2010 at 10:28

6 Answers 6

Reset to default
14

Your getprice() method calls itself instead of checking price. This is leading to an infinite recursion in this case.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

+1, and to comment on Ignacio's answer: modern IDEs will notice the infinite recursion and warn you about it in real time (I use IntelliJ and it warns you about such errors, I'm sure other IDEs do the same).
you could also rewrite it like this: if(price < 0 ) { return 0; } return price; The else isn't needed.
If you wanted to get REALLY picky you could write it as return Math.max(price, 0);.
@WizardOfOdds: Findbugs is great for this too, and it has plugins for several of the big IDEs.
1

Ignacio has explained the cause, and the solution:

Change the line

if(getprice() < 0)

to this:

if(price < 0)
Improve this answer

Comments

1

Your getting infinite recursion, because your if condition checks your getprice() method, not your price variable.

Many modern compilers will warn you when you have coded something that results in infinite recursion.

I still sometimes come across this error too, especially with IDE's that have intellisense.

Good luck learning Java! :)

Improve this answer

Comments

1

When you write a bean you generally want to check if the price being set is < 0, instead of making this calculation every time you try to get the variable.

Improve this answer

1 Comment

A newbie might have difficulties in understanding the 'bean' concept ;)
1

Not a cure for the recursion problem, but you should also consider checking the price at construction time.
Sometimes (most times?) it is better that your constructor fails with an Exception instead of allowing the construction of an inconsistent object. This way it is easier to localize such an Error.
Example: 

public Book(String bookTitle, String bookAuthor, String bookCode, int bookPages, boolean paperback, double bookRetail)
{
    if (bookRetail < 0.0)
        throw new IllegalArgumentException("negative bookRetail: " + bookRetail);
    ...
}

The risk is that your application can fail when in the production environment, which can be a mess. To avoid this, you can use an assert or, at least, give out or log the error and use some alternative. The assert check must be turned on for development and may be turned off at production. For Detail see Programming With Assertions 

public Book(String bookTitle, String bookAuthor, String bookCode, int bookPages, boolean paperback, double bookRetail)
{
    assert bookRetail >= 0.0 : bookRetail;
    ...
}

or 

public Book(String bookTitle, String bookAuthor, String bookCode, int bookPages, boolean paperback, double bookRetail)
{
    if (bookRetail >= 0.0) {
        price = bookRetail;
    } else {
        price = 0.0;
        // display or log the "illegal argument"
        Exception ex = new IllegalArgumentException("negative bookRetail: " + bookRetail);
        ex.printStackTrace();
    }
    ...
}
Improve this answer

6 Comments

Just make sure that when you are implementing an IDisposable/Finalizer pattern, it can handle a partially constructed object.
@TToni; why partially constructed object? I am just considering to check the value at construction time instead of when accessing the field. The object will be fully constructed or there will be no object at all (in case of throwing an Exception).
Imagine for example an object which opens two filehandles in its constructor. An exception in the constructor might leave none, one or two files open. So if an constructor-exception occurs, the runtime calls your finalizer (if you have one) which has to deal with this situation.
@TToni: Imagine? filehandles (in Java)? The question is about a simple object (a Book) just holding some values. Anyway that's the reason to first check the parameters before doing anything else (...). And, in your example, it would be much better/safer to close any eventually opened file when throwing an Exception.
Sure, it doesn't apply to the simple sample. That's why I restricted my original comment to classes that implement IDisposable/Finalizer. If you throw an exception in a constructor, the runtime will call your finalizer (at least in C#), so there you have to deal with potentially unfinished contructors. And do you know out of hand what happens with the virtual constructor chain if one of these throws an exception? I would have to look it up. So all I'm saying is: Be careful, especially if your class handles unmanaged ressources.
|
0

Your getprice should simply be written as : 

return price < 0 ? 0 : price;

Btw, nice to see that a stackoverflow error is solved by stackoverflow.com

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.