Use a more explicit variable naming where the action is taken on a simulation movement.

Remove LOG mesage.
Improve docstrings.
Improve coding style.
Add a warning about a strange behaviour in SplitQuantity.



git-svn-id: https://svn.erp5.org/repos/public/erp5/trunk@6867 20353a03-c40f-0410-a6d1-a30d3c3de9de

product/ERP5/TargetSolver/CopyToTarget.py

@@ -35,21 +35,22 @@ class CopyToTarget(TargetSolver):
     only acceptable for root movements. The meaning of
     this solver of other movements is far from certain.
   """
-  def _generateValueDeltaDict(self, movement):
+  def _generateValueDeltaDict(self, simulation_movement):
     """
-    Get interesting value
+      Get interesting values
+      XXX: better description is possible. But is it needed ?
     """
     # Get interesting value
-    old_quantity = movement.getQuantity()
-    old_start_date = movement.getStartDate()
-    old_stop_date = movement.getStopDate()
-    new_quantity = movement.getDeliveryQuantity() * \
-                   movement.getDeliveryRatio()
-    new_start_date = movement.getDeliveryStartDateList()[0]
-    new_stop_date = movement.getDeliveryStopDateList()[0]
+    old_quantity = simulation_movement.getQuantity()
+    old_start_date = simulation_movement.getStartDate()
+    old_stop_date = simulation_movement.getStopDate()
+    new_quantity = simulation_movement.getDeliveryQuantity() * \
+                   simulation_movement.getDeliveryRatio()
+    new_start_date = simulation_movement.getDeliveryStartDateList()[0]
+    new_stop_date = simulation_movement.getDeliveryStopDateList()[0]
     # Calculate delta
     quantity_ratio = 0
-    if old_quantity not in (None,0.0):
+    if old_quantity not in (None,0.0): # XXX: What if quantity happens to be an integer ?
       quantity_ratio = new_quantity / old_quantity
     start_date_delta = 0
     stop_date_delta = 0
@@ -63,51 +64,51 @@ class CopyToTarget(TargetSolver):
       'stop_date_delta': stop_date_delta,
     }
 
-  def solve(self, movement):
+  def solve(self, simulation_movement):
     """
       Adopt values as new target
     """
-    value_dict = self._generateValueDeltaDict(movement)
+    value_dict = self._generateValueDeltaDict(simulation_movement)
     # Modify recursively simulation movement
-    self._recursivelySolve(movement, **value_dict)
+    self._recursivelySolve(simulation_movement, **value_dict)
 
-  def _generateValueDict(self, movement, quantity_ratio=1, 
+  def _generateValueDict(self, simulation_movement, quantity_ratio=1, 
                          start_date_delta=0, stop_date_delta=0,
                          **value_delta_dict):
     """
-    Generate values to save on movement.
+      Generate values to save on simulation movement.
     """
     value_dict = {}
     # Modify quantity, start_date, stop_date
-    start_date = movement.getStartDate()
+    start_date = simulation_movement.getStartDate()
     if start_date is not None:
       value_dict['start_date'] = start_date + start_date_delta
-    stop_date = movement.getStopDate()
+    stop_date = simulation_movement.getStopDate()
     if stop_date is not None:
       value_dict['stop_date'] = stop_date + stop_date_delta
-    value_dict['quantity'] = movement.getQuantity() * quantity_ratio
+    value_dict['quantity'] = simulation_movement.getQuantity() * quantity_ratio
     return value_dict
 
-  def _getParentParameters(self, movement, 
+  def _getParentParameters(self, simulation_movement, 
                            **value_delta_dict):
     """
       Get parent movement, and its value delta dict.
     """
-    applied_rule = movement.getParentValue()
+    applied_rule = simulation_movement.getParentValue()
     parent_movement = applied_rule.getParentValue()
     if parent_movement.getPortalType() != "Simulation Movement":
       parent_movement = None
     return parent_movement, value_delta_dict
 
-  def _recursivelySolve(self, movement, **value_delta_dict):
+  def _recursivelySolve(self, simulation_movement, **value_delta_dict):
     """
