DRY – Don’t Repeat Yourself

One of the most common principles of development that I hear is DRY. Don’t Repeat Yourself. Why is this principle so important and how do people break it? I mean, it should be common sense, right? Don’t repeat code. Don’t copy paste. I will repeat that last sentence. Don’t copy paste. If you have to copy paste your code all the time you should think about: can I change this code so I can reuse it?

Real Code vs. Test Code

As much as it pains me to admit, we’re not saints; especially when it comes to unit tests. We don’t treat our test code as production code. You’ll probably find stacks of examples of under engineering and copy paste in your unit test projects. Why? Because we’re lazy and we usually don’t want to overcomplicate our test code. Just get it done. It’s just a unit test, right? Wrong. I’ll talk about this more in my next post.

Take for example the following:

import * as React from 'react';
import { mount } from 'enzyme';

import * as PayoutMethodContainer from './PayoutMethodContainer';

type IPayoutMethodContainerProps = PayoutMethodContainer.IPayoutMethodContainerProps;

describe('<PayoutMethodContainer />', () => {

    const onChange = () => {};

    it('should render properly', () => {

        const wrapper = mount<IPayoutMethodContainerProps, void>(
            <PayoutMethodContainer.default
                payoutSubtitle=""
                directDepositText=""
                ifYouAcceptCreditCardsAtYourPropertiesText=""
                paypalDepositText=""
                payoutTitle=""
                pleaseSelectPayoutMethodText=""
                selectedPayoutMethod={1}
                usingOurSecureSystemText=""
                virtualCreditCardText=""
                youWillBePromptedToEnterText=""
                hasError={false}
                onChange={onChange}
                isPaypalAvailable={true}
                isVirtualCcAvailable={true}
                visible={true} 
            />,             
         );

        expect(wrapper.find('Radio')).toHaveLength(3);
    });

    it('should not render if visible false', () => {

        const wrapper = mount<IPayoutMethodContainerProps, void>(
            <PayoutMethodContainer.default
                payoutSubtitle=""
                directDepositText=""
                ifYouAcceptCreditCardsAtYourPropertiesText=""
                paypalDepositText=""
                payoutTitle=""
                pleaseSelectPayoutMethodText=""
                selectedPayoutMethod={1}
                usingOurSecureSystemText=""
                virtualCreditCardText=""
                youWillBePromptedToEnterText=""
                hasError={false}
                onChange={onChange}
                isPaypalAvailable={true}
                isVirtualCcAvailable={true}
                visible={false} 
            />,
         );

        expect(wrapper.find('Radio')).toHaveLength(0);
    });

    it('should render properly without paypal', () => {

        const wrapper = mount<IPayoutMethodContainerProps, void>(
            <PayoutMethodContainer.default
                payoutSubtitle=""
                directDepositText=""
                ifYouAcceptCreditCardsAtYourPropertiesText=""
                paypalDepositText=""
                payoutTitle=""
                pleaseSelectPayoutMethodText=""
                selectedPayoutMethod={1}
                usingOurSecureSystemText=""
                virtualCreditCardText=""
                youWillBePromptedToEnterText=""
                hasError={false}
                onChange={onChange}
                isPaypalAvailable={false}
                isVirtualCcAvailable={true}
                visible={true} 
            />,
         );

        expect(wrapper.find('Radio')).toHaveLength(2);
    });

    it('should trigger on change when radio button clicked', () => {

        let onChangeWasCalled = false;

        const onChangeCall = () => onChangeWasCalled = true;

        const wrapper = mount<IPayoutMethodContainerProps, void>(
            <PayoutMethodContainer.default
                payoutSubtitle=""
                directDepositText=""
                ifYouAcceptCreditCardsAtYourPropertiesText=""
                paypalDepositText=""
                payoutTitle=""
                pleaseSelectPayoutMethodText=""
                selectedPayoutMethod={1}
                usingOurSecureSystemText=""
                virtualCreditCardText=""
                youWillBePromptedToEnterText=""
                hasError={false}
                onChange={onChangeCall}
                isPaypalAvailable={true}
                isVirtualCcAvailable={true}
                visible={true} 
            />,
         );

        wrapper.find('Radio input').at(0).simulate('change', { target: { checked: true } });

        expect(onChangeWasCalled).toBeTruthy();
    });

    it('should not render Virtual credit card', () => {
        const wrapper = mount<IPayoutMethodContainerProps, void>(
            <PayoutMethodContainer.default
                payoutSubtitle=""
                directDepositText=""
                ifYouAcceptCreditCardsAtYourPropertiesText=""
                paypalDepositText=""
                payoutTitle=""
                pleaseSelectPayoutMethodText=""
                selectedPayoutMethod={1}
                usingOurSecureSystemText=""
                virtualCreditCardText=""
                youWillBePromptedToEnterText=""
                hasError={false}
                onChange={onChange}
                isPaypalAvailable={true}
                isVirtualCcAvailable={false}
                visible={true} 
            />,
        );

        expect(wrapper.find('.virtual-credit-card-radio')).toHaveLength(0);
    });

    it('should render Virtual credit card', () => {
        const wrapper = mount<IPayoutMethodContainerProps, void>(
            <PayoutMethodContainer.default
                payoutSubtitle=""
                directDepositText=""
                ifYouAcceptCreditCardsAtYourPropertiesText=""
                paypalDepositText=""
                payoutTitle=""
                pleaseSelectPayoutMethodText=""
                selectedPayoutMethod={1}
                usingOurSecureSystemText=""
                virtualCreditCardText=""
                youWillBePromptedToEnterText=""
                hasError={false}
                onChange={onChange}
                isPaypalAvailable={true}
                isVirtualCcAvailable={true}
                visible={true} 
            />,
        );

        expect(wrapper.find('.virtual-credit-card-radio').hostNodes()).toHaveLength(1);
    });
});

