Software Craftsmanship

Clean vs Messy Code

April 19, 2016

What is clean code?

  • Reads like well-written prose
  • Clarifies the designer's intent
  • Sustains simple and direct modules
  • Preserves straightforward logical flow
  • Employs efficient abstractions
  • Fosters maintenance and enhancement
  • Provides one way to do one thing
  • Presents a clear and minimal API
  • Includes unit and acceptance tests
  • Requires minimal dependencies

Clean code is the reward for elegant design, planning, and execution.

http://xkcd.com/844/

What is messy code?

  • Smells bad
  • Follows anti-patterns
  • Includes spaghetti code
  • Contains duplication
  • Comprises complexity
  • Obscures intent
  • Complicates logical flow
  • Perpetuates inconsistencies
  • Becomes unmaintainable

Messy code is a symptom of poor design or some other problem.

@implementation LoginVC

//###########################
#pragma Life Cycle

- (void)viewDidLoad
{
    [super viewDidLoad];
    
    //Let the NavigationController know the rewind point
    [(ApplicationNavigationController*)self.navigationController setLoginViewController:self];

    //Set the current Error
    self.currentError = nil;
    
    //Prepare UI
    self.titleLabel.font = [UIFont c1BoldHeader];
    self.rememberMeCheckBoxLabel.font = [UIFont c1];
    
    [self.usernameTextField setTextColor:[UIColor blackColor]];
    [self.usernameTextField setFont:[UIFont c1ButtonTitle]];
    
    [self.passwordTextField setTextColor:[UIColor blackColor]];
    [self.passwordTextField setFont:[UIFont c1ButtonTitle]];
}

-(void)viewDidAppear:(BOOL)animated
{
    [super viewDidAppear:YES];
    
    [[NSNotificationCenter defaultCenter] addObserver:self
                                             selector:@selector(keyboardFrameDidChange:)
                                                 name:UIKeyboardWillChangeFrameNotification
                                               object:nil];

    //Configure controls based up login manager
    LoginManager *loginManager = [LoginManager sharedInstance];
    
    if (loginManager.rememberMe)
    {
        self.usernameTextField.text= [loginManager username];
        self.usernameTextField.enabled = NO;
        self.usernameTextField.text = [loginManager maskedUsername];
        [self.passwordTextField becomeFirstResponder];
    }
    else
    {
        [self.usernameTextField becomeFirstResponder];
        [self.BottomHalf updateConstraints];
        [self.BottomHalf layoutSubviews];
    }
    
    if (loginManager.rememberMe) {
        self.rememberMeCheckBoxSelected.hidden = false;
        self.rememberMeCheckBoxUnselected.hidden = true;
        self.rememberUserName = true;
    }
    else
    {
        self.rememberMeCheckBoxSelected.hidden = true;
        self.rememberMeCheckBoxUnselected.hidden = false;
        self.rememberUserName = false;
    }
}

-(void)viewWillDisappear:(BOOL)animated
{
    [super viewWillDisappear:animated];
    
    [self clearTextFields];
}

//###########################
#pragma Keyboard

//Adjust the constraint for the login button taking into account the size of the keyboard
- (void)keyboardFrameDidChange:(NSNotification *)notification
{
    CGRect keyboardBeginFrame = [[notification userInfo][UIKeyboardFrameBeginUserInfoKey] CGRectValue];
    CGRect keyboardFrameBegin = [self.view convertRect:keyboardBeginFrame toView:nil];

    CGRect keyboardEndFrame = [[notification userInfo][UIKeyboardFrameEndUserInfoKey] CGRectValue];
    CGRect keyboardFrameEnd = [self.view convertRect:keyboardEndFrame toView:nil];
    
    if (keyboardFrameBegin.origin.y > keyboardFrameEnd.origin.y)
    { //Keyboard opening

        [self.ButtonDistanceFromBottomConstraint setConstant:keyboardFrameEnd.size.height + 8];
        
        if (self.BottomHalf.bounds.size.height - keyboardFrameEnd.size.height - 16 > 32)
        {
            //Button can fit within the bottom half of the screen
            [self.ButtonHeightConstraint setConstant:(self.BottomHalf.bounds.size.height - keyboardFrameEnd.size.height) - 16];
        }
        else
        {
            //Button does not fit so lock to a height of 32
            [self.ButtonHeightConstraint setConstant:32];
        }

    }
    else
    { //Keyboard closing
        [self.ButtonDistanceFromBottomConstraint setConstant:8];
        [self.ButtonHeightConstraint setConstant:32];
    }

    [UIView animateWithDuration:[[notification userInfo][UIKeyboardAnimationDurationUserInfoKey] doubleValue]
        animations:^{
            [self.loginButton layoutIfNeeded];
        }];

    [UIView commitAnimations];
}

