Refactor options handling

2 views
Skip to first unread message

Udo Borkowski

unread,
Apr 7, 2006, 2:27:25 PM4/7/06
to Tiddly...@googlegroups.com
Jeremy: may I ask for a little refactoring regarding the options handling, to simplify later extensions?

Currently you find code like this

        switch(opt.substr(0,3))
            {
            case "txt":
                elementType = "input";
                valueField = "value";
                break;
            case "chk":
                elementType = "input";
                valueField = "checked";
                break;
            }

that handles the various kinds of "options" (currently just "txt" and "chk"). This kind of switch construct is used in four functions.

Now assume that someone want to introduce a "pas" (for password) option. This would require to copy/paste those four functions, add the new type (i.e. another case 'pas' in the switch) and replace the old functions with the enhanced ones. Even though this would work it is already a lot "copy/paste", typically not a good idea.

Now assume that another guy wants to introduce a "sel" (for Selection) option. Of cause he does not know about the pas feature, so he also does a copy/paste of the original core code, adds his 'sel' option and hopes for the best.

As a consequence it will never be possible to have both the "pas" and the "sel" supported in TiddlyWiki. The plugin last executed will win.


To solve this issue I suggest to get rid of the switches but define a "handler" for each of the option types, that codes the option specific behaviour. Here my proposal:
config.optionHandler =  {
    txt: {
        elementType: "input",
        valueField: "value",
        appendElement: function(place, opt, onChangeOption) {
            var c = document.createElement("input");
            c.onkeyup = onChangeOption;
            c.setAttribute("option",opt);
            c.size = 15;
            place.appendChild(c);
            c.value = config.options[opt];
            },
        toValue: function(cookieText) {
            return unescape(cookieText);
            },
        toCookieText: function(value) {
            return escape(value.toString());
            }
        },
       
    chk: {
        elementType: "input",
        valueField: "checked",
        appendElement: function(place, opt, onChangeOption) {
            var c = document.createElement("input");
            c.setAttribute("type","checkbox");
            c.onclick = onChangeOption;
            c.setAttribute("option",opt);
            place.appendChild(c);
            c.checked = config.options[opt];
            },
        toValue: function(cookieText) {
            return cookieText == "true";
            },
        toCookieText: function(value) {
            return value ? "true" : "false";
            }
        }
    };
As you can see this mainly means moving code from the switches to the handlers.

Now we can remove the switches and access the handlers instead. Therefore we first select the proper handler using this new function:
function getOptionHandler(opt) {
    return config.optionHandler[opt.substr(0,3)];
}
And then we use the handler to perform the action.  E.g.:
    var handler = getOptionHandler(name);
    if (!handler)
            return;
    c += handler.toCookieText(config.options[name]);
 
Supporting a new option type (e.g. "pas" or "sel") just requires adding another handler to the config.optionHandler object.

If you are interested in this refactoring you find a complete patch file (in unified diff format) at http://tiddlywiki.abego-software.de/patch/options.diff.txt.



Cheers,

Udo

P.S.: I tested the stuff under Firefox and it works fine.


BidiX

unread,
Apr 7, 2006, 3:53:24 PM4/7/06
to Tiddly...@googlegroups.com
Great Udo. No need to hijack macro.options. functions anymore for a new option type like I've done in PasswordTweak.

Jeremy, do you think it could be useful to define a password option in the Core because it is very often used in Plugins and ServerSide adaptations ...

... and Udo should then consider to manage pasOption in his really valuable SettingsPlugin ;)


-- BidiX
TiddlyWiki.bidix.info
Reply all
Reply to author
Forward
0 new messages