Generalization pitfalls

Experienced developers, including me, tend to prefer generalized code over highly specialized one, but they usually love very simple and highly readable code much more and the two don’t always pair nicely.

Disclaimer

I’ll use Java and Cucumber to make my point clear (I hope) but what I’m going to assert is not strictly related to neither of those, actually is not even strictly related to programming!

I’ve smashed my face into this problem when I started using Cucumber and my team decided a single generalized method to map to a UI button click should be sufficient: I believe many of you would agree the code below isn’t a bad idea.

@When("I click on \"(.*)\" button")
public void buttonClick(String buttonLabel) {
  // ui specific code to click the button
}

I now tend to advice against such generalizations because it’s very rare a single method can suffice your needs because they are too generic to be implementable. Let me explain.

The very first step to implement such method is to resolve the label into some sort of reference to the UI button to click and then click such component. Now, unless you establish some sort of very strict convention you’ll end up with something like the following:

@When("I click on \"(.*)\" button")
public void buttonClick(String buttonLabel) {
  UIButton button = null;
  if ("OK".equals(buttonLabel))
    button = this.getOkButton();
  else if ("Cancel".equals(buttonLabel))
    button = this.getCancelButton();
  else if ("Save".equals(buttonLabel))
    button = this.getSaveButton();
  else if ("Edit".equals(buttonLabel))
    button = this.getEditButton();
  else if ("Delete".equals(buttonLabel))
    button = this.getDeleteButton();
  else
    // button unknown!
  button.click();
}

I’m obviously simplifying the code above for sake of readability: the very first implementation of that method was 200 lines when we got to 20 buttons!

Please note the issue is not related to the Cucumber capturing group nor to the string parameter, but to the use we are doing of it: we are de-generalizing a generalized method!

Now, wouldn’t it been simpler and more readable to have something like the following?

@When("I click on \"OK\" button")
public void buttonOkClick() {
  UIButton button = this.getOkButton();
  button.click();
}

@When("I click on \"Cancel\" button")
public void buttonCancelClick() {
  UIButton button = this.getCancelButton();
  else if ("Save".equals(buttonLabel))
  button.click();
}

@When("I click on \"Save\" button")
public void buttonSaveClick() {
  UIButton button = this.getSaveButton();
  button.click();
}

@When("I click on \"Edit\" button")
public void buttonEditClick() {
  UIButton button = this.getEditButton();
  button.click();
}

@When("I click on \"Delete\" button")
public void buttonDeleteClick() {
  UIButton button = this.getDeleteButton();
  button.click();
}

The code is longer, I agree, but it’s way much clear, a lot easier to debug and, above all, if there’s a missing button mapping a clear error is returned!

You might argue this case is not applicable to you because a strict button naming convention is well established on your project and a button is always identifiable from it’s label (let’s suppose you are using an HTML based UI and each button has an id in the form of label-btn so that the above can be resolved into the following:

@When("I click on \"(.*)\" button")
public void buttonClick(String buttonLabel) {
  // enforce naming convention here and prepare the buttonLabel
  UIButton button = this.getButton(buttonLabel + "-btn");
  button.click();
}

Lovely, isn’t it? Now a paginated data set comes in, something like the one in the picture below, and you suddenly need to assign an id of «PreviousPage-btn (note the initial angle quote) to an HTML tag!pagination

If you modify the above code into the following one you are falling into the same pitfall, no excuse granted!

@When("I click on \"(.*)\" button")
public void buttonClick(String buttonLabel) {
  if (buttonLabel.startsWith("«") || buttonLabel.startsWith("»"))
    buttonLabel = buttonLabel.substring(6);
  UIButton button = this.getButton(buttonLabel + "-btn");
  button.click();
}

The code above might still look clean and neat, but it is indeed a source of problems: it’s your first shovel of dirt while digging your own grave. You should have gone with the following instead:

@When("I click on previous page button")
public void buttonPrevPageClick() {
  UIButton button = this.getButton("prevPage-btn");
  button.click();
}

@When("I click on next page button")
public void buttonNextPageClick() {
  UIButton button = this.getButton("nextpage-btn");
  button.click();
}

@When("I click on \"(.*)\" button")
public void buttonClick(String buttonLabel) {
  UIButton button = this.getButton(buttonLabel + "-btn");
  button.click();
}

Again, a little more code but a lot cleaner and readable and, above all, it doesn’t imply you have to type EXACTLY the same button label characters into your feature files or do some killer loops to describe a test for a stupid button!

Now, if you read this post up to this point you have probably fallen into this pitfall multiple times and you probably still have doubts about what I’m saying, probably due to the fact you really love the generalization concept/method, so here is the generalized reason why method generalization is not always a good practice:

While performing step to stepdef matching Cucumber is performing a de-generalization, going from a generic string to a specific method.
By using stepdef parameters you are performing a generalization, allowing multiple strings to match the same method.

But when within the stepdef you add control logic on the parameters you are performing an additional de-generalization, trying to identify a specific match for a specific parameter value: something Cucumber already tried to do during stepdef matching!

It’s like trying to change gold for gold by transforming gold into silver and then back into gold, all with little or no gain and a lot of unnecessary confusion.

Let Cucumber do it’s work and don’t try to generalize everywhere, instead let the natural language unleash it’s expressive power and use it at your own benefit.

Now, while I used Cucumber steps and stepdefs as a demonstration, this concept is applicable in many other contexts, and here it comes my statement in a more generalized version:

Question yourself twice whenever you are trying to generalize something right after a de-generalization has just happened: you’ll discover you are doing the wrong thing 99% of the time.

Whenever you are certain your case falls within that remaining 1% then you are surely doing the wrong thing: doubting about yourself is your only salvation!

If you allow me the hyperbole, wouldn’t be messy to develop by always using Java Reflection? Well, Java Reflection is the generalization above the whole Java framework: one API to rule them all!

Advertisements

6 thoughts on “Generalization pitfalls

  1. I completely agree with your intent, Roberto.

    Of course, it’s often fine to write step definitions that pass their input arguments through to the system being developed. I wouldn’t want anyone to get the idea that using capture groups in Cucumber regular expressions was a bad idea 😉

    Like

    • If that is the message that comes through I have to reword something because that wasn’t my intention at all!
      The problem is not with passing parameters, but using the input parameter as a switch condition to redirect the flow into a separate branch.

      What part of this post suggested you that I might be against stepdef capturing groups? I will reword it immediately…

      Like

      • Your post doesn’t suggest that at all – it just doesn’t explicitly mention it. Read in isolation, a naive visitor might come to the wrong conclusion.

        Maybe contrast your example of generalisation with an example of capturing meaningful data and passing it through to the system?

        Or maybe I’m worrying unnecessarily and it doesn’t need anything more at all….

        Like

      • I get your point and made a few changes to the post here and there trying to clarify that to the naive visitor ;-D

        Thanks a million!

        Like

  2. Funnily enough we came to the same conclusion on a certain project that I think you are familiar with 😃

    It just becomes lots of effort and error prone to keep things so generic.

    A follow up we found was that method names mapped directly to the stepdef, rather than usual java naming conventions; as you say, let cucumber do what it does best.

    Like

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s