- (BOOL)textFieldShouldReturn:(UITextField *)textField
{
    if (textField == self.usernameTextField)
    {
        if ([textField.text length] > 0)
        {
            [self.passwordTextField becomeFirstResponder];
        }
    }
    else if (textField == self.passwordTextField)
    {
        [self performLogin];
    }
    return NO;
}

- (void)clearTextFields
{
    LoginManager *loginManager = [LoginManager sharedInstance];
    
    self.passwordTextField.text = @"";
    [loginManager setPassword:@""];
    
    if (self.rememberMeCheckBoxSelected.hidden && !loginManager.rememberMeUsed)
    {
        self.usernameTextField.text = @"";
        [self.usernameTextField becomeFirstResponder];
    }
    else if (!self.rememberMeCheckBoxSelected.hidden && !loginManager.rememberMeUsed)
    {
        loginManager.rememberMe = NO;
    }
    else if([loginManager maskedUsername] != nil)
    {
        self.usernameTextField.text = [loginManager maskedUsername];
        [self.passwordTextField becomeFirstResponder];
    }
}

//###########################
#pragma Login Events

- (IBAction)LoginAction:(id)sender
{
    
    if ([self.usernameTextField.text length] > 0)
    {
        if ([self.passwordTextField.text length] > 0)
        {
            [self performLogin];
        }
        else
        {
            [self.passwordTextField becomeFirstResponder];
        }
    }
    else
    {
        [self.usernameTextField becomeFirstResponder];
    }
}

- (void)performLogin
{
    LoginManager *loginManager = [LoginManager sharedInstance];
    
    if (self.usernameTextField.enabled)
    {
        [loginManager setUsername:self.usernameTextField.text];
    }
    loginManager.password = self.passwordTextField.text;
    loginManager.rememberMe = self.rememberUserName;
    
    if (loginManager)
    {
        [[UIApplication sharedApplication] beginIgnoringInteractionEvents];
        
        __weak LoginVC *weakself = self;
        
        // this is the block to execute when login is completed
        self.loginCompletion = ^()
        {
            [C1DataManager sharedManager].userLoggedInSuccessfully = TRUE;
            
            //TODO - Remove, hard coded for testing
            if ([weakself.usernameTextField.text isEqualToString:@"2xTablet144"])
            {
                [weakself segueToMFA];
            }
            else
            {
                [weakself segueViaNavigationController];
            }
        };
        
        loginManager.loginSuccessBlock = ^(LoginManager *mgr)
        {
            mgr.loginSession.userName = [mgr username];
            weakself.loginCompletion();
        };
        
        loginManager.loginFailureBlock = ^(LoginManager *mgr, NSError *error)
        {
            [self generatePopupFromError:error];
        };
        
        loginManager.loginMFABlock = ^(LoginManager *mgr)
        {            
            [[UIApplication sharedApplication] endIgnoringInteractionEvents];
        };
        
        loginManager.loginMFACollectBlock = ^(LoginManager *mgr)
        {
            [[UIApplication sharedApplication] endIgnoringInteractionEvents];
        };
        
        [loginManager performLogin];
    }
}