-    Update value of the current simulation movement, and update his parent
-    movement.
+      Update value of the current simulation movement, and update
+      his parent movement.
     """
-    value_dict = self._generateValueDict(movement, **value_delta_dict)
-    movement.edit(**value_dict)
+    value_dict = self._generateValueDict(simulation_movement, **value_delta_dict)
+    simulation_movement.edit(**value_dict)
     parent_movement, parent_value_delta_dict = \
-                self._getParentParameters(movement, **value_delta_dict)
+                self._getParentParameters(simulation_movement, **value_delta_dict)
     if parent_movement is not None:
       # Modify the parent movement
       self._recursivelySolve(parent_movement, **parent_value_delta_dict)

product/ERP5/TargetSolver/ProfitAndLoss.py

@@ -29,7 +29,6 @@
 
 # from Products.ERP5.Tool.SimulationTool import registerTargetSolver
 from CopyToTarget import CopyToTarget
-from zLOG import LOG
 
 class ProfitAndLoss(CopyToTarget):
   """
@@ -45,13 +44,17 @@ class ProfitAndLoss(CopyToTarget):
       Movement difference as a profit (ie. a quantity coming from nowhere)
       Accumulate into delivered movement
     """
-    LOG('profit and loss called on movement', 0, repr(movement))
     delivery_line = movement.getDeliveryValue()
     delivery_line_quantity = delivery_line.getQuantity()
     if delivery_line_quantity is not None:
       target_quantity = delivery_line_quantity * movement.getDeliveryRatio()
       added_quantity = movement.getQuantity() - target_quantity
-      movement.edit(profit_quantity=added_quantity)
+      movement.edit(profit_quantity = added_quantity)
     delivery = movement.getDeliveryValue()
     if delivery is not None:
-      delivery.activate(after_path_and_method_id=(movement.getPath(), ['immediateReindexObject', 'recursiveImmediateReindexObject'])).edit()
+      delivery.activate(
+        after_path_and_method_id = (
+          movement.getPath(),
+          ['immediateReindexObject', 'recursiveImmediateReindexObject']
+        )
+      ).edit()

product/ERP5/TargetSolver/Redirect.py

@@ -35,7 +35,7 @@ class Redirect(TargetSolver):
     Redirects all simulation movements to new target
   """
 
-  def solve(self, movement, new_target):
+  def solve(self, simulation_movement, new_target):
     """
       Updates all sources and destinations to new values defined 
       in self by mapping 
@@ -46,15 +46,15 @@ class Redirect(TargetSolver):
     """
     for p in ('source', 'destination'):
       for q in ('source', 'destination'):
-        if movement.getProperty(p) == getattr(self, q):
+        if simulation_movement.getProperty(p) == getattr(self, q):
           self.setProperty(p, getattr(self, 'target_%s' % q))
           break
     for p in ('source_section', 'destination_section'):
       for q in ('source_section', 'destination_section'):
-        if movement.getProperty(p) == getattr(self, q):
+        if simulation_movement.getProperty(p) == getattr(self, q):
           self.setProperty(p, getattr(self, 'target_%s' % q))
           break
-    delivery_value = movement.getDeliveryValue() # Get delivery movement
+    delivery_value = simulation_movement.getDeliveryValue() # Get delivery movement
     if delivery_value is not None:
       delivery_value = delivery_value.getDeliveryValue() # Get root delivery
       if delivery_value is not None:

product/ERP5/TargetSolver/SplitAndDefer.py

@@ -42,39 +42,39 @@ class SplitAndDefer(CopyToTarget):
     (excessive qty is not covered)
   """
 
-  def solve(self, movement):
+  def solve(self, simulation_movement):
     """
