How Should I Move Forward With Refactoring This Code?

71 views Asked by At

So I'm working on a side-project web-application that lets you track packages from multiple companies on one website. i.e. you can enter any tracking number and the server will check the format of it and spit out the appropriate tracking information.

Click here to see my file directory!

I've decided to move forward with a strategy pattern, and a facade, because I think it would serve the purpose of the application well.

Above is my current repo.

The structure would be:

Client -> DeliveryFacade -> DeliveryController -> TrackingInterface -> Various Company API's (FedEx, CanadaPost, UPS).

I'm currently working on DeliveryController, and encountered some smelly code, as I was working on it.

This is it:

export default class DeliveryController{

    /** Checks the format of tracking number to determine the shipping company it
     *  has been given. If the tracking number is able to be validated, then return
     *  the correct shipping company. If it is not able to be validated (not a string), return
     *  null.
     *
     *  @params trackingNumber  : string
     *  @return shippingCompany : string
     *
     *  e.g. returns "Fedex" , "CanadaPost", "UPS"
     *  */
    public checkFormat(trackingNumber : string) : string{

        if (typeof trackingNumber == "string" || trackingNumber instanceof String) {

            // CanadaPost
            if (/{regex}/.test(trackingNumber)){
                return "CanadaPost";
            }

            // FedEx
            if (/{regex}/.test(trackingNumber)) {
                return "FedEx";
            }
            // UPS
            if (/{regex}/.test(trackingNumber)) {
                return "UPS";
            }
        }
        else return null;
    }

    /** Processes a tracking number and returns a deliveryInfo.
     *  @param trackingNumber
     *  @return deliveryInfo
     */

    public process(trackingNumber : string) : deliveryInfo{
        var company = this.checkFormat(trackingNumber);
        switch (company){
            case "CanadaPost":
                break;
            case "FedEx":
                break;
            case "UPS":
                break;
        }
        return /*some deliveryInfo*/;
    }

My question is this: is there some way that I can eliminate the obvious repetition of each strategy?

If I want to add a new tracking number format from a new type of company, I have to add a new class in my tracking strategy folder, and then I have to add the cases to checkFormat and process (which is obviously unfinished).

Is there some form of abstraction that I can use to avoid this?

1

There are 1 answers

2
alek kowalczyk On BEST ANSWER

You forgot to mention that you are using TypeScript.

I would do something like that:

interface IDeliveryCompany {
     tackingNumberMatch(trackingNumber: string): boolean;
     process(): void;
}

class UPSDeliveryCompany implements IDeliveryCompany {

     trackingNumberMatch(trackingNumber: string): boolean {
         return /{regex}/.test(trackingNumber);
     }

     process(): void {
          // ... do the processing
     }
}

// ... similar other delivery companies ...

class DeliveryCompanyFactory {

    private _companies: Array<IDeliveryCompany>;

    constructor() {
        _companies = [
            new UPSDeliveryCompany(),
            new CanadaPostDeliveryCompany(),
            new FedExDeliveryCompany()
        ];
    }

    getCompany(trackingNumber: string): IDeliveryCompany {
        return this._companies.find((c) => c.trackingNumberMatch(trackingNumber));
    }
}

/// example usage:
const factory = new DeliveryCompanyFactory();
factory.getCompany("some_tracking_number").process();

That way, when you need to add a new delivery company, you will have to create a new class for it, which will encapsulate all the logic of that delivery company, and add a instance of it to the DeliveryCompanyFactory.

If you have some logic which is common for all delivery companies, then you can replace the IDeliveryCompany interface with a DeliveryCompany base class which will have the common logic.