Sunday May 20, 2012

Tip #17 Don't Be Lazy #2

I was debugging some GlassFish code today.  This one Admin Command needed to find out if a given node was local or remote.  It is (obviously, clearly)  impossible for the node to change this attribute during this command call.  The code calls this method:

node.isLocal()

Now if you write a final getter method this is perfectly OK because the compiler will simply inline the code so that it becomes a simple variable access.

But NO WAY is that happening in this case.  The "node" object is a proxy. The value needs to get processed through code that resolves tokens, etc.  The stack shows 8 calls to get this value.  So it is an expensive call.

 I had a breakpoint in this method.  It was called over TWENTY times by the same command!  That's 152 or so pointless method calls.

What it needed was just ONE line of code:

boolean isLocal = node.isLocal();


Saturday May 19, 2012

Tip #16 Don't Be Lazy

Hmmm.  What's wrong with the code below?

I don't know if I've ever seen code screaming out louder for re-factoring.  Obviously there should be ONE method with the 4 lines that are repeated over and over and over and over.  What happens when one of the lines changes?  Well, then FOUR lines will have to change.  Of course if these copy&pasted lines are spread around in a huge file you'll probably miss a few. 

The only reason this can possibly happen is because copy&paste exists.  If we had to type in those identical blocks again and again -- THEN we would re-factor into a method call!

       if (!env.isDas()) {
            String msg = Strings.get("notAllowed");
            logger.warning(msg);
            report.setActionExitCode(ActionReport.ExitCode.FAILURE);
            report.setMessage(msg);
            return;
        }

        // Make sure Node is valid
        theNode = nodes.getNode(node);
        if (theNode == null) {
            String msg = Strings.get("noSuchNode", node);
            logger.warning(msg);
            report.setActionExitCode(ActionReport.ExitCode.FAILURE);
            report.setMessage(msg);
            return;
        }

        if (lbEnabled != null && clusterName == null) {
            String msg = Strings.get("lbenabledNotForStandaloneInstance");
            logger.warning(msg);
            report.setActionExitCode(ActionReport.ExitCode.FAILURE);
            report.setMessage(msg);
            return;
        }       

        if (demo != null) {
            String msg = Strings.get("demoOnly");
            logger.warning(msg);
            report.setActionExitCode(ActionReport.ExitCode.FAILURE);
            report.setMessage(msg);
            return;
        }

About

ByronNevins

Search

Archives
« May 2012 »
SunMonTueWedThuFriSat
  
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
21
22
23
24
25
26
27
28
29
30
31
  
       
Today