notoriousb1t

Simple Coding - Part 4: Focus and Consistency

Debugging 2000+ line classes is no picnic. The worst offender I've seen was a Swing class written in Java. The initComponents() method did all of the layout manually and attached all action listeners as anonymous classes. Aside from the sheer size of the class, the property setting was done in no particular order. Somewhere along the line someone decided to start naming things differently, so there were two competing naming conventions that got added over time.

I don't advocate creating monsters like these. You can, however, make large pieces of code like that more manageable by following two concepts.

Be Focused

Don't try do more than one thing at a time. If you multitask while writing code, the reader will have to multitask to read it. Consider the following code:

In no particular order...

    protected void Page_Load(object sender, EventArgs e)
    {
        var lastName = LastNameField.Text;
        var age = AgeField.Text;
        if (string.IsNullOrWhiteSpace(lastName))
        {
            throw new Exception("Last Name is Required");
        }
        var firstName = FirstNameField.Text;
        Session["LastName"] = lastName;
        if (string.IsNullOrWhiteSpace(age))
        {
            throw new Exception("Age is Required");
        }
        if (string.IsNullOrWhiteSpace(firstName))
        {
            throw new Exception("First Name is Required");
        }
        Session["FirstName"] = firstName;
        Session["Age"] = age;
    }

Notice how it is difficult to tell that there is repetition at a glance. Now consider the same operations, refactored so like operations and properties are grouped.

One operation at a time

    protected void Page_Load(object sender, EventArgs e)
    {
        var firstName = FirstNameField.Text;
        var lastName = LastNameField.Text;
        var age = AgeField.Text;
        if (string.IsNullOrWhiteSpace(firstName))
        {
            throw new Exception("First Name is Required");
        }
        if (string.IsNullOrWhiteSpace(lastName))
        {
            throw new Exception("Last Name is Required");
        }
        if (string.IsNullOrWhiteSpace(age))
        {
            throw new Exception("Age is Required");
        }
        Session["FirstName"] = firstName;
        Session["LastName"] = lastName;
        Session["Age"] = age;
    }

One property at a time

    protected void Page_Load(object sender, EventArgs e)
    {
        // get first name and validate
        var firstName = FirstNameField.Text;
        if (string.IsNullOrWhiteSpace(firstName))
        {
            throw new Exception("First Name is Required");
        }
        Session["FirstName"] = firstName;

        // get last name and validate
        var lastName = LastNameField.Text;
        if (string.IsNullOrWhiteSpace(lastName))
        {
            throw new Exception("Last Name is Required");
        }
        Session["LastName"] = lastName;

        // get age and validate
        var age = AgeField.Text;
        if (string.IsNullOrWhiteSpace(age))
        {
            throw new Exception("Age is Required");
        }
        Session["Age"] = age;
    }

A lot easier to follow. I prefer to group statements by what I'm operating on rather than what operations I'm performing, but I think that is too subjective to call.

Be Consistent

Consistency is important because it increases readability. At the very least, be consistent in the same file. Consider the following example:

Names galore!

    private void initComponents() {
        firstNameLbl = new JLabel();
        firstNameLbl.setText("First Name");
        panel1.add(firstNameLbl);

        textFirst = new JTextField();
        textFirst.setText("");
        panel1.add(textFirst);

        lastNameLabel = new JLabel();
        lastNameLabel.setText("Last Name");
        panel1.add(lastNameLabel);

        lastNameTxt = new JTextField();
        lastNameTxt.setText("");
        panel1.add(lastNameTxt);

        labelAge = new JLabel();
        labelAge.setText("Age");
        panel1.add(labelAge);

        spnnrAge = new JSpinner();
        spnnrAge.setValue(0);
        panel1.add(spnnrAge);

        submitButton = new JButton();
        submitButton.setText("Submit");
        panel1.add(submitButton);
    }

Not especially hard to read. Though, we can do better:

More consistent

    private void initComponents(){
        firstNameLabel = new JLabel();
        firstNameLabel.setText("First Name");
        panel1.add(firstNameLabel);

        firstNameText = new JTextField();
        firstNameText.setText("");
        panel1.add(firstNameText);

        lastNameLabel = new JLabel();
        lastNameLabel.setText("Last Name");
        panel1.add(lastNameLabel);

        lastNameText = new JTextField();
        lastNameText.setText("");
        panel1.add(lastNameText);

        ageLabel = new JLabel();
        ageLabel.setText("Age");
        panel1.add(ageLabel);

        ageSpinner = new JSpinner();
        ageSpinner.setValue(0);
        panel1.add(ageSpinner);

        submitButton = new JButton();
        submitButton.setText("Submit");
        panel1.add(submitButton);
    }

That is a lot easier to follow. For me, it isn't so much what you call each variable; it is more about making sure to consistently name things. I couldn't care less if you want to shorten Label to Lbl or drop "name" from the first and last fields. It is more important to pick a way to do something and stick to it.