- (void)generatePopupFromError:(NSError*)error
{
    self.currentError = error;
    
    UIAlertView *alert;
    
    //Error generating from from successfully connecting to the login server
    if ([error.domain isEqual:@"com.capitalone.core.eapi"])
    {
        //[2]	(null)	@"C1SSErrorButtonTypes" : @"2 objects" C1SSErrorButtonType
        NSArray *button =  error.userInfo[C1SSErrorUserInfoKeyButtonTextLocalizationKey];
        //We need to handle the recovery option
        if (button.count == 2)
        {
            alert = [[UIAlertView alloc]initWithTitle:error.localizedDescription
                                              message:error.localizedRecoverySuggestion
                                             delegate:self
                                    cancelButtonTitle:button[0]
                                    otherButtonTitles:button[1], nil];
        }
        else if (button.count == 0)
        {
            alert = [[UIAlertView alloc]initWithTitle:error.localizedDescription
                                              message:error.localizedRecoverySuggestion
                                             delegate:self
                                    cancelButtonTitle:@"OK"
                                    otherButtonTitles:nil];
        }
        else
        {
            alert = [[UIAlertView alloc]initWithTitle:error.localizedDescription
                                              message:error.localizedRecoverySuggestion
                                             delegate:self
                                    cancelButtonTitle:button[0]
                                    otherButtonTitles:nil];
        }
    }
    //Error from not being able to hit the login server
    else
    {
        alert = [[UIAlertView alloc]initWithTitle:@"Network Error"
                                          message:@"Unable to connect to COF"
                                         delegate:self
                                cancelButtonTitle:@"Ok"
                                otherButtonTitles:nil];
    }

    [alert show];
    
    [self clearTextFields];
    
    [[UIApplication sharedApplication] endIgnoringInteractionEvents];
}

- (void)alertView:(UIAlertView *)alertView clickedButtonAtIndex:(NSInteger)buttonIndex
{
    assert(self.currentError != nil);
    
    if (buttonIndex != 1) { return; }
    
    NSDictionary* errorAttributes = (NSDictionary*)(self.currentError.userInfo[C1SSErrorUserInfoKeyAttributes][0]);
    
    NSNumber* button = (NSNumber*)(self.currentError.userInfo[C1SSErrorUserInfoKeyButtonTypes][1]);

    switch ([button unsignedIntegerValue])
    {
        case C1SSErrorButtonTypeNone:
            break;
        case C1SSErrorButtonTypeOK:
            break;
        case C1SSErrorButtonTypeDismiss:
            break;
        case C1SSErrorButtonTypeCall:
            [[UIApplication sharedApplication] openURL:
                    [NSURL URLWithString:[self buildPhoneNumberString:errorAttributes[C1SSErrorAttributesKeyPhone]]]];
            break;
        case C1SSErrorButtonTypeGo:
            [[UIApplication sharedApplication] openURL:
                    [NSURL URLWithString:errorAttributes[C1SSErrorAttributesKeyURL]]];
            break;
        default:
            break;        
    }
    
    self.currentError = nil;
}

- (NSString*)buildPhoneNumberString:(NSString*)phoneNumber
{
    phoneNumber = [NSString stringWithFormat:@"tel:%@", phoneNumber];
    phoneNumber = [phoneNumber stringByReplacingOccurrencesOfString:@"(" withString:@""];
    phoneNumber = [phoneNumber stringByReplacingOccurrencesOfString:@")" withString:@""];
    phoneNumber = [phoneNumber stringByReplacingOccurrencesOfString:@"-" withString:@""];
    phoneNumber = [phoneNumber stringByReplacingOccurrencesOfString:@" " withString:@""];
    return phoneNumber;
}

//###########################
#pragma Navigation

- (void)prepareForSegue:(UIStoryboardSegue *)segue sender:(id)sender
{
    [[UIApplication sharedApplication] endIgnoringInteractionEvents];
}

- (void)segueViaNavigationController
{
    [(ApplicationNavigationController*)self.navigationController segueFromLoginToLegalOrAccountDetails:self];
}

- (void)segueToMFA
{
    [self performSegueWithIdentifier:@"toMFA" sender:self];
}

