-2

I have my model class with constructor that can throw exceptions:

public class BookModel {

private Book book;
String authorName;

public BookModel(Book book) {
    this.book = book;
    try {
        this.authorName = (AuthorLocalServiceUtil.getAuthor(book.getAuthorId())).getAuthorName();
    } catch (PortalException | SystemException e) {
        e.printStackTrace();
    }
}

After that, I create new object here:

for (Book book: bookList) {
        books.add(new BookModel(book));
}

So my question is, should I catch my exception in constructor or in the moment of object's initialization? Im very newbie so I really need your help.

Al.Boldyrev
  • 486
  • 7
  • 30
  • I don't know if this is the right place for the exception. It really depends on what you're going to do with the exception, but I personally would catch it one step higher when creating the object and not in the constructor. – Sam Orozco Sep 27 '16 at 18:52
  • One more example of the damage checked exceptions do to code. And they were supposed to _improve_ it. – Marko Topolnik Sep 27 '16 at 19:05
  • You are piercing the layer boundaries here. Model should not actively access the service layer, it should be a passive component that gets the data pushed into it. – Marko Topolnik Sep 27 '16 at 19:06
  • @MarkoTopolnik so what should I do? – Al.Boldyrev Sep 27 '16 at 19:59

2 Answers2

1

The BookModel constructor should throw an exception. "Swallowing" the exception by logging it in the constructor and continuing as if nothing is wrong is likely to cause bugs down the line.

If you think it is unlikely that these exceptions will occur, or you think that it's unlikely that a downstream-caller will be able to handle the exceptions, you can wrap them in a RuntimeException so you don't have to advertise them in the constructor's signature.

public BookModel(Book book) {
    this.book = book;
    try {
        this.authorName = (AuthorLocalServiceUtil.getAuthor(book.getAuthorId())).getAuthorName();
    } catch (PortalException | SystemException e) {
        throw new RuntimeException(e);
    }
}
Andrew Rueckert
  • 4,858
  • 1
  • 33
  • 44
1

In your special case, the exception handling should imo not be part of the constructor of BookModel. I assume that you want to create a BookModelinstance with a valid Book object and the author name. So I would modify the constructor to something like

public BookModel(final Book book, final String authorName) {

For each of the Book objects you are trying to retrieve the authors name. If there is a problem with the retrieval, the catch-block is entered. So I would try it like this:

for (final Book book: bookList) {
  String authorName =null;
  try {
     authorName = (AuthorLocalServiceUtil.getAuthor(book.getAuthorId())).getAuthorName();

  } catch (YourException exception){
     ...logging, exception handling continue in loop,...
  }
  BookModel model = new BookModel(book, authorName);
  books.add(model);
}
Stefan Freitag
  • 3,578
  • 3
  • 26
  • 33