Opened 12 years ago

Closed 12 years ago

#69 closed defect (fixed)

DBControl bad code style, beware and fix it!

Reported by: Jari Häkkinen Owned by: Jari Häkkinen
Priority: critical Milestone:
Component: se.lu.thep.affymetrix Keywords:
Cc:

Description

Read the below comments and go through the code and fix the issues!

Nicklas says about the affymetrix plug-in:

I checked the code for the Plier plug-in and the getAPTPath is opening a new DbControl?, but it never calls close() or commit(). The pattern for using a DbControl? should always include a try-finally:

DbControl dc = ....
try
{
   ... do something here
   dc.commit();
}
finally
{
   if (dc != null) dc.close();
}

Checking the code in the package reveals the code block below. Nicklas comments it:

This code is not dangerous at all, and Java doesn't has to be smart. The code in the "finally" block is always executed. It doesn't matter if there is a return statement in the "try" block or if there is an exception. The most dangerous thing with this code is the "catch" block. It just chews up the exception and then the "return null;" statement is executed telling the core that everything is OK and that the plug-in can be used. I think it would be better to remove the "catch" block.

public String isInContext(GuiContext context, Object item)
{
  DbControl dc = sc.newDbControl();
  try {
    Experiment e=Experiment.getById(dc, sc.getCurrentContext(Item.EXPERIMENT).getId());
    if (!e.getRawDataType().isAffymetrix())
      return ("Raw data type '" + e.getRawDataType() + "' is not supoorted by this plug-in.");
  }
  catch (Throwable e) {
  }
  finally {
    if (dc != null) dc.close();
  }
  return null;
}

Change History (2)

comment:1 Changed 12 years ago by Jari Häkkinen

Status: newassigned

comment:2 Changed 12 years ago by Jari Häkkinen

Resolution: fixed
Status: assignedclosed

(In [343]) Fixes #69. DBControl usage is improved.

Note: See TracTickets for help on using tickets.