killbill-memoizeit

Details

diff --git a/entitlement/src/main/java/com/ning/billing/entitlement/api/user/PrivateFields.java b/entitlement/src/main/java/com/ning/billing/entitlement/api/user/PrivateFields.java
index ecd10eb..4c48164 100644
--- a/entitlement/src/main/java/com/ning/billing/entitlement/api/user/PrivateFields.java
+++ b/entitlement/src/main/java/com/ning/billing/entitlement/api/user/PrivateFields.java
@@ -18,6 +18,11 @@ package com.ning.billing.entitlement.api.user;
 
 public class PrivateFields implements IPrivateFields {
 
+    /**
+     * It seems lots of various classes extend this as a means of tacking random attributes
+     * onto data objects, but it doesn't actually do anything. Why is this super class in use?
+     */
+
     public PrivateFields() {
     }
 
diff --git a/entitlement/src/main/java/com/ning/billing/entitlement/api/user/Subscription.java b/entitlement/src/main/java/com/ning/billing/entitlement/api/user/Subscription.java
index dcecb92..047fd40 100644
--- a/entitlement/src/main/java/com/ning/billing/entitlement/api/user/Subscription.java
+++ b/entitlement/src/main/java/com/ning/billing/entitlement/api/user/Subscription.java
@@ -68,10 +68,17 @@ public class Subscription extends PrivateFields  implements ISubscription {
 
     public Subscription(SubscriptionBuilder builder, boolean rebuildTransition) {
         super();
+        
+        /**
+         * Why are these found via static lookup rather than passed in via DI? 
+         * See http://martinfowler.com/articles/injection.html for explanation of
+         * why DI is your friend. -brianm
+         */
         this.clock = InjectorMagic.getClock();
         this.dao = InjectorMagic.getEntitlementDao();
         this.catalog = InjectorMagic.getCatlog();
         this.planAligner = InjectorMagic.getPlanAligner();
+        
         this.id = builder.getId();
         this.bundleId = builder.getBundleId();
         this.startDate = builder.getStartDate();
@@ -191,6 +198,12 @@ public class Subscription extends PrivateFields  implements ISubscription {
         if (nextPhaseEvent != null) {
             uncancelEvents.add(nextPhaseEvent);
         }
+        
+        /**
+         * I think you might be better of disentangling state storage from business logic. 
+         * This happens in a number of places as well as here (such as the subsequent call to
+         * rebuildTransitions() so assume this comment applies at all such places :-) -brianm
+         */
         dao.uncancelSubscription(id, uncancelEvents);
         rebuildTransitions();
     }
@@ -426,7 +439,6 @@ public class Subscription extends PrivateFields  implements ISubscription {
     }
 
     private void rebuildTransitions() {
-
         List<IEvent> events = dao.getEventsForSubscription(id);
         if (events == null) {
             return;