Search code examples
jsfrichfacesfacelets

Refactor a link and an image


I have to write an link with an image inside. Instead of explaining, here's the code I have now:

<c:if test="${userSession.loggedUser eq null and company.image != null}">
    <a onclick="${rich:component('loginPanel')}.show()">
        <img src="/download.do?hash=#{company.image.hash}" />
    </a>
</c:if>
<c:if test="${userSession.loggedUser eq null and company.image == null}">
    <a onclick="${rich:component('loginPanel')}.show()">
        <img src="${request.contextPath}/img/icons/logo_default.jpg" />
    </a>
</c:if>
<c:if test="${userSession.loggedUser ne null and company.image != null}">
    <a href="company.xhtml?${company.name}">
        <img src="/download.do?hash=#{company.image.hash}" />
    </a>
</c:if>
<c:if test="#{userSession.loggedUser ne null and company.image == null}">
    <a href="company.xhtml?${company.name}">
        <img src="${request.contextPath}/img/icons/logo_default.jpg" />
    </a>
</c:if>

This code looks awful - there are two exact links with two exact images but combined in all possible combinations. Is there a better way? Is there a way to avoid c:if - it created tables?

Update: Bozho proposes: You can replace <c:if and <a with <h:outputLink rendered="#{..}". Apart from that I don't see any other optimization.

But it doesn't work. This does not render correctly:

<a href=>
<h:outputLink rendered="#{..}
<h:outputLink rendered="#{..}
</a>

(the image is outside the anchor)

This does render fine:

<h:outputLink value=>
<h:outputLink rendered="#{..}
<h:outputLink rendered="#{..}
</a>

, but it always adds href and in two of the cases I don't want href when rendered.

Update2: I also had them combined 2 by 2:

<c:if>
  <a href...>
    <c:if><img...></c:if>
    <c:if><img...></c:if>
  </a>
</c:if>
<c:if>
  <a onclick...>
    <c:if><img...></c:if>
    <c:if><img...></c:if>
  </a>
</c:if>

This also does not render correctly - the <a does not surround the images for some crazy reason.


Solution

  • How about this?

    <c:set var="isLoggedIn" value="#{userSession.loggedUser != null}" />
    <c:set var="showPage" value="company.xhtml?#{company.name}" />
    <c:set var="showLogin" value="javascript:${rich:component('loginPanel')}.show()" />
    <c:set var="hasImage" value="#{company.image != null}" />
    <c:set var="companyImage" value="/download.do?hash=#{company.image.hash}" />
    <c:set var="defaultImage" value="#{request.contextPath}/img/icons/logo_default.jpg" />
    
    <a href="#{isLoggedIn ? showPage : showLogin}">
        <img src="#{hasImage ? companyImage : defaultImage}" />
    </a>
    

    This can be shortened as follows (will only be a bit nastier to read/interpret quickly):

    <c:set var="showPage" value="company.xhtml?#{company.name}" />
    <c:set var="companyImage" value="/download.do?hash=#{company.image.hash}" />
    <c:set var="defaultImage" value="#{request.contextPath}/img/icons/logo_default.jpg" />
    
    <a href="#{userSession.loggedUser != null ? showPage : 'javascript:${rich:component(\'loginPanel\')}.show()'}">
        <img src="#{company.image != null ? companyImage : defaultImage}" />
    </a>