1
\$\begingroup\$

Here is a light command line I built with JavaScript. I would like to ask if there are many stupidities, bad code and such? The only test command right now is add arg1 arg2 so writing add 1 1 should get you >>> 2. It will print Command *** not found with other commands than add. See the fiddle below.

jsfiddle.net http://jsfiddle.net/Nr2Am/26/

JavaScript

function cli_focus() {
    var cli = document.getElementById("commandline");
    cli.focus();
}

function arguments_to_array(args) {
    var arr = new Array();
    for (var i=0; i<args.lentgh; ++i) {
        arr[i] = args[i];
    }
    return arr;
}

function cli_go(input) {
    var lines = input.value;
    var lines_arr = lines.split(/\n+/);
    var cmd = lines_arr[lines_arr.length-1];

    cli_run(cmd);
    return false;
}

function cli_parse(cmd) {
    return cmd.split(/\s+/);
}

function cli_remove_blank_words(words) {

    while (words.length>0 && words[0]==="") {
        words = words.slice(1);    
    }
    while (words.length>0 && words[words.length-1]==="") {
        words = words.slice(0, words.length-1);    
    }

    return words;
}

function cli_run(cmd) {
    var words = cli_parse(cmd);
    words = cli_remove_blank_words(words);

    var last_word = null;

    for (var i=0; i<words.length; ++i) {
        var func_name = words.slice(0, i+1).join("_");
        if (window[func_name] === undefined) {
            break;   
        } else {
            last_word = i;
        }
    }

    if (last_word===null || words.length===0) {
        document.getElementById('commandline').value = document.getElementById('commandline').value + '\n>>> Command ' + words[0] + ' not found';
        return;
    }

    var func_name = words.slice(0, last_word+1).join( "_" );
    var func = window[func_name];
    var args = words.slice(last_word+1);
    func.apply(this, args);
}

function add(a,b) {
    var result = parseInt(a)+parseInt(b);
    document.getElementById('commandline').value = document.getElementById('commandline').value + '\n>>> ' + result;   
}
\$\endgroup\$
1
\$\begingroup\$

Looks like arguments_to_array() can be removed.

You may want to consider having cli_parse() call cli_remove_blank_words() rather than cli_run().

Also, depending on the direction you're going with the command line, you can probably simplify cli_run() in other ways. That long document.getElementById expression should probably be isolated in its own function, since it seems it's going to be used the same way every time for the most part. By doing shift() to the cmd array, you can get the function you want to call from that (and the remaining elements in the array are simply your args to that function). Then you can use a logical operator to check whether that function exists, otherwise call a function that handles errors and such.

Here's how I'd do it:

function cli_parse(cmd) {
    return cli_remove_blank_words(cmd.split(/\s+/));
}

function cli_run(cmd) {
    var words = cli_parse(cmd);
    var func_name = words.shift();
    var func = window[func_name] || window.cli_error;
    func.apply(this, words);
}

function cli_print(txt) {
    document.getElementById('commandline').value =  document.getElementById('commandline').value + '\n>>> ' + txt;   
}

function cli_error(error_info) {
    cli_print("Unknown command");
}

function add(a,b) {
    var result = parseInt(a)+parseInt(b);
    cli_print(result);  
}

Although, I'd really say using a more object oriented approach would probably facilitate expansion easier, but this works fine.

\$\endgroup\$
  • \$\begingroup\$ cli_print could be simplified to document.getElementById('commandline').value += '\n>>> ' + txt; \$\endgroup\$ – Victor Sep 6 '13 at 11:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.