Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/471#issuecomment-71718907
  
    @michaeldye I agree that: if any of the Strings in `items` is null, it 
should throw an exception that includes the rest of the path in its message.
    
    I'd prefer a `NullPointerException` rather than an 
`IllegalArgumentException`. To quote Joshua Bloch's Effective Java item 60: "If 
a caller passes null in some parameter for which null values are prohibited, 
convention dictates that NullPointerException be thrown rather than 
IllegalArgumentException."
    
    You can call the NPE constructor that takes a String, so can still pass in 
the message.
    
    If you can update the PR accordingly (and ideally add a test case to 
`brooklyn.util.net.UrlsTest`), then that would be fantastic.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to