CODEWITHSUNDEEP
oopdesign-patterns
Positonic
Positonic

Reputation: 9411

Pattern or recommended refactoring for method

I've written a method that looks like this:

public TimeSlotList processTimeSlots (DateTime startDT, DateTime endDT, string bookingType, IList<Booking> normalBookings, GCalBookings GCalBookings, List<DateTime> otherApiBookings) {
{

..... common process code ......

    while (utcTimeSlotStart < endDT)
    {

        if (bookingType == "x")
        {
            //process normal bookings using IList<Booking> normalBookings

        }
        else if (bookingType == "y") {

            //process google call bookings using GCalBookings GCalBookings
        }
        else if (bookingType == "z" {

            //process other apibookings using List<DateTime> otherApiBookings

        }

    }

}

So I'm calling this from 3 different places, each time passing a different booking type, and each case passing the bookings I'm interested in processing, as well as 2 empty objects that aren't used for this booking type.

I'm not able to get bookings all into the same datatype, which would make this easier and each booking type needs to be processed differently, so I'm not sure how I can improve this.

Upvotes: 2

Views: 93

Answers (4)

tcarvin
tcarvin

Reputation: 10855

You said

"I'm not able to get bookings all into the same datatype"

and I say "why not"?

If at the lowest level these booking objects inherit from some other type then use composition and have those objects as members of three subclasses of an AbstractBooking class (NormalBooking, GoogleBooking, OtherBooking).

At this point, as I think you know based on your post, you can have the custom booking code in each subclass.

Upvotes: 1

ctrl-alt-delor
ctrl-alt-delor

Reputation: 7735

Make the 3 data structures into booking classes, all inheriting from a common parent that has the interface:

public TimeSlotList processTimeSlots (DateTime startDT, DateTime endDT)

or

see similar but better explained answer https://stackoverflow.com/a/11154505/537980

or

write 3 routines

public TimeSlotList processTimeSlots_Normal (DateTime startDT, DateTime endDT, string bookingType, IList<Booking> normalBookings) {
{
    common_process_code(local_vars);

    //process normal bookings using IList<Booking> normalBookings
}

etc.

or

make the original private, and write 3 public wrappers (this is less scalable).

Upvotes: 1

Jord&#227;o
Jord&#227;o

Reputation: 56467

If you can change the calling code, I would suggest creating separate classes for each different booking processor, with a common base class:

public abstract class BookingProcessor<TBookings> {
  public TimeSlotList ProcessTimeSlots(DateTime startDT, DateTime endDT, TBookings bookings) {
    // ..... common process code ......
    while (utcTimeSlotStart < endDT) {
      ProcessTimeSlots(utcTimeSlotStart, bookings);
      // ...
    }
  }
  protected abstract ProcessTimeSlots(DateTime utcTimeSlotStart, TBookings bookings);
}

The specific booking processors follow this pattern:

public class NormalBookingsProcessor : BookingProcessor<IList<Booking>> {
  protected override ProcessTimeSlots(DateTime utcTimeSlotStart, IList<Booking> bookings) {
    // process normal bookings using IList<Booking> normalBookings 
  }
}

This way, the code to process one type of booking is separate from the others, which is better for testability and maintainability.

Upvotes: 3

sll
sll

Reputation: 62484

// Just do extract method for each booking type processign logic
// also use typed booking type (enum) to leverage switch() conditional flow
// rather than multiple nested if/else

public enum BookingType
{
    X,
    Y,
    Z
}

private void EntryPoint(DateTime startDT, 
    DateTime endDT, 
    string bookingType, 
    IList<Booking> normalBookings, 
    GCalBookings GCalBookings, 
    List<DateTime> otherApiBookings)
{

    // common logic

    BookingType type = (BookingType)Enum.Parse(bookingType.ToUpperInvariant(), typeof(BookingType));

    switch (type)
    {
        case BookingType.X:
            ProcessNormalBookings(startDt, endDt, normalBookings);
            break;
        case BookingType.Y:
            ProcessGcCalBookings(startDt, endDt, GCalBookings);
            break;
        case BookingType.Z:
            ProcessOtherApiBookings(startDt, endDt, otherApiBookings);
            break;
    }
}

Upvotes: 1

Related Questions