-      Split a movement and accumulate
+      Split a simulation movement and accumulate
     """
-    movement_quantity = movement.getQuantity()
-    delivery_quantity = movement.getDeliveryQuantity()
-    new_movement_quantity = delivery_quantity * movement.getDeliveryRatio()
+    movement_quantity = simulation_movement.getQuantity()
+    delivery_quantity = simulation_movement.getDeliveryQuantity()
+    new_movement_quantity = delivery_quantity * simulation_movement.getDeliveryRatio()
 
     if movement_quantity > new_movement_quantity:
       split_index = 0
-      new_id = "%s_split_%s" % (movement.getId(), split_index)
-      while getattr(movement.aq_parent, new_id, None) is not None:
+      new_id = "%s_split_%s" % (simulation_movement.getId(), split_index)
+      while getattr(simulation_movement.aq_parent, new_id, None) is not None:
         split_index += 1
-        new_id = "%s_split_%s" % (movement.getId(), split_index)
+        new_id = "%s_split_%s" % (simulation_movement.getId(), split_index)
       # Adopt different dates for defferred movements
-      new_movement = movement.aq_parent.newContent(
+      new_movement = simulation_movement.aq_parent.newContent(
                         portal_type="Simulation Movement",
                         id=new_id,
-                        efficiency=movement.getEfficiency(),
+                        efficiency=simulation_movement.getEfficiency(),
                         start_date=self.start_date,
                         stop_date=self.stop_date,
-                        order=movement.getOrder(),
-                        deliverable=movement.isDeliverable(),
+                        order=simulation_movement.getOrder(),
+                        deliverable=simulation_movement.isDeliverable(),
                         quantity=movement_quantity-new_movement_quantity,
-                        source = movement.getSource(),
-                        destination = movement.getDestination(),
-                        source_section = movement.getSourceSection(),
-                        resource = movement.getResource(),
-                        destination_section = movement.getDestinationSection(),
+                        source = simulation_movement.getSource(),
+                        destination = simulation_movement.getDestination(),
+                        source_section = simulation_movement.getSourceSection(),
+                        resource = simulation_movement.getResource(),
+                        destination_section = simulation_movement.getDestinationSection(),
                         activate_kw=self.activate_kw,
                         **self.additional_parameters
       )
       new_movement.activate(**self.additional_parameters).expand()
-    movement._v_activate_kw = self.activate_kw
-    movement.activate(**self.additional_parameters).expand()
-    CopyToTarget.solve(self, movement)
+    simulation_movement._v_activate_kw = self.activate_kw
+    simulation_movement.activate(**self.additional_parameters).expand()
+    CopyToTarget.solve(self, simulation_movement)

product/ERP5/TargetSolver/SplitQuantity.py

@@ -31,34 +31,46 @@ from CopyToTarget import CopyToTarget
 from zLOG import LOG
 
 class SplitQuantity(CopyToTarget):
+  """
+    Split a simulation movement based on a resource quantity.
+  """
 
-  def solve(self, movement):
+  def solve(self, simulation_movement):
     """
-      Split a movement based on a given quantity
+      From simulation_movement, generate new_movement containing self.quantity
+      resources, of start_date self.start_date and stop_date self.stop_date.
+      
+      movement.quantity is updated
+      
+      XXX incomplete docstring
     """
     split_index = 0
-    new_id = "%s_split_%s" % (movement.getId(), split_index)
-    while getattr(movement.aq_parent, new_id, None) is not None:
+    new_id = "%s_split_%s" % (simulation_movement.getId(), split_index)
+    while getattr(simulation_movement.aq_parent, new_id, None) is not None:
       split_index += 1
-      new_id = "%s_split_%s" % (movement.getId(), split_index)
+      new_id = "%s_split_%s" % (simulation_movement.getId(), split_index)
     # Adopt different dates for defferred movements
-    new_movement = movement.aq_parent.newContent(
-                      portal_type="Simulation Movement",
-                      id=new_id,
-                      efficiency=movement.getEfficiency(),
-                      start_date=self.start_date,
-                      stop_date=self.stop_date,
+    new_movement = simulation_movement.aq_parent.newContent(
+      portal_type = "Simulation Movement",
+      id = new_id,
+      efficiency = simulation_movement.getEfficiency(),
+      start_date = self.start_date,
+      stop_date = self.stop_date,
       # XXX resource
-                      order=movement.getOrder(),
-                      deliverable=movement.isDeliverable(),
-                      quantity=self.quantity,
-                      source = movement.getSource(),
-                      destination = movement.getDestination(),
-                      source_section = movement.getSourceSection(),
-                      destination_section = movement.getDestinationSection(),
-                      activate_kw=self.activate_kw,
+      order = simulation_movement.getOrder(),
+      deliverable = simulation_movement.isDeliverable(),
+      quantity = self.quantity,
+      source = simulation_movement.getSource(),
+      destination = simulation_movement.getDestination(),
+      source_section = simulation_movement.getSourceSection(),
+      destination_section = simulation_movement.getDestinationSection(),
+      activate_kw = self.activate_kw,
       **self.additional_parameters
     )
-    movement._v_activate_kw = self.activate_kw
-    movement.edit(quantity=(movement.getQuantity() - self.quantity) * movement.getDeliveryRatio())
+    simulation_movement._v_activate_kw = self.activate_kw
+    simulation_movement.edit (
+      quantity = (simulation_movement.getQuantity() - self.quantity)
+                 * simulation_movement.getDeliveryRatio()
+    )
+    #XXX: vincent: I don't understand why it's multiplicated.
     return new_movement

product/ERP5/TargetSolver/TargetSolver.py

@@ -44,9 +44,12 @@ class TargetSolver:
 
     - call 'updateNewTarget' on Applied rule
       to update parent target
+
+    This class is the base class for all target solvers.
+    It's virtual due to "solve", which needs to be overloaded.
   """
 
