brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aledsage <...@git.apache.org>
Subject [GitHub] incubator-brooklyn pull request: fixed BROOKLYN-129: brooklyn.util...
Date Tue, 27 Jan 2015 20:11:22 GMT
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message