Just look at how much repeated-code there is. It’s crazy. Seriously, this is something we wrote when we were first starting out with React. We thought we were the bees-knees when all we were was just getting by. Now, let’s see the same tests – cleaned up and a few tests we missed added. Code coverage++.

import * as React from 'react';
import { shallow } from 'enzyme';
import PayoutMethodContainer, { IPayoutMethodContainerProps } from './PayoutMethodContainer';
import RadioButton from './RadioButton';

describe('<PayoutMethodContainer />', () => {

    const props: IPayoutMethodContainerProps = {
        payoutSubtitle: '',
        directDepositText: '',
        ifYouAcceptCreditCardsAtYourPropertiesText: '',
        paypalDepositText: '',
        payoutTitle: '',
        pleaseSelectPayoutMethodText: '',
        selectedPayoutMethod: 1,
        usingOurSecureSystemText: '',
        virtualCreditCardText: '',
        youWillBePromptedToEnterText: '',
        hasError: false,
        onChange: jest.fn(),
        isPaypalAvailable: false,
        isVirtualCcAvailable: false,
        visible: false,
    };

    it('should render properly', () => {
        const newProps: IPayoutMethodContainerProps = {
            ...props,
            isPaypalAvailable: true,
            isVirtualCcAvailable: true,
            visible: true,
        };

        const wrapper = shallow(<PayoutMethodContainer {...newProps} />);
        expect(wrapper.find(RadioButton)).toHaveLength(3);
    });

    it('should not render if visible false', () => {
        const newProps: IPayoutMethodContainerProps = {
            ...props,
            isPaypalAvailable: true,
            isVirtualCcAvailable: true,
            visible: false,
        };

        const wrapper = shallow(<PayoutMethodContainer {...newProps} />);

        expect(wrapper.find(RadioButton)).toHaveLength(0);
    });

    it('should render properly without paypal', () => {
        const newProps: IPayoutMethodContainerProps = {
            ...props,
            isPaypalAvailable: false,
            isVirtualCcAvailable: true,
            visible: true,
        };

        const wrapper = shallow(<PayoutMethodContainer {...newProps} />);

        expect(wrapper.find(RadioButton)).toHaveLength(2);
    });

    it('should render properly with paypal', () => {
        const newProps: IPayoutMethodContainerProps = {
            ...props,
            isPaypalAvailable: true,
            isVirtualCcAvailable: false,
            visible: true,
        };

        const wrapper = shallow(<PayoutMethodContainer {...newProps} />);

        expect(wrapper.find(RadioButton)).toHaveLength(2);
    });

    it('should trigger on change when radio button clicked', () => {
        const onChange = jest.fn();
        const newProps: IPayoutMethodContainerProps = {
            ...props,
            onChange,
            isPaypalAvailable: true,
            visible: true,
        };

        const wrapper = shallow(<PayoutMethodContainer {...newProps} />);
        const onChangeProp = wrapper.find(RadioButton).at(0).prop('onChange') as any;
        onChangeProp({ target: { checked: true } });

        expect(onChange).toHaveBeenCalled();
    });

    it('should not render Virtual credit card', () => {
        const newProps: IPayoutMethodContainerProps = {
            ...props,
            visible: true,
        };

        const wrapper = shallow(<PayoutMethodContainer {...newProps} />);

        expect(wrapper.find(RadioButton)).toHaveLength(1);
    });

    it('should render Virtual credit card', () => {
        const newProps: IPayoutMethodContainerProps = {
            ...props,
            visible: true,
            isVirtualCcAvailable: true,
        };

        const wrapper = shallow(<PayoutMethodContainer {...newProps} />);

        expect(wrapper.find(RadioButton)).toHaveLength(2);
    });
});

The first thing I want you to notice is how much easier the tests are to read. We’re no longer repeating (hint: copy + paste) the props. We have a global prop for all the tests which can be easily extended and changed for each of the tests. We only need to include the specific props that need to change. With fewer lines of code to scan, you can now understand what each test is testing.

Copy + Paste is Not Always Bad

Don’t get me wrong. Sometimes you need to copy and paste. It’s just life. You don’t have time to engineer a pattern or you copy and paste a solution from the Internet. But what I do want you to think about is if there is a better way. Like the example above, you can see we removed code duplication and increased visibility. Sometimes, you might find you “copy and paste” because that’s what’s always done. But if you know the reason is that it was always done this way – then there’s something you need to investigate.

Remember, don’t repeat yourself. Copy and paste is a huge smell that you’re repeating yourself. Can you do it another way?