Tuesday, June 18, 2013

Don't Use a Regular Expression When a String Will Do

‹prev | My Chain | next›

It is deadline-day for 3D Game Programming for Kids, so today's post may be briefer than normal. But the gods of the chain who have been so very good to me require their offering and an offering they shall have.

Not coincidentally, the ICE Code Editor, which is used exclusively in the book is nearing a milestone deadline as well: the it-has-to-work-for-the-book deadline. Fortunately, most of the major issues have been cleared up. In fact, there are only two issues still outstanding: a project sharing features that one of my collaborators has nearly finished and a minor bug. Saaay… minor bug sounds perfect tonight!

In the ICE Code Editor, we work hard to ensure that none of the actions overwrites an existing projects. This is a new feature of the Dart version of ICE. I believe that it was possible to overwrite existing projects in the JavaScript version by creating a new project, renaming a project, making a copy, opening a shared project or looking funny at a project. Maybe not the last one.

The feature that prevents overwriting projects in the Dart version is not so much thanks to Dart as it is thanks to the ease of Dart's unit testing library. As with any tool, it is only as good as the person wielding it and I seem to have overlooked something.

Existing tests verify that copied projects get the copy number placed in parentheses:
    test("copied project includes copy number in parentheses", (){
      helpers.createProject('Project #1');

      helpers.click('button', text: '☰');
      helpers.click('li', text: 'Make a Copy');

      expect(
        query('.ice-dialog input[type=text]').value,
        equals("Project #1 (1)")
      );
     });
The problem facing me tonight is that projects ending with parentheses are not creating copies correctly. Or, in unit-test-ese:
    test("project name ending in parens", (){
      helpers.createProject('projectNamedForFunction()');

      helpers.click('button', text: '☰');
      helpers.click('li', text: 'Make a Copy');

      expect(
        query('.ice-dialog input[type=text]').value,
        equals("projectNamedForFunction() (1)")
      );
     });
Thankfully, that fails:
Exception: Bad state: No elements dart:core-patch/growable_array.dart:152
List.last dart:core-patch/growable_array.dart:152
Store.nextProjectNamed package:ice_code_editor/store.dart:119
CopyDialog._copiedProjectName package:ice_code_editor/full/copy_dialog.dart:30
CopyDialog.open package:ice_code_editor/full/copy_dialog.dart:12
MenuItem.el.<anonymous closure>
If that had not failed, then I would have been forced to determine what was different between my test case and the live application before I could get down to fixing the bug. As it is, I have another bit of good news: the bug appears to be on line 119 of the store.dart code file. In other words, I only have to solve this bug in one place for it to be solved anywhere that is trying to use this copy-number functionality.

I eventually track this down to the code that finds the list of all projects with the same base name:
    // ...
    RegExp copy_number_re = new RegExp(r"\s+\((\d+)\)$");
    var title = original_title.replaceFirst(copy_number_re, "");

    var same_base = values.where((p) {
      return new RegExp("^" + title + r"(?:\s+\(\d+\))?$").hasMatch(p['filename']);
    });
    // ...
The problem is that the title in there is projectNamedForFunction()The parentheses in there are interpreted as regular expression syntax instead of literal strings that should match. I could go through and replace parentheses with their escaped counterparts (e.g. '\\('), but then what about project names with square brackets or curly braces?

And then it hits me. Why am I using a regular expression to match literal strings when a simple literal string comparison will do? So instead, I determine the list of projects with the same base name by asking for all projects that start with the base:
    // ...
    RegExp copy_number_re = new RegExp(r"\s+\((\d+)\)$");
    var title = original_title.replaceFirst(copy_number_re, "");

    var same_base = values.where((p) {
      return p['filename'].startsWith(title);
    });
    // ...
And that does the trick:
PASS: Copy Dialog project name ending in parens
I have no idea why I would have used the regular expression in the first place, but my test suite confirms that the simpler string works. With that bug squashed, I can safely return to the last few fixes in 3D Game Programming for Kids. Wish me luck…


Day #786

No comments:

Post a Comment