681891 - GCLI Field.claim should be optimized

Status: RESOLVED WONTFIX

(DevTools Graveyard :: Graphic Commandline and Toolbar, defect, P4)

Description

[Joe Walker [:jwalker]]

The whole Field.claim stuff seems like there could be a better/more performant way. I like the abstraction, but I'm not sure about the implementation. How many different types of fields do we expect there to be? Would it make sense to use an algorithm that isn't O(n)? Instead of checking everytime, could we just look this info up in an object as if it was a hash table? This would require the types to have unique string representations, which I'm not sure whether or not they do (feel free to correct me here).

If Field.claim were replaced by the imaginary claimField, the code might look something like this:

    function StringField(...) {
      ...
    }
    StringField.prototype = Object.create(Field.prototype);
    
    claimField(Field.MATCH, StringType.toString(), StringField);
    claimField(Field.IF_NOTHING_BETTER, BooleanType.toString(), StringField);

Not only would this move the algorithm from O(n) -> O(1), but it would make the calculations for which field is the best fit for a given type happen only when new fields are claimed, not every time we are looking for the right field.

Then getField and friends would need to be updated a little as well:

    var fieldCtors = {};
    
    function claimField(level, typeString, Ctor) {
      if (!fieldCtors[typeString] || level > fieldCtors[typeString].level) {
        fieldCtors[typeString] = { level: level, ctor: Ctor };
      }
    }
    
    function getField(...) {
      ...
      var typeString = type.constructor.toString();
      if (typeString in fieldCtors) {
        return new fieldCtors[typeString].ctor(...)
      } else if ('DEFAULT_FIELD' in fieldCtors) {
        return fieldCtors.DEFAULT_FIELD.ctor(...)
      } else {
        throw new Error(...)
      }
    }
    
    ...

It definitely means that less will have to be computed after initialization, and less redundant computations.

Comment 1

[Joe Walker [:jwalker]]

I agree that this is probably a good idea, and I'm particularly pleased that you picked up on it because I didn't like it.

However I have 2 reservations:
- It seems to me not inconcievable that we might want to select different fields based on more than just the type of the argument. If we have a selection with less than 5 options then maybe we'd use a set of <input type=radio> rather than a <select>, for example.
- This could be a premature optimization

On the other hand:
- I don't like the current implemenation
- The flexability might not be a good idea anyway

So I've made it block GCLI-HELP which is the pot for 'nice to have' for the next release.

Comment 2

[Joe Walker [:jwalker]]

Moving to GCLI-FUTURE because we've done enough of the bugs in GCLI-HELP that it's not really a 'nice to have' list.

Comment 3

[Joe Walker [:jwalker]]

Moving GCLI bugs to Developer Tools: Console. Filter on 'baked beans are off'.

Comment 4

[Joe Walker [:jwalker]]

Triage. Filter on PEGASUS.

Comment 5

[Joe Walker [:jwalker]]

Triage. Filter on PEGASUS.

Comment 6

[Joe Walker [:jwalker]]

New component triage. Filter on "Lobster Thermidor aux crevettes with a Mornay sauce"

Comment 7

[Joe Walker [:jwalker]]

I'm not sure having trivial code enhancements bugs like this on file helps. They slow down triage, and practically we'll fix them when the become a problem.

aka: http://www.joelonsoftware.com/items/2012/07/09.html