Generic code or code easy to understand?

Question

At work now I had an argument with co-worker, because I made a page that he feels is too generic.

The page has 3 modes (simple, adv and special) - it has to work like this, because we don't choose how the specification is written. in each mode the page looks different(the changes are not big - it shows/hides 5 text fields in different combinations based on the mode).

My co-worker think it should be 3 pages and when you change something you should just merge the changes to other two modes.

The fields now has code like rendered="#{mode!=2}", etc.

P.S. Now the difference is 5 fields, but in the future no1 know how much it will be changed.

---

We use Seam(JSF/Facelets), here is pseudo facelet code(to make it easier to understand). I did not put it in panelGroups to better present the problem.

<h:output rendered="mode=2" value="Name: " />
<h:input rendered="mode=2" value="bean.name" />
<h:output rendered="mode=2" value="Surename: " />
<h:input rendered="mode=2" value="bean.surname"  />

<h:output rendered="mode!=2" value="Surename: " />
<h:input rendered="mode!=2" value="bean.surname"  />
<h:output rendered="mode!=2" value="#{mode==1?'My Name':'My friends name'}" />
<h:input rendered="mode!=2" value="bean.name" />

---

I duplicated version it would look like this(pseudo code)

<c:if test=mode=1>
        <ui:include view=modeSimple.xhtml>
</c:if>
<c:if test=mode=2>
        <ui:include view=modeAdv.xhtml>
</c:if>
<c:if test=mode=3>
        <ui:include view=modeSpec.xhtml>
</c:if>


modeSimple.xhtml
    <h:output value="Surename: " />
    <h:input value="bean.surname"  />
    <h:output value="My Name" />
    <h:input value="bean.name" />
modeAdv.xhtml
    <h:output value="Name: " />
    <h:input value="bean.name" />
    <h:output value="Surename: " />
    <h:input value="bean.surname"  />
modeSpec.xhtml
    <h:output value="Surename: " />
    <h:input value="bean.surname"  />
    <h:output value="My friends name" />
    <h:input value="bean.name" />

Answer 1

You should structure your templates using the same rules as when programming. This essentially means extracting common design elements into separate files to avoid duplication, and thus achieve more reusable design. That is rule #1 (DRY) and the method is called refactoring in programming terms.

The second rule is that you should strive for simplicity and clarity. It should be easy to understand the layout and where to go when to you need to change things.

Following the first rule to the letter leads to heavy decomposition of your gui into lots and lots of small snippets which can make it hard to understand how the layout is created. You have to hunt around a lot to find where things are. This violates the second rule, so you need to find a balance between these two opposites.

But the best approach is to find a solution that avoids this issue completely. I think, in this case, you should consider a simpler approach altogether. I assume this is HTML and you mention "simple and advanced" states in the GUI. Have you considered always generating all fields and then handle the GUI logic in JavaScript? In other words, write client-side code which hides and shows the relevant fields on the fly depending on which mode (state) it is in?

This also has the benefit of being able to switch mode on demand without involving the server and you don't have to compromise any of the rules. Another great thing is that you can let the front-end guys do these changes without having to involve the back-end programmers who usually are not that overly keen on spending time tweaking and polishing the design and interaction.

Answer 2

Without seeing the code, I can only speculate, but I would guess that at the moment your code is "on the edge".

For 2 or 3 modes with a limited number of changes between those modes what you have now is probably OK. However, were it to be any more complicated then it sounds like it should be refactored into separate pages.

Personally I prefer easy to understand code - it's much more maintainable, both for other developers and yourself when you have to pick it up again 6 months down the line.

However, there's no sense in refactoring the code now to solve a problem which might not exist in the future. You say you don't know how much the requirements will change - and that includes no change - so applying YAGNI (You Ain't Gonna Need It) don't do anything now.

Mark the code as requiring attention in the future so you don't forget it, but don't change (and potentially break) it now.

Answer 3

I've done pretty much the same thing. Believe me, after implementing few different behaviours for each mode, the "main page" becomes a mess nearly impossible to read. You would then see only a bunch of control flow blocks (shortly followed by Matrix digital rain). I found myself removing some blocks just to get a clear view of what's inside a certain mode.

So the "separate pages" is the way to go. However when you do that, you have to "fight" code duplication problem. This is where your collegue might be wrong by suggesting merges. Sadly so I've done that also. Basically it works, but it consumes a lot of precious time and eventually by poor merge "common area" of the pages is out of sync and merging becomes a real nightmare. So you are right in objecting a merge as a reasonable solution.

As you can see from many responses, the correct approach is to extract truly "common" parts to external entities (JSP page fragments, JSF snippets, whatever) and include them in those pages.

Answer 4

Duplication of code is practically never a good thing. That said, a small amount duplicated not many more than twice is acceptable in real life - it is better to be pragmatic than religious about it. In your case, it sounds like a larger amount of code and 3 places, so it's slightly over my personal threshold. Thus I lean towards the generic version. And at any rate, if it works now, there is no need to touch it just for theoretical reasons.

OTOH when you say that there can be more differences between the pages in the future, this might mean that at some point in time it becomes more economic to switch to separate pages (while striving to minimize duplication between them by extracting commonalities as much as possible). So reevaluate the situation before each future change.

Answer 5

This looks like JSF. Each rendered="..." essentially means you have an if in your code, which cause additional complexity.

I've done the same thing because "it is the same page just with a few differences here and there" and put very simply, it doesn't work in the long run because it isn't the same page and you will need to do more and more elaborate tricks to make it work properly causing it to be needlessly hard to maintain.

If you use JSF with facelets instead of JSP (which you should if you could) then you can build XML-fragments corresponding to a block of the page, which you can then include where needed. This will buy you modularity and still allow you to reuse content across pages.

Listen to your co-worker. He is right.

Answer 6

Is there a chance to generate the page using code? In this case, how about something like:

page1 = {
   showField1 = true;
   showField2 = false;
   showField3 = true;
  }
page2 = {
   showField1 = true;
   showField2 = false;
   showField3 = false;
  }

And in the page rendering code

if page.showField1 then ....
if page.showField2 then ....

Or in OOP-style:

class Page1 {
   renderField1() { ...}
   renderField2() {}
   renderField2() { ...}
}
class Page2 {
   renderField1() { ...}
   renderField2() {}
   renderField2() {}
}

Answer 7

modal code can only get worse. Classify and decide once at the start, and branch to clean implementations without modes.

Answer 8

The essence of this question seems to be: when you have three very similar web pages with a few minor differences, is it better to duplicate those three pages and modify each as needed, or build into a single page the logic to dynamically render the page according to which "mode" is being viewed.

For the record, I am a JSF guy and I like it, and I make use of conditional rendering.

If you opt to have three pages, there is risk that as changes happen the maintainer may not apply that correctly to all three pages.

If you opt for using conditional rendering, you solve that problem but you have moved some business logic into your page which might need some thought.

I personally would have no problem with the use of conditional rendering, but would really like to see the mode logic moved to a backing bean, e.g: instead of:

<h:output rendered="mode=2" value="Name: " />

I like:

<h:output rendered="#{myBean.advancedMode}" value="Name: " />

Where the bean handles the logic and returns true or false to control the rendering of the element.

The reason is the mode being viewed may need to control more than just this one page, may need to persist it beyond even the user's single session, or you may need to change the modes completely down the road.

Or, you can accept that either approach has potential downsides, and all the downsides seem to be regarding easing pains down the road, and agree to worry about it further when one of those "down the road" things actually come up, and then make a better decisions now that more is actually known about how the requirements might change.