Github user sebfz1 commented on the pull request:

    https://github.com/apache/wicket/pull/143#issuecomment-161439689
  
    Hi Tobias,
    
    > Thank you for the time you spend to review the code!
    
    You are welcome, thanks a lot for your time too! :)
    
    > Any other things to be changed? 
    
    Well... yes ! :) You have removed the null check on the *model*  in 
`#buildSrcAttribute`, but there is still no null check on the model *object*. 
Sorry if I had not been clear enough.
    As I wrote previously, I see 2 options to handle this:
    
    1/ Check null *object*, and do not render the src tag. This will lead the 
image to not show at all (well, at least FF works like that)
    
    ```java
    protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    {
        if (srcModel.getObject() != null)
        {
                tag.put("src", srcModel.getObject().toString());
        }
    }
    ```
    
    2/ use `String#valueOf`. This will lead to the src tag to have null value 
and display a broken image, which can also make sense to alert without throwing 
a runtime NPE...
    
    ```java
    protected void buildSrcAttribute(final ComponentTag tag, IModel<?> srcModel)
    {
        tag.put("src", String.valueOf(srcModel.getObject()));
    }
    ```
    
    I have a slight preference for the 2nd choice. I guess you will opt for the 
first choice; so if everyone is fine with this first choice, that's fine for me 
too! (@svenmeier , @martin-g)
    
    Thanks again,
    Sebastien


---
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