-  def __init__(self, additional_parameters=None,activate_kw=None,**kw):
+  def __init__(self, additional_parameters=None, activate_kw=None, **kw):
     """
       Creates an instance of TargetSolver with parameters
     """
@@ -69,11 +72,14 @@ class TargetSolver:
 
       XXX: maybe we do not need to pass previous_target as parameter
       (since we accumulate it)
+
+      This function must be implemented by the actual solver which derivates
+      from this class.
     """
 
   def solveDelivery(self, delivery):
     """
-      Called in case it is needed for the solving process
+      Solves the whole delivery.
     """
     # Then apply to all movements
     for movement in delivery.getMovementList():
@@ -81,9 +87,9 @@ class TargetSolver:
 
   def solveMovement(self, movement):
     """
-      Called in case it is needed for the solving process
+      Solves a movement.
     """
-    # apply to all movements
+    # Apply to all simulation movements
     simulation_movement_list = movement.getDeliveryRelatedValueList(
                                              portal_type="Simulation Movement")
     solved_movement_list = []
@@ -100,4 +106,3 @@ class TargetSolver:
     # XXX this is not the job of TargetSolver to create new Delivery !
     pass
 
-

product/ERP5/TargetSolver/TransformationSourcingCopyToTarget.py

@@ -33,37 +33,38 @@ class TransformationSourcingCopyToTarget(CopyToTarget):
     Copy values simulation movement as target, and
     recursively solve the sourcing tree.
   """
-  def _generateValueDeltaDict(self, movement):
+  def _generateValueDeltaDict(self, simulation_movement):
     """
       Get interesting value
     """
-    value_dict = CopyToTarget._generateValueDict(self, movement)
+    value_dict = CopyToTarget._generateValueDict(self, simulation_movement)
     value_dict.update({
-      'aggregate_list': movement.getDeliveryValue().getAggregateList(),
+      'aggregate_list':
+        simulation_movement.getDeliveryValue().getAggregateList(),
     })
     return value_dict
 
-  def _generateValueDict(self, movement, aggregate_list=None,
+  def _generateValueDict(self, simulation_movement, aggregate_list = None,
                          **value_delta_dict):
     """
       Generate values to save on movement.
     """
-    value_dict = CopyToTarget._generateValueDict(self, movement,
+    value_dict = CopyToTarget._generateValueDict(self, simulation_movement,
                                                  **value_delta_dict)
     # Modify aggregate_list
     if aggregate_list is not None:
       value_dict['aggregate_list'] = aggregate_list
     return value_dict
 
-  def _getParentParameters(self, movement, 
+  def _getParentParameters(self, simulation_movement, 
                            **value_delta_dict):
     """
       Get parent movement, and its value delta dict.
     """
-    applied_rule = movement.getParentValue()
+    applied_rule = simulation_movement.getParentValue()
     rule = applied_rule.getSpecialiseValue()
     if rule.getPortalType() != "Transformation Sourcing Rule":
       value_delta_dict.pop('aggregate_list', None)
-    return CopyToTarget._getParentParameters(self, movement, 
+    return CopyToTarget._getParentParameters(self, simulation_movement, 
                                              **value_delta_dict)