Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have the below code which is still under development - but I'm trying to get myself to find the ideal way to move forward.

public interface IMemberSubscriptionService
    void SelectSubscription(IMember member, IMemberSubscriptionTypePrice subscription, IOrder linkedOrderPaidWith);
    IMemberSubscriptionRenewal GetCurrentApplicableRenewal(IMember member);


public class MemberSubscriptionService : IMemberSubscriptionService
    private readonly IMemberSubscriptionRenewalService memberSubscriptionRenewalService;
    public MemberSubscriptionService(IMemberSubscriptionRenewalService memberSubscriptionRenewalService)
        this.memberSubscriptionRenewalService = memberSubscriptionRenewalService;

    public void SelectSubscription(IMember member, IMemberSubscriptionTypePrice subscription, IOrder linkedOrderPaidWith)
        var renewal = GetCurrentApplicableRenewal(member);
        if (renewal == null || !object.Equals(renewal.Subscription, subscription))
        {//if they are different

            if (renewal != null)
            var newRenewal = createFirstSubscriptionRenewal(member, subscription, linkedOrderPaidWith);


    private IMemberSubscriptionRenewal createFirstSubscriptionRenewal(IMember member, IMemberSubscriptionTypePrice subscription, IOrder linkedOrder)
        IMemberSubscriptionRenewal renewal = member.SubscriptionRenewals.CreateNewItem();
        renewal.StartDate = CS.General_v3.Util.Date.Now.Date;
        renewal.EndDate = CS.General_v3.Util.Date.GetEndOfDay(PayMammoth.Connector.Util.GeneralUtil.GetDateTimeAfterIncrementingBillingPeriod(renewal.StartDate,
        renewal.LinkedOrder = linkedOrder;
        return renewal;


    private void updateRenewalDateToEndAsOfYesterday(IMemberSubscriptionRenewal renewal)
        renewal.EndDate = CS.General_v3.Util.Date.Now.AddDays(-1);


    #region IMemberSubscriptionService Members

    public IMemberSubscriptionRenewal GetCurrentApplicableRenewal(IMember member)
        throw new NotImplementedException();


What this code should do is 'select a subscription' (SelectSubscription) for a particular member. A member has a list of SubscriptionRenewals which basically contain information for each period of a subscription, e.g taking a monthly subscription would have a SubscriptionRenewal for Jan 2012, another for Feb 2012, etc.

The SelectSubscription should get the current subscription, and if the subscription is to be changed, the existing renewal is updated to end as of yesterday, and the new subscription starts from today, for the total period (e.g changing from monthly to yearly). If you have a SubscriptionRenewal from 1 - 29 Feb 2012, and today is 15 Feb and you call 'SelectSubscription' with a yearly interval, this should change the existing SubscriptionRenewal to end on 14Feb 2012, and create a new one from 15 Feb 2012 till 14 Feb 2013 for this particular case.

This is just to give you an idea of the functionality. I need to create the respective unit tests for such a functionality, and came out with several scenarios, as per below:

  • if subscription is the same as the current, do nothing.
  • if a current subscription renewal exists, it is updated to end as of yesterday
  • test that SelectSubscription creates new renewal is created, based on today and the subscription

My main issue is on how to structure the code for unit-testing. Should you create the GetCurrentApplicableRenewal as part of the same service? This way, I cannot simply mock that functionality, when I am testing out a portion of the SelectSubscription. Does it make sense to take that functionality out into a different interface/class, so that it can be mocked?

share|improve this question

I see two options:

  1. Test/implement GetCurrentApplicableRenewal first.
  2. Implement the code you need (which will likely go into GetCurrentApplicableRenewal) in your SelectSubscription method now, imeplement/test GetCurrentApplicableRenewal next, and refactor it to use the GetCurrentApplicableRenewal method later.

The latter allows you to implement and define tests for both methods in isolation from one another, only refactoring when you have a full set of tests. With these tests, you can safely validate whether refactoring SelectSubscription can safely switch to using GetCurrentApplicableRenewal or not.

The former way will save you some time, and as long as you don't skip any tests for SelectSubscription, you will probably be OK. The downside is that you have to context-switch to GetCurrentApplicableRenewal.

share|improve this answer
So you would leave it in the same service? Another problem I find this way is that in order to test SelectSubscription, I must also create the data in such a way that is correct also for GetCurrentApplicableRenewal. If I could mock that functionality, I could just make it return whatever renewal I want as SelectSubscription is based on what renewal is returned, so I can more easily test out certain functionality. – Karl Cassar Dec 3 '12 at 11:55

Your Answer


By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.