1

I'm really new to Java so bear with me here. I have a class that essential gets the parent of a page and returns it. When I navigate to the very top of a node I get a null pointer exception which is the line that reads for(Page page = to........ I understand why because at the top node there is no parent. How do I prevent my code from generating an error and gracefully displays a message if a user does navigate to the top level node.

Class Code:

public class Pages {
public static List<Page> getPath(Page from, Page to) {
    if (from == null || to == null) throw new IllegalArgumentException();

    List<Page> path = new ArrayList<Page>();
    for (Page page = to.getParent(), last = from.getParent(); page != null && !(page.getPath().equals(last.getPath())); page = page.getParent())
        path.add(page);
    Collections.reverse(path);
    return path.contains(from) ? path : null;
}

}

JSP Code:

Page rootPage = resourceResolver.adaptTo(PageManager.class).getPage(properties.get("rootNode",Page)currentPage).getPath()));
List<Page> listPages = Pages.getPath(rootPage, currentPage);

for (Page showContent : listPages) {
%>
<li><a href="#">listPages.getDisplayTitle(showContent)) %></a></li>
<%
} //end page for loop
Delmon Young
  • 2,013
  • 5
  • 39
  • 51

4 Answers4

1

Since you requested an example:

public class Pages {
public static List<Page> getPath(Page from, Page to) {
    if (from == null || to == null) throw new IllegalArgumentException();

    List<Page> path = new ArrayList<Page>();
    Page page=to.getParent();
    Page last=from.getParent();
    // I'm assuming getPath() can be null occassionaly and is a String
    String lastPath;
    if(last!=null && (lastPath=last.getPath())!=null){
    // The assignment above is an acceptable one, as it saves a nested if statement
        // traverse your path
        while(page!=null && page.getPath()!=null && !(page.getPath().equals(lastPath))) {
            path.add(page);
            page=page.getParent();
        }
    }
    Collections.reverse(path);
    return path.contains(from) ? path : null;
}

It is worth noting that this piece of code can still break your page if you enter "null" for your "from" or "to" argument since you are throwing a new IllegalArgumentException in that case. I prefer to handle as much of the logic and error handling in the actual java backend and as little as possible on the jsp frontend. A possible improvement could be to have return just an empty list if there are invalid params.

public class Pages {
public static List<Page> getPath(Page from, Page to) {
if (from == null || to == null) return new ArrayList<Page>();

List<Page> path = new ArrayList<Page>();
Page page=to.getParent();
Page last=from.getParent();
// I'm assuming getPath() can be null occassionaly and is a String
String lastPath;
if(last!=null && (lastPath=last.getPath())!=null){
// The assignment above is an acceptable one, as it saves a nested if statement
    // traverse your path
    while(page!=null && page.getPath()!=null && !(page.getPath().equals(lastPath))){
        path.add(page);
        page=page.getParent();
    }
}
Collections.reverse(path);
return path;//return path or empty list
}

With this improvement, you shouldn't get any execeptions anymore. In the frontend you can now just check if your list is empty or not and act accordingly

*Side note 1:* I'm not really sure under what circumstances you'd want to add the "from" to the path (so I left it out), so this is something you might need to add to the provided code.

*Side note 2:* You might want to consider EL instead of scriptlets, just a thought :)

3xil3
  • 1,546
  • 1
  • 12
  • 16
0

You need to employ a try... catch block which can catch the error when at the top node and then pass it to the user display. Read up the documentation for try... catch :)

banjoSas
  • 184
  • 1
  • 10
0

Add an if statement inside getParent method definition with condition that if the Node your getParent-ing is the top node, throw new exception.

If you are a beginner I believe this instead of 'try-catch' would suffice.

Eric
  • 2,635
  • 6
  • 26
  • 66
  • thanks @user25409 I'm kind of confused here. Would it be possible to show me a sample or modify my code. Complete newb here so a bit confused. – Delmon Young Apr 12 '13 at 22:37
  • Can you add getParent method definition and what variable or expression points to the top Node? – Eric Apr 12 '13 at 22:40
  • It's not good practise to assign this many variables inside your for condition. Besides, you are kinda missing the point of a for. A while statement is a better choice in this case. And I'd really recommend to add an if check for the "top node" case so you can avoid the Nullpointer instead of expecting it to happen in some specific cases. – 3xil3 Apr 12 '13 at 22:48
  • @user25409 The top node is generated by user input, so they input a specific path into field. the line that does this is **List listPages = Pages.getPath(rootPage, currentPage);** how would I do a null check against that? – Delmon Young Apr 12 '13 at 22:55
  • thanks @3xil3 I'm really new to this could you show me a sample of the best way to do this. Thanks! – Delmon Young Apr 12 '13 at 22:56
  • By the way, inside the for loop, where are you declaring the variable last? Was it okay without declaring its type? like: for (Page page = to.getParent(), Page last = from.getParent(); ? – Eric Apr 12 '13 at 22:58
  • It is likely that this is the reason you are getting the nullpointerexception – Eric Apr 12 '13 at 23:03
0

You need to check for that condition inside the loop, like this:

public static List<Page> getPath(Page from, Page to) {
  ...
  for (Page page = to.getParent(), .. page = page.getParent()) {
    path.add(page);

    // Check for topmost page with no parent.
    if (page.getParent() == null) {
      break;
    }
  }
  ...
  return path.contains(from) ? path : null;
}
Cebence
  • 2,406
  • 2
  • 19
  • 20