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:
- Is this the simplest code possible.
- Am I only passing into methods that which I need.
- Does this read well? Am I going to understand what I have written two weeks from now.
- 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.
Leave a Reply