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;