From 673b223f61899c536d31877383d6d8053d2bda06 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Wed, 11 Oct 2017 05:32:05 +0000 Subject: [PATCH] Remove "filling" in IntList as this has no effect whatsoever as far as I could see Fix some IntelliJ warnings, missing JavaDoc, typos, Findbugs issues git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1811793 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/poi/hpsf/CustomProperties.java | 11 +- src/java/org/apache/poi/hpsf/PropertySet.java | 4 +- src/java/org/apache/poi/hpsf/Section.java | 19 +-- .../ss/formula/DataValidationEvaluator.java | 23 ++-- src/java/org/apache/poi/util/IntList.java | 35 +----- .../openxml4j/opc/internal/FileHelper.java | 112 +++++++++--------- .../hwpf/model/TestDocumentProperties.java | 26 ++-- .../org/apache/poi/util/TestIntList.java | 97 +++++++++++---- 8 files changed, 169 insertions(+), 158 deletions(-) diff --git a/src/java/org/apache/poi/hpsf/CustomProperties.java b/src/java/org/apache/poi/hpsf/CustomProperties.java index 295251b8c..84373c589 100644 --- a/src/java/org/apache/poi/hpsf/CustomProperties.java +++ b/src/java/org/apache/poi/hpsf/CustomProperties.java @@ -202,15 +202,12 @@ public class CustomProperties implements Map { @Override public boolean equals(Object obj) { - if (!(obj instanceof CustomProperties)) { - return false; - } - return props.equals(((CustomProperties)obj).props); + return obj instanceof CustomProperties && props.equals(((CustomProperties) obj).props); } @Override - public void putAll(Map m) { - for (Map.Entry me : m.entrySet()) { + public void putAll(Map m) { + for (Map.Entry me : m.entrySet()) { put(me.getKey(), me.getValue()); } } @@ -365,7 +362,7 @@ public class CustomProperties implements Map { *
  • Otherwise find the highest ID and use its value plus one. * * - * @param customProperty + * @param customProperty The {@link CustomProperty} to add. * @return If there was already a property with the same name, the old property * @throws ClassCastException */ diff --git a/src/java/org/apache/poi/hpsf/PropertySet.java b/src/java/org/apache/poi/hpsf/PropertySet.java index b21d1a036..a9e8e961a 100644 --- a/src/java/org/apache/poi/hpsf/PropertySet.java +++ b/src/java/org/apache/poi/hpsf/PropertySet.java @@ -754,8 +754,8 @@ public class PropertySet { * #getPropertyIntValue} or {@link #getProperty} tried to access * was available or not. This information might be important for * callers of {@link #getPropertyIntValue} since the latter - * returns 0 if the property does not exist. Using {@link - * #wasNull}, the caller can distiguish this case from a + * returns 0 if the property does not exist. Using wasNull, + * the caller can distinguish this case from a * property's real value of 0. * * @return {@code true} if the last call to {@link diff --git a/src/java/org/apache/poi/hpsf/Section.java b/src/java/org/apache/poi/hpsf/Section.java index 33fc3d4bb..720249946 100644 --- a/src/java/org/apache/poi/hpsf/Section.java +++ b/src/java/org/apache/poi/hpsf/Section.java @@ -516,10 +516,7 @@ public class Section { */ protected boolean getPropertyBooleanValue(final int id) { final Boolean b = (Boolean) getProperty(id); - if (b == null) { - return false; - } - return b.booleanValue(); + return b != null && b.booleanValue(); } /** @@ -560,8 +557,8 @@ public class Section { * properties) and the properties themselves. * * @return the section's length in bytes. - * @throws WritingNotSupportedException - * @throws IOException + * @throws WritingNotSupportedException If the document is opened read-only. + * @throws IOException If an error happens while writing. */ private int calcSize() throws WritingNotSupportedException, IOException { sectionBytes.reset(); @@ -765,9 +762,6 @@ public class Section { * property. */ position += p.write(propertyStream, codepage); } else { - if (codepage == -1) { - throw new IllegalPropertySetDataException("Codepage (property 1) is undefined."); - } position += writeDictionary(propertyStream, codepage); } } @@ -865,7 +859,6 @@ public class Section { * Writes the section's dictionary. * * @param out The output stream to write to. - * @param dictionary The dictionary. * @param codepage The codepage to be used to write the dictionary items. * @return The number of bytes written * @exception IOException if an I/O exception occurs. @@ -956,8 +949,8 @@ public class Section { long hashCode = 0; hashCode += getFormatID().hashCode(); final Property[] pa = getProperties(); - for (int i = 0; i < pa.length; i++) { - hashCode += pa[i].hashCode(); + for (Property aPa : pa) { + hashCode += aPa.hashCode(); } return (int) (hashCode & 0x0ffffffffL); } @@ -973,7 +966,7 @@ public class Section { } public String toString(PropertyIDMap idMap) { - final StringBuffer b = new StringBuffer(); + final StringBuilder b = new StringBuilder(); final Property[] pa = getProperties(); b.append("\n\n\n"); b.append(getClass().getName()); diff --git a/src/java/org/apache/poi/ss/formula/DataValidationEvaluator.java b/src/java/org/apache/poi/ss/formula/DataValidationEvaluator.java index c6dadfff7..c49d5f0e3 100644 --- a/src/java/org/apache/poi/ss/formula/DataValidationEvaluator.java +++ b/src/java/org/apache/poi/ss/formula/DataValidationEvaluator.java @@ -96,7 +96,8 @@ public class DataValidationEvaluator { /** * Lazy load validations by sheet, since reading the CT* types is expensive - * @param sheet + * + * @param sheet The {@link Sheet} to load validations for. * @return The {@link DataValidation}s for the sheet */ private List getValidations(Sheet sheet) { @@ -250,8 +251,9 @@ public class DataValidationEvaluator { /** * Note that this assumes the cell cached value is up to date and in sync with data edits - * @param cell - * @param type + * + * @param cell The {@link Cell} to check. + * @param type The {@link CellType} to check for. * @return true if the cell or cached cell formula result type match the given type */ public static boolean isType(Cell cell, CellType type) { @@ -278,7 +280,7 @@ public class DataValidationEvaluator { if (super.isValidValue(cell, context)) { // we know it is a number in the proper range, now check if it is an int final double value = cell.getNumericCellValue(); // can't get here without a valid numeric value - return Double.valueOf(value).compareTo(Double.valueOf((int) value)) == 0; + return Double.compare(value, (int) value) == 0; } return false; } @@ -342,7 +344,7 @@ public class DataValidationEvaluator { /** * Note the formula result must either be a boolean result, or anything not in error. * If boolean, value must be true to pass, anything else valid is also passing, errors fail. - * @see org.apache.poi.ss.formula.DataValidationEvaluator.ValidationEnum#isValidValue(org.apache.poi.ss.usermodel.Cell, org.apache.poi.ss.usermodel.DataValidationConstraint, org.apache.poi.ss.formula.WorkbookEvaluator) + * @see org.apache.poi.ss.formula.DataValidationEvaluator.ValidationEnum#isValidValue(Cell, DataValidationContext) */ public boolean isValidValue(Cell cell, DataValidationContext context) { // unwrapped single value @@ -409,8 +411,9 @@ public class DataValidationEvaluator { * Evaluate a numeric formula value as either a constant or numeric expression. * Note that Excel treats validations with constraint formulas that evaluate to null as valid, * but evaluations in error or non-numeric are marked invalid. - * @param formula - * @param context + * + * @param formula The text of the formula or a numeric value + * @param context The {@link DataValidationContext} which is used for evaluation * @return numeric value or null if not defined or the formula evaluates to an empty/missing cell. * @throws NumberFormatException if the formula is non-numeric when it should be */ @@ -522,11 +525,7 @@ public class DataValidationEvaluator { private final CellReference target; /** - * - * @param dv - * @param dve - * @param region - * @param target + * Populate the context with the necessary values. */ public DataValidationContext(DataValidation dv, DataValidationEvaluator dve, CellRangeAddressBase region, CellReference target) { this.dv = dv; diff --git a/src/java/org/apache/poi/util/IntList.java b/src/java/org/apache/poi/util/IntList.java index 62276d920..9687d2fab 100644 --- a/src/java/org/apache/poi/util/IntList.java +++ b/src/java/org/apache/poi/util/IntList.java @@ -47,7 +47,6 @@ public class IntList { private int[] _array; private int _limit; - private int fillval; private static final int _default_size = 128; /** @@ -61,7 +60,8 @@ public class IntList public IntList(final int initialCapacity) { - this(initialCapacity,0); + _array = new int[ initialCapacity ]; + _limit = 0; } @@ -79,29 +79,7 @@ public class IntList } /** - * create an IntList with a predefined initial size - * - * @param initialCapacity the size for the internal array - */ - - public IntList(final int initialCapacity, int fillvalue) - { - _array = new int[ initialCapacity ]; - if (fillval != 0) { - fillval = fillvalue; - fillArray(fillval, _array, 0); - } - _limit = 0; - } - - private void fillArray(int val, int[] array, int index) { - for (int k = index; k < array.length; k++) { - array[k] = val; - } - } - - /** - * add the specfied value at the specified index + * add the specified value at the specified index * * @param index the index where the new value is to be added * @param value the new value @@ -651,12 +629,7 @@ public class IntList : new_size; int[] new_array = new int[ size ]; - if (fillval != 0) { - fillArray(fillval, new_array, _array.length); - } - System.arraycopy(_array, 0, new_array, 0, _limit); _array = new_array; } -} // end public class IntList - +} diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/FileHelper.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/FileHelper.java index 0af90a238..b1e8192d5 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/FileHelper.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/FileHelper.java @@ -31,64 +31,62 @@ import java.nio.channels.FileChannel; */ public final class FileHelper { - /** - * Get the directory part of the specified file path. - * - * @param f - * File to process. - * @return The directory path from the specified - */ - public static File getDirectory(File f) { - if (f != null) { - String path = f.getPath(); + /** + * Get the directory part of the specified file path. + * + * @param f + * File to process. + * @return The directory path from the specified + */ + public static File getDirectory(File f) { + if (f != null) { + String path = f.getPath(); int num2 = path.length(); - while (--num2 >= 0) { - char ch1 = path.charAt(num2); - if (ch1 == File.separatorChar) { - return new File(path.substring(0, num2)); - } - } - } - return null; - } + while (--num2 >= 0) { + char ch1 = path.charAt(num2); + if (ch1 == File.separatorChar) { + return new File(path.substring(0, num2)); + } + } + } + return null; + } - /** - * Copy a file. - * - * @param in - * The source file. - * @param out - * The target location. - * @throws IOException - * If an I/O error occur. - */ - public static void copyFile(File in, File out) throws IOException { - FileInputStream fis = new FileInputStream(in); - FileOutputStream fos = new FileOutputStream(out); - FileChannel sourceChannel = fis.getChannel(); - FileChannel destinationChannel = fos.getChannel(); - sourceChannel.transferTo(0, sourceChannel.size(), destinationChannel); - sourceChannel.close(); - destinationChannel.close(); - fos.close(); - fis.close(); - } - - /** - * Get file name from the specified File object. - */ - public static String getFilename(File file) { - if (file != null) { - String path = file.getPath(); - int len = path.length(); - int num2 = len; - while (--num2 >= 0) { - char ch1 = path.charAt(num2); - if (ch1 == File.separatorChar) - return path.substring(num2 + 1, len); - } - } - return ""; - } + /** + * Copy a file. + * + * @param in + * The source file. + * @param out + * The target location. + * @throws IOException + * If an I/O error occur. + */ + public static void copyFile(File in, File out) throws IOException { + try (FileInputStream fis = new FileInputStream(in); + FileOutputStream fos = new FileOutputStream(out); + FileChannel sourceChannel = fis.getChannel(); + FileChannel destinationChannel = fos.getChannel()) { + + sourceChannel.transferTo(0, sourceChannel.size(), destinationChannel); + sourceChannel.close(); + } + } + /** + * Get file name from the specified File object. + */ + public static String getFilename(File file) { + if (file != null) { + String path = file.getPath(); + int len = path.length(); + int num2 = len; + while (--num2 >= 0) { + char ch1 = path.charAt(num2); + if (ch1 == File.separatorChar) + return path.substring(num2 + 1, len); + } + } + return ""; + } } diff --git a/src/scratchpad/testcases/org/apache/poi/hwpf/model/TestDocumentProperties.java b/src/scratchpad/testcases/org/apache/poi/hwpf/model/TestDocumentProperties.java index 60703f280..05ca7d64a 100644 --- a/src/scratchpad/testcases/org/apache/poi/hwpf/model/TestDocumentProperties.java +++ b/src/scratchpad/testcases/org/apache/poi/hwpf/model/TestDocumentProperties.java @@ -17,33 +17,23 @@ package org.apache.poi.hwpf.model; -import static org.apache.poi.POITestCase.assertReflectEquals; - import org.apache.poi.hwpf.HWPFDocFixture; import org.apache.poi.hwpf.model.types.DOPAbstractType; import org.junit.After; import org.junit.Before; import org.junit.Test; +import static org.apache.poi.POITestCase.assertReflectEquals; + // TODO: Add DocumentProperties#equals ??? public final class TestDocumentProperties { private DocumentProperties _documentProperties; private HWPFDocFixture _hWPFDocFixture; - @Test - public void testReadWrite() throws Exception { - int size = DOPAbstractType.getSize(); - byte[] buf = new byte[size]; - _documentProperties.serialize(buf, 0); - DocumentProperties newDocProperties = new DocumentProperties(buf, 0, size); - - assertReflectEquals(_documentProperties, newDocProperties); - } - @Before public void setUp() throws Exception { - /** TODO verify the constructors*/ + // TODO verify the constructors _hWPFDocFixture = new HWPFDocFixture(this, HWPFDocFixture.DEFAULT_TEST_FILE); _hWPFDocFixture.setUp(); _documentProperties = new DocumentProperties(_hWPFDocFixture._tableStream, _hWPFDocFixture._fib.getFcDop(), _hWPFDocFixture._fib.getLcbDop()); @@ -55,4 +45,14 @@ public final class TestDocumentProperties { _hWPFDocFixture.tearDown(); _hWPFDocFixture = null; } + + @Test + public void testReadWrite() throws Exception { + int size = DOPAbstractType.getSize(); + byte[] buf = new byte[size]; + _documentProperties.serialize(buf, 0); + DocumentProperties newDocProperties = new DocumentProperties(buf, 0, size); + + assertReflectEquals(_documentProperties, newDocProperties); + } } diff --git a/src/testcases/org/apache/poi/util/TestIntList.java b/src/testcases/org/apache/poi/util/TestIntList.java index 092f8af35..7f5f721f5 100644 --- a/src/testcases/org/apache/poi/util/TestIntList.java +++ b/src/testcases/org/apache/poi/util/TestIntList.java @@ -17,15 +17,21 @@ package org.apache.poi.util; -import junit.framework.TestCase; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * Class to test IntList * * @author Marc Johnson */ -public final class TestIntList extends TestCase { - +public final class TestIntList { + @Test public void testConstructors() { IntList list = new IntList(); @@ -40,6 +46,7 @@ public final class TestIntList extends TestCase { assertTrue(list3.isEmpty()); } + @Test public void testAdd() { IntList list = new IntList(); int[] testArray = @@ -117,6 +124,7 @@ public final class TestIntList extends TestCase { } } + @Test public void testAddAll() { IntList list = new IntList(); @@ -191,6 +199,17 @@ public final class TestIntList extends TestCase { assertEquals(list.get(4), empty.get(14)); } + @Test + public void testAddAllGrow() { + IntList list = new IntList(0); + IntList addList = new IntList(0); + addList.add(1); + addList.add(2); + + assertTrue(list.addAll(0, addList)); + } + + @Test public void testClear() { IntList list = new IntList(); @@ -212,6 +231,7 @@ public final class TestIntList extends TestCase { } } + @Test public void testContains() { IntList list = new IntList(); @@ -227,11 +247,12 @@ public final class TestIntList extends TestCase { } else { - assertTrue(!list.contains(j)); + assertFalse(list.contains(j)); } } } + @Test public void testContainsAll() { IntList list = new IntList(); @@ -246,18 +267,19 @@ public final class TestIntList extends TestCase { assertTrue(list.containsAll(list2)); list2.add(10); assertTrue(list2.containsAll(list)); - assertTrue(!list.containsAll(list2)); + assertFalse(list.containsAll(list2)); list.add(11); - assertTrue(!list2.containsAll(list)); - assertTrue(!list.containsAll(list2)); + assertFalse(list2.containsAll(list)); + assertFalse(list.containsAll(list2)); } + @Test public void testEquals() { IntList list = new IntList(); assertEquals(list, list); //noinspection ObjectEqualsNull - assertTrue(!list.equals(null)); + assertFalse(list.equals(null)); IntList list2 = new IntList(200); assertEquals(list, list2); @@ -267,16 +289,18 @@ public final class TestIntList extends TestCase { list.add(1); list2.add(1); list2.add(0); - assertTrue(!list.equals(list2)); + assertFalse(list.equals(list2)); list2.removeValue(1); list2.add(1); assertEquals(list, list2); assertEquals(list2, list); + assertEquals(list.hashCode(), list2.hashCode()); list2.add(2); - assertTrue(!list.equals(list2)); - assertTrue(!list2.equals(list)); + assertFalse(list.equals(list2)); + assertFalse(list2.equals(list)); } + @Test public void testGet() { IntList list = new IntList(); @@ -304,6 +328,7 @@ public final class TestIntList extends TestCase { } } + @Test public void testIndexOf() { IntList list = new IntList(); @@ -324,6 +349,7 @@ public final class TestIntList extends TestCase { } } + @Test public void testIsEmpty() { IntList list1 = new IntList(); IntList list2 = new IntList(1000); @@ -335,9 +361,9 @@ public final class TestIntList extends TestCase { list1.add(1); list2.add(2); list3 = new IntList(list2); - assertTrue(!list1.isEmpty()); - assertTrue(!list2.isEmpty()); - assertTrue(!list3.isEmpty()); + assertFalse(list1.isEmpty()); + assertFalse(list2.isEmpty()); + assertFalse(list3.isEmpty()); list1.clear(); list2.remove(0); list3.removeValue(2); @@ -346,6 +372,7 @@ public final class TestIntList extends TestCase { assertTrue(list3.isEmpty()); } + @Test public void testLastIndexOf() { IntList list = new IntList(); @@ -366,6 +393,7 @@ public final class TestIntList extends TestCase { } } + @Test public void testRemove() { IntList list = new IntList(); @@ -399,6 +427,7 @@ public final class TestIntList extends TestCase { } } + @Test public void testRemoveValue() { IntList list = new IntList(); @@ -413,10 +442,11 @@ public final class TestIntList extends TestCase { assertTrue(list.removeValue(j)); assertTrue(list.removeValue(j)); } - assertTrue(!list.removeValue(j)); + assertFalse(list.removeValue(j)); } } + @Test public void testRemoveAll() { IntList list = new IntList(); @@ -439,16 +469,26 @@ public final class TestIntList extends TestCase { listOdd.add(j); } } - list.removeAll(listEven); + + assertTrue(list.removeAll(listEven)); assertEquals(list, listOdd); - list.removeAll(listOdd); + + assertTrue(list.removeAll(listOdd)); assertTrue(list.isEmpty()); - listCopy.removeAll(listOdd); + + assertTrue(listCopy.removeAll(listOdd)); assertEquals(listCopy, listEven); - listCopy.removeAll(listEven); + + assertTrue(listCopy.removeAll(listEven)); assertTrue(listCopy.isEmpty()); + + assertFalse(list.removeAll(listEven)); + assertFalse(list.removeAll(listOdd)); + assertFalse(listCopy.removeAll(listEven)); + assertFalse(listCopy.removeAll(listEven)); } + @Test public void testRetainAll() { IntList list = new IntList(); @@ -471,16 +511,25 @@ public final class TestIntList extends TestCase { listOdd.add(j); } } - list.retainAll(listOdd); + assertTrue(list.retainAll(listOdd)); assertEquals(list, listOdd); - list.retainAll(listEven); + + assertTrue(list.retainAll(listEven)); assertTrue(list.isEmpty()); - listCopy.retainAll(listEven); + + assertTrue(listCopy.retainAll(listEven)); assertEquals(listCopy, listEven); - listCopy.retainAll(listOdd); + + assertTrue(listCopy.retainAll(listOdd)); assertTrue(listCopy.isEmpty()); + + assertFalse(list.retainAll(listOdd)); + assertFalse(list.retainAll(listEven)); + assertFalse(listCopy.retainAll(listEven)); + assertFalse(listCopy.retainAll(listOdd)); } + @Test public void testSet() { IntList list = new IntList(); @@ -509,6 +558,7 @@ public final class TestIntList extends TestCase { } } + @Test public void testSize() { IntList list = new IntList(); @@ -526,6 +576,7 @@ public final class TestIntList extends TestCase { } } + @Test public void testToArray() { IntList list = new IntList();