fixing a bunch of inconsistencies and potential bugs as indicated by findbugs, pmd and eclipse

git-svn-id: http://google-refine.googlecode.com/svn/trunk@2301 7d457c2a-affb-35e4-300a-418c747d4874

main/src/com/google/refine/RefineServlet.java

@@ -78,7 +78,7 @@ public class RefineServlet extends Butterfly {
     static final private Map<String, Command> commands = new HashMap<String, Command>();
 
     // timer for periodically saving projects
-    static private Timer _timer;
+    static private Timer _timer = new Timer("autosave");
 
     static final Logger logger = LoggerFactory.getLogger("refine");
 
@@ -132,10 +132,7 @@ public class RefineServlet extends Butterfly {
         FileProjectManager.initialize(s_dataDir);
         ImportingManager.initialize(this);
 
-        if (_timer == null) {
-            _timer = new Timer("autosave");
         _timer.schedule(new AutoSaveTimerTask(), s_autoSavePeriod);
-        }
 
         logger.trace("< initialize");
     }

main/src/com/google/refine/browsing/facets/ScatterplotDrawingRowVisitor.java

@@ -56,7 +56,6 @@ public class ScatterplotDrawingRowVisitor implements RowVisitor, RecordVisitor {
     int col_y;
     int dim_x;
     int dim_y;
-    int rotation;
 
     double l;
     double dot;
@@ -84,7 +83,6 @@ public class ScatterplotDrawingRowVisitor implements RowVisitor, RecordVisitor {
         this.dot = dot;
         this.dim_x = dim_x;
         this.dim_y = dim_y;
-        this.rotation = rotation;
         
         l = size;
         r = ScatterplotFacet.createRotationMatrix(rotation, l);

main/src/com/google/refine/browsing/util/ExpressionTimeValueBinner.java

@@ -170,7 +170,7 @@ public class ExpressionTimeValueBinner implements RowVisitor, RecordVisitor {
                 long t = ((Date) value).getTime();
                 hasTime = true;
                     
-                int bin = (int) Math.floor((t - _index.getMin()) / _index.getStep());
+                int bin = (int) Math.floor((double) (t - _index.getMin()) / (double) _index.getStep());
                 if (bin >= 0 && bin < bins.length) { // as a precaution
                     bins[bin]++;
                 }

main/src/com/google/refine/exporters/CsvExporter.java

@@ -67,7 +67,7 @@ public class CsvExporter implements WriterExporter{
     public void export(Project project, Properties params, Engine engine, final Writer writer)
             throws IOException {
         
-        String optionsString = params == null ? null : params.getProperty("options");
+        String optionsString = (params == null) ? null : params.getProperty("options");
         JSONObject options = null;
         if (optionsString != null) {
             try {
@@ -115,8 +115,7 @@ public class CsvExporter implements WriterExporter{
             }
         };
         
-        CustomizableTabularExporterUtilities.exportRows(
+        CustomizableTabularExporterUtilities.exportRows(project, engine, params, serializer);
-                project, engine, params, serializer);
         
         csvWriter.close();
     }

main/src/com/google/refine/exporters/CustomizableTabularExporterUtilities.java

@@ -70,7 +70,7 @@ abstract public class CustomizableTabularExporterUtilities {
         Properties params,
         final TabularSerializer serializer) {
         
-        String optionsString = params.getProperty("options");
+        String optionsString = (params != null) ? params.getProperty("options") : null;
         JSONObject optionsTemp = null;
         if (optionsString != null) {
             try {

main/src/com/google/refine/expr/ExpressionUtils.java

@@ -50,7 +50,8 @@ import com.google.refine.model.Project;
 import com.google.refine.model.Row;
 
 public class ExpressionUtils {
-    static protected Set<Binder> s_binders = new HashSet<Binder>();
+    
+    static final protected Set<Binder> s_binders = new HashSet<Binder>();
 
     static public void registerBinder(Binder binder) {
         s_binders.add(binder);

main/src/com/google/refine/expr/MetaParser.java

@@ -44,6 +44,7 @@ import clojure.lang.IFn;
 import com.google.refine.grel.Parser;
 
 abstract public class MetaParser {
+
     static public class LanguageInfo {
         final public String                 name;
         final public LanguageSpecificParser parser;
@@ -56,10 +57,9 @@ abstract public class MetaParser {
         }
     }
     
-    static protected Map<String, LanguageInfo> s_languages;
+    static final protected Map<String, LanguageInfo> s_languages = new HashMap<String, LanguageInfo>();
-    static {
-        s_languages = new HashMap<String, LanguageInfo>();
     
+    static {
         registerLanguageParser("grel", "Google Refine Expression Language (GREL)", new LanguageSpecificParser() {
             
             @Override

main/src/com/google/refine/expr/functions/Get.java

@@ -103,7 +103,7 @@ public class Get implements Function {
                                     return ExpressionUtils.toObjectList(v).get(start);
                                 }
                             } else {
-                                int end = (to != null) ? ((Number) to).intValue() : length;
+                                int end = ((Number) to).intValue();
                                             
                                 if (end < 0) {
                                     end = length + end;

main/src/com/google/refine/expr/functions/math/Combin.java

@@ -56,10 +56,10 @@ public class Combin implements Function {
             return new EvalError(ControlFunctionRegistry.getFunctionName(this) + " expects the second argument to be a number");
         }
 
-        return Combin.Combination(((Number) args[0]).intValue(), ((Number) args[1]).intValue());
+        return Combin.combination(((Number) args[0]).intValue(), ((Number) args[1]).intValue());
     }
 
-    public static int Combination(int n, int k){
+    public static int combination(int n, int k){
         if (k > n) {
             throw new IllegalArgumentException ("the number of elements, n, should be larger than the number of combinations, k");
         }

main/src/com/google/refine/expr/functions/math/Even.java

@@ -47,12 +47,12 @@ public class Even implements Function {
     @Override
     public Object call(Properties bindings, Object[] args) {
         if (args.length == 1 && args[0] != null && args[0] instanceof Number) {
-            return Even.RoundUpToEven(((Number) args[0]).doubleValue());
+            return Even.roundUpToEven(((Number) args[0]).doubleValue());
         }
         return new EvalError(ControlFunctionRegistry.getFunctionName(this) + " expects a number");
     }
 
-    public static double RoundUpToEven(double d){
+    public static double roundUpToEven(double d){
         double temp = Math.ceil(d);
         return ((temp % 2) == 0) ? temp : temp+1;
     }

main/src/com/google/refine/expr/functions/math/FactN.java

@@ -52,7 +52,7 @@ public class FactN implements Function {
         if (args[0] == null || !(args[0] instanceof Number)) {
             return new EvalError(ControlFunctionRegistry.getFunctionName(this) + " expects the first parameter to be a number");
         }
-        if(args[1] == null && !(args[1] instanceof Number)) {
+        if (args[1] == null || !(args[1] instanceof Number)) {
             return new EvalError(ControlFunctionRegistry.getFunctionName(this) + " expects the second parameter to be a number");
         }
 

main/src/com/google/refine/expr/functions/math/Multinomial.java

@@ -52,7 +52,7 @@ public class Multinomial implements Function {
         int sum = 0;
         int product = 1;
         for (int i = 0; i < args.length; i++){
-            if(args[i] == null && !(args[i] instanceof Number)) {
+            if (args[i] == null || !(args[i] instanceof Number)) {
                 return new EvalError(ControlFunctionRegistry.getFunctionName(this) + " expects parameter " + (i + 1) + " to be a number");
             }
             int num = ((Number) args[i]).intValue();

main/src/com/google/refine/expr/functions/math/Odd.java

@@ -47,12 +47,12 @@ public class Odd implements Function {
     @Override
     public Object call(Properties bindings, Object[] args) {
         if (args.length == 1 && args[0] != null && args[0] instanceof Number) {
-            return Odd.RoundUpToOdd(((Number) args[0]).doubleValue());
+            return Odd.roundUpToOdd(((Number) args[0]).doubleValue());
         }
         return new EvalError(ControlFunctionRegistry.getFunctionName(this) + " expects a number");
     }
 
-    public static double RoundUpToOdd(double d){
+    public static double roundUpToOdd(double d){
         double temp = Math.ceil(d);
         return ((temp % 2) == 0) ? temp + 1 : temp;
     }

main/src/com/google/refine/expr/functions/strings/SmartSplit.java

@@ -46,7 +46,8 @@ import com.google.refine.grel.ControlFunctionRegistry;
 import com.google.refine.grel.Function;
 
 public class SmartSplit implements Function {
-    static protected CSVParser s_tabParser = new CSVParser(
+
+    static final protected CSVParser s_tabParser = new CSVParser(
         '\t',
         CSVParser.DEFAULT_QUOTE_CHARACTER,
         CSVParser.DEFAULT_ESCAPE_CHARACTER,
@@ -54,7 +55,8 @@ public class SmartSplit implements Function {
         CSVParser.DEFAULT_IGNORE_LEADING_WHITESPACE,
         false
     );
-    static protected CSVParser s_commaParser = new CSVParser(
+
+    static final protected CSVParser s_commaParser = new CSVParser(
         ',',
         CSVParser.DEFAULT_QUOTE_CHARACTER,
         CSVParser.DEFAULT_ESCAPE_CHARACTER,
@@ -62,6 +64,7 @@ public class SmartSplit implements Function {
         CSVParser.DEFAULT_IGNORE_LEADING_WHITESPACE,
         false
     );    
+
     @Override
     public Object call(Properties bindings, Object[] args) {
         if (args.length >= 1 && args.length <= 2) {

main/src/com/google/refine/importers/FixedWidthImporter.java

@@ -154,10 +154,10 @@ public class FixedWidthImporter extends TabularImportingParserBase {
     static public int[] guessColumnWidths(File file, String encoding) {
         try {
             InputStream is = new FileInputStream(file);
-            try {
+            Reader reader = (encoding != null) ? new InputStreamReader(is, encoding) : new InputStreamReader(is);
-                Reader reader = encoding != null ? new InputStreamReader(is, encoding) : new InputStreamReader(is);
             LineNumberReader lineNumberReader = new LineNumberReader(reader);
 
+            try {
                 int[] counts = null;
                 int totalBytes = 0;
                 int lineCount = 0;
@@ -213,6 +213,8 @@ public class FixedWidthImporter extends TabularImportingParserBase {
                     return widthA;
                 }
             } finally {
+                lineNumberReader.close();
+                reader.close();
                 is.close();
             }
         } catch (UnsupportedEncodingException e) {

main/src/com/google/refine/importers/SeparatorBasedImporter.java

@@ -108,15 +108,12 @@ public class SeparatorBasedImporter extends TabularImportingParserBase {
         final LineNumberReader lnReader = new LineNumberReader(reader);
         
         TableDataReader dataReader = new TableDataReader() {
-            long bytesRead = 0;
-            
             @Override
             public List<Object> getNextRowOfCells() throws IOException {
                 String line = lnReader.readLine();
                 if (line == null) {
                     return null;
                 } else {
-                    bytesRead += line.length();
                     return getCells(line, parser, lnReader);
                 }
             }
@@ -172,10 +169,10 @@ public class SeparatorBasedImporter extends TabularImportingParserBase {
     static public Separator guessSeparator(File file, String encoding) {
         try {
             InputStream is = new FileInputStream(file);
-            try {
             Reader reader = encoding != null ? new InputStreamReader(is, encoding) : new InputStreamReader(is);
             LineNumberReader lineNumberReader = new LineNumberReader(reader);
 
+            try {
                 List<Separator> separators = new ArrayList<SeparatorBasedImporter.Separator>();
                 Map<Character, Separator> separatorMap = new HashMap<Character, SeparatorBasedImporter.Separator>();
                 
@@ -236,6 +233,8 @@ public class SeparatorBasedImporter extends TabularImportingParserBase {
                     }
                 }
             } finally {
+                lineNumberReader.close();
+                reader.close();
                 is.close();
             }
         } catch (UnsupportedEncodingException e) {

main/src/com/google/refine/importers/TextFormatGuesser.java

@@ -16,9 +16,9 @@ public class TextFormatGuesser implements FormatGuesser {
     public String guess(File file, String encoding, String seedFormat) {
         try {
             InputStream is = new FileInputStream(file);
-            try {
             Reader reader = encoding != null ? new InputStreamReader(is, encoding) : new InputStreamReader(is);
 
+            try {
                 int totalBytes = 0;
                 int openBraces = 0;
                 int closeBraces = 0;
@@ -63,6 +63,7 @@ public class TextFormatGuesser implements FormatGuesser {
                 }
                 return "text/line-based";
             } finally {
+                reader.close();
                 is.close();
             }
         } catch (UnsupportedEncodingException e) {

main/src/com/google/refine/importers/tree/XmlImportUtilities.java

@@ -40,8 +40,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 
-import javax.servlet.ServletException;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -163,26 +161,13 @@ public class XmlImportUtilities extends TreeImportUtilities {
         List<RecordElementCandidate> descendantCandidates = new ArrayList<RecordElementCandidate>();
 
         Map<String, Integer> immediateChildCandidateMap = new HashMap<String, Integer>();
-        int textNodeCount = 0;
-        int childElementNodeCount = 0;
 
         try {
             while (parser.hasNext()) {
                 Token eventType = parser.next();
                 if (eventType == Token.EndEntity ) {
                     break;
-                } else if (eventType == Token.Value) {
-                    try{
-                        if (parser.getFieldValue().trim().length() > 0) {
-                            textNodeCount++;
-                        }
-                    }catch(TreeReaderException e){
-                        e.printStackTrace();
-                        //silent
-                    }
                 } else if (eventType == Token.StartEntity) {
-                    childElementNodeCount++;
-
                     String tagName = parser.getFieldName();
 
                     immediateChildCandidateMap.put(

main/src/com/google/refine/importing/ImportingManager.java

@@ -169,7 +169,7 @@ public class ImportingManager {
         controller.init(servlet);
     }
 
-    static public File getImportDir() {
+    static synchronized public File getImportDir() {
         if (importDir == null) {
             File tempDir = servlet.getTempDir();
             importDir = tempDir == null ? new File(".import-temp") : new File(tempDir, "import");

main/src/com/google/refine/io/FileProjectManager.java

@@ -371,7 +371,7 @@ public class FileProjectManager extends ProjectManager {
                 logger.warn("Error reading file", e);
             } finally {
                 try {
-                    reader.close();
+                    if (reader != null) reader.close();
                 } catch (IOException e) {
                     logger.warn("Exception closing file",e);
                 }

main/src/com/google/refine/model/ColumnModel.java

@@ -70,7 +70,7 @@ public class ColumnModel implements Jsonizable {
         this._maxCellIndex = Math.max(this._maxCellIndex, maxCellIndex);
     }
 
-    public int getMaxCellIndex() {
+    synchronized public int getMaxCellIndex() {
         return _maxCellIndex;
     }
 
@@ -78,12 +78,12 @@ public class ColumnModel implements Jsonizable {
         return _maxCellIndex++;
     }
     
-    public void setKeyColumnIndex(int keyColumnIndex) {
+    synchronized public void setKeyColumnIndex(int keyColumnIndex) {
         // TODO: check validity of new cell index, e.g., it's not in any group
         this._keyColumnIndex = keyColumnIndex;
     }
 
-    public int getKeyColumnIndex() {
+    synchronized public int getKeyColumnIndex() {
         return _keyColumnIndex;
     }
     

main/src/com/google/refine/model/Row.java

@@ -240,7 +240,7 @@ public class Row implements HasFields, Jsonizable {
         StringBuffer result = new StringBuffer();
         for (Cell cell : cells) {
             result.append(cell == null ? "null" : cell.toString());
-            result.append(",");
+            result.append(',');
         }
         return result.toString();
     }

main/src/com/google/refine/preference/TopList.java

@@ -46,9 +46,7 @@ import org.json.JSONWriter;
 
 import com.google.refine.Jsonizable;
 
-
 public class TopList implements Jsonizable, Iterable<String> {
-    private static final long serialVersionUID = 2666669643063493350L;
     
     final protected int          _top;
     final protected List<String> _list = new ArrayList<String>();

main/tests/server/src/com/google/refine/tests/ProjectManagerStub.java

@@ -49,10 +49,6 @@ import com.google.refine.model.Project;
  */
 public class ProjectManagerStub extends ProjectManager {
 
-    public ProjectManagerStub(){
-        super();
-    }
-
     @Override
     public void deleteProject(long projectID) {
         // empty