//###########################
#pragma Actions

- (IBAction)RememberUsernameToggle:(id)sender
{
    self.rememberUserName = !(self.rememberUserName);
    
    LoginManager *loginManager = [LoginManager sharedInstance];
    loginManager.rememberMe = self.rememberUserName;
    
    if (self.rememberUserName)
    {
        self.rememberMeCheckBoxSelected.hidden = false;
        self.rememberMeCheckBoxUnselected.hidden = true;
    }
    else
    {
        self.rememberMeCheckBoxSelected.hidden = true;
        self.rememberMeCheckBoxUnselected.hidden = false;
    
        if(!self.usernameTextField.enabled){
            self.usernameTextField.enabled = YES;
            self.usernameTextField.text = @"";
            self.passwordTextField.text = @"";
            [self.usernameTextField becomeFirstResponder];
        }
    }
}

@end
if (appObject.isDsclsrOnlineRequiredInd() || appObject.isDsclsrCreditScoreRequiredInd()) {
    if (appObject.isDsclsrOnlineRequiredInd()) {
        // Pull account disclosure
        setDisclosureVariableFromContent(AOD, aodVariableReplacer,
            appObject.getDecisionedOfferID(), model, evalContext);
    }
            
    if (appObject.isDsclsrCreditScoreRequiredInd() && appObject.getMinCreditScore() != null 
            && appObject.getMinCreditScore().getBureauCode()!=null) {
        // Pull credit disclosure
        if(appObject.getMinCreditScore().getBureauCode().equals("")){
            setDisclosureVariableFromContent(CSD, csdVariableReplacer,
               NO_HIT_KEY , model, evalContext);
        }else{
            setDisclosureVariableFromContent(CSD, csdVariableReplacer,
                appObject.getMinCreditScore().getBureauCode() , model, evalContext);
        }
    }
}

if (appObject.getApplicant() != null && appObject.getApplicant().get(0) != null ) { 
    userInfo.setFirstName(appObject.getApplicant().get(0).getFirstName());
    userInfo.setLastName(appObject.getApplicant().get(0).getLastName());			
    userInfo.setLangCode(appObject.getApplicant().get(0).getLangCode());
    userInfo.setSsoId(null);
    userInfo.setUserId(appObject.getApplicant().get(0).getCustomerID());
			
    if (appObject.getApplicant().get(0).getEmailAddresses() != null  &&
            appObject.getApplicant().get(0).getEmailAddresses().size() > 0) {	
        userInfo.setEmailAddress(appObject.getApplicant().get(0).getEmailAddresses().get(0));
    }	
}

The Perils of Messy Code

Mess builds, productivity of team decreases

Under pressure, the team makes more messes

Entropy - The amount of disorder in a system. When disorder increases in software, it is called "code rot".

The only way to develop quickly and meet deadlines is to keep the code as clean as possible at all times.

Code Smells

"A code smell is a surface indication that usually corresponds to a deeper problem in the system".

- Martin Fowler

Bloaters

  • Long Method
  • Large Class
  • Primitive Obsession
    • ​Using multiple primitives to represent concepts instead of an object.
  • Long Parameter List
  • Data Clumps
    • ​Data that always appears together and in the same locations 

OO Abusers

  • Switch Statements
  • Temporary Field
  • Refused Bequest
  • Alternative Classes with Different Interfaces

Change Preventers

  • Divergent Change
  • Shotgun Surgery 
  • Parallel Inheritance Hierarchies

The Dispensables

  • Lazy class
  • Data class 
  • Duplicate Code
  • Dead Code
  • Speculative Generality

The Couplers

  • Feature Envy
  • Inappropriate Intimacy
  • Message Chains 
  • Middle Man

https://gist.github.com/ClassicThunder/2132ccc8154de4d13fb4034973880ce5

Software Craftsmanship - Clean vs Messy Code

By cof_softwarecraftsmanship

Software Craftsmanship - Clean vs Messy Code

2016 - Day 1.3

  • 481