I was reminded again today how much better your code can be if only pass in those variables you need.

Here’s where I started.

 [self.fbMessenger shareViaAppExtensionEntityData:self.viewModel.data.entityData utmTagsEnabled:utmTagsEnabled];
- (void)shareViaAppExtensionEntityData:(SPTShareEntityData *)entityData utmTagsEnabled:(BOOL)utmTagsEnabled
{
    // create fb messenger share URL
    NSString *facebookAppExtensionURL = entityData.webURI.absoluteString;

    if (utmTagsEnabled) {
        facebookAppExtensionURL = [self facebookAppExtensionURLForEntityDataURL:entityData.webURI.absoluteString];
    }

    // share
    FBSDKShareLinkContent *content = [[FBSDKShareLinkContent alloc] init];
    content.contentURL = [NSURL URLWithString:facebookAppExtensionURL];
    [FBSDKMessageDialog showWithContent:content delegate:nil];
}

But what’s wrong with this method. It’s passing in too much! I don’t need the full entity. Just the string.

My trick to catching this like this is to consciously refactor. Every 10 or 15 mins, or every time I think I am done a feature, go back and review your code.

These are the questions you continuously want to be asking yourself:

  1. Is this the simplest code possible.
  2. Am I only passing into methods that which I need.
  3. Does this read well? Am I going to understand what I have written two weeks from now.
  4. Is each method only doing one thing.

Here’s where I ended up after some more refactoring.

[self.fbMessenger shareEntityURL:entityURL utmTagsEnabled:utmTagsEnabled];
- (void)shareEntityURL:(NSString *)entityURL utmTagsEnabled:(BOOL)utmTagsEnabled
{
    // create URL to share
    NSString *urlToShare = [self urlToShareForEntityURL:entityURL utmTagsEnabled:utmTagsEnabled];

    // share
    FBSDKShareLinkContent *content = [[FBSDKShareLinkContent alloc] init];
    content.contentURL = [NSURL URLWithString:urlToShare];
    [FBSDKMessageDialog showWithContent:content delegate:nil];
}

- (NSString *)urlToShareForEntityURL:(NSString *)entityURL utmTagsEnabled:(BOOL)utmTagsEnabled
{
    NSString *urlToShare = entityURL;

    if (utmTagsEnabled) {
        urlToShare = [self addUTMTagsToURL:entityURL];
    }

    return urlToShare;
}

By pulling out the if statement into a method dedicated to building the URL, I get away from the old method from doing x2 things instead of one.

Now it’s much easier to test. I can write a test that adds UTM tags, and another that doesn’t.

Advertisements