Made code more generic: Used Map instead of HashMap in method signatures.

Jean-Noel Rouvignac

30.50.2013 369d439a72b19cd7f95aaae555b180cd4f0eaedd

Made code more generic: Used Map instead of HashMap in method signatures.

AttrHistoricalMultiple.java, AttrHistoricalSingle.java:
Changed comments to javadocs.
Fixed condition checking.
Inlined local variables and used the field instead.
Replaced use of Boolean with boolean.

We were looking at this code with Ludo and thought replacing the List with a Map had to be done in conjunction with breaking up AttrValueHistorical class to reduce memory usage (removing stroing unnecessary references).
The value field should be used as the key of the Map, and the value of the Map should contain an object with a reference to a ChangeNumber and a boolean indicating whether it is an update or a delete (Since they are mutually exclusive).

 opends/src/server/org/opends/server/replication/plugin/AttrHistoricalMultiple.java

@@ -29,6 +29,7 @@

import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;

import org.opends.server.replication.common.ChangeNumber;
import org.opends.server.types.Attribute;
@@ -51,9 +52,11 @@
 */
public class AttrHistoricalMultiple extends AttrHistorical
{
   private ChangeNumber deleteTime, // last time when the attribute was deleted
                        lastUpdateTime; // last time the attribute was modified
   private HashMap<AttrValueHistorical,AttrValueHistorical> valuesHist;
  /** Last time when the attribute was deleted */
  private ChangeNumber deleteTime;
  /** Last time the attribute was modified */
  private ChangeNumber lastUpdateTime;
  private final Map<AttrValueHistorical, AttrValueHistorical> valuesHist;

   /**
    * Create a new object from the provided informations.
@@ -63,7 +66,7 @@
    */
   public AttrHistoricalMultiple(ChangeNumber deleteTime,
       ChangeNumber updateTime,
       HashMap<AttrValueHistorical,AttrValueHistorical> valuesHist)
       Map<AttrValueHistorical, AttrValueHistorical> valuesHist)
   {
     this.deleteTime = deleteTime;
     this.lastUpdateTime = updateTime;
@@ -96,6 +99,7 @@
    * Returns the last time when the attribute was deleted.
    * @return the last time when the attribute was deleted
    */
   @Override
   public ChangeNumber getDeleteTime()
   {
     return deleteTime;
@@ -227,7 +231,8 @@
   *
   * @return the list of historical informations for the values.
   */
  public HashMap<AttrValueHistorical,AttrValueHistorical> getValuesHistorical()
  @Override
  public Map<AttrValueHistorical, AttrValueHistorical> getValuesHistorical()
  {
    return valuesHist;
  }
@@ -235,6 +240,7 @@
  /**
   * {@inheritDoc}
   */
  @Override
  public boolean replayOperation(
      Iterator<Modification> modsIterator, ChangeNumber changeNumber,
      Entry modifiedEntry, Modification m)
@@ -327,6 +333,7 @@
   * @param changeNumber The changeNumber of the operation to process
   * @param mod The modify operation to process.
   */
  @Override
  public void processLocalOrNonConflictModification(ChangeNumber changeNumber,
      Modification mod)
  {
@@ -453,22 +460,18 @@
    }
    else
    {
      /*
       * we are processing DELETE of some attribute values
       */
      HashMap<AttrValueHistorical,AttrValueHistorical> valuesInfo =
          getValuesHistorical();
      // we are processing DELETE of some attribute values
      AttributeBuilder builder = new AttributeBuilder(modAttr);

      for (AttributeValue val : modAttr)
      {
        Boolean deleteIt = true;  // true if the delete must be done
        Boolean addedInCurrentOp = false;
        boolean deleteIt = true; // true if the delete must be done
        boolean addedInCurrentOp = false;

        /* update historical information */
        AttrValueHistorical valInfo =
          new AttrValueHistorical(val, null, changeNumber);
        AttrValueHistorical oldValInfo = valuesInfo.get(valInfo);
        AttrValueHistorical oldValInfo = valuesHist.get(valInfo);
        if (oldValInfo != null)
        {
          /* this value already exist in the historical information */
@@ -481,7 +484,7 @@
          if (changeNumber.newerOrEquals(oldValInfo.getValueDeleteTime()) &&
              changeNumber.newerOrEquals(oldValInfo.getValueUpdateTime()))
          {
            valuesInfo.put(valInfo, valInfo);
            valuesHist.put(valInfo, valInfo);
          }
          else if (oldValInfo.isUpdate())
          {
@@ -490,7 +493,7 @@
        }
        else
        {
          valuesInfo.put(valInfo, valInfo);
          valuesHist.put(valInfo, valInfo);
        }

        /* if the attribute value is not to be deleted
@@ -558,21 +561,19 @@
    AttributeBuilder builder = new AttributeBuilder(m.getAttribute());
    for (AttributeValue addVal : m.getAttribute())
    {
      HashMap<AttrValueHistorical,AttrValueHistorical> valuesInfo =
          getValuesHistorical();
      AttrValueHistorical valInfo =
        new AttrValueHistorical(addVal, changeNumber, null);
      if (valuesInfo.containsKey(valInfo) == false)
      if (!valuesHist.containsKey(valInfo))
      {
        /* this values does not exist yet
         * add it in the historical information
         * let the operation process normally
         */
        valuesInfo.put(valInfo, valInfo);
        valuesHist.put(valInfo, valInfo);
      }
      else
      {
        AttrValueHistorical oldValueInfo = valuesInfo.get(valInfo);
        AttrValueHistorical oldValueInfo = valuesHist.get(valInfo);
        if  (oldValueInfo.isUpdate())
        {
          /* if the value is already present
@@ -582,7 +583,7 @@
           */
          if (changeNumber.newer(oldValueInfo.getValueUpdateTime()))
          {
            valuesInfo.put(valInfo, valInfo);
            valuesHist.put(valInfo, valInfo);
          }
          builder.remove(addVal);
        }
@@ -598,7 +599,7 @@
             * and add our more recent one
             * let the operation process
             */
            valuesInfo.put(valInfo, valInfo);
            valuesHist.put(valInfo, valInfo);
          }
          else
          {

 opends/src/server/org/opends/server/replication/plugin/AttrHistoricalSingle.java

@@ -29,6 +29,7 @@

import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;

import org.opends.server.replication.common.ChangeNumber;
import org.opends.server.types.Attribute;
@@ -48,13 +49,17 @@
 */
public class AttrHistoricalSingle extends AttrHistorical
{
  private ChangeNumber deleteTime = null; // last time when the attribute was
                                          // deleted
  private ChangeNumber addTime = null;    // last time when a value was added
  private AttributeValue value = null;    // last added value
  /** Last time when the attribute was deleted */
  private ChangeNumber deleteTime = null;
  /** Last time when a value was added */
  private ChangeNumber addTime = null;
  /** Last added value */
  private AttributeValue value = null;

  // last operation applied. This is only used for multiple mods on the same
  // single valued attribute in the same modification.
  /**
   * last operation applied. This is only used for multiple mods on the same
   * single valued attribute in the same modification.
   */
  private HistAttrModificationKey lastMod = null;

  /**
@@ -70,7 +75,7 @@
   * {@inheritDoc}
   */
  @Override
  public HashMap<AttrValueHistorical,AttrValueHistorical> getValuesHistorical()
  public Map<AttrValueHistorical, AttrValueHistorical> getValuesHistorical()
  {
    if (addTime == null)
    {