Does this code snippet look okay?

Discussion in 'Javascript' started by Tom Cole, Apr 25, 2008.

  1. Tom Cole

    Tom Cole Guest

    The code section works just fine, but I want to see if

    A) I'm writing this type of code correctly, and
    B) I'm actually getting what I expect.

    The code looks like this (it is for mapping file extensions to the
    document types recognized by SAP's DMS):

    var Extensions = function() {
    this.types = new Array();
    this.extensions = new Array();

    this.addType = function(type, ext) {
    this.types.push(type);
    this.extensions[type] = ext;
    }

    this.getTypeForExtension = function(ext) {
    for (var i = 0; i < this.types.length; i++) {
    var type = this.types;
    var extension = this.extensions[type];
    if (containsExtension(ext, extension)) {
    return type;
    }
    }
    return null;
    }

    var containsExtension = function(value, array) {
    for (var i = 0; i < array.length; i++) {
    if (array.toLowerCase() == value.toLowerCase()) {
    return true;
    }
    }
    return false;
    }
    }

    What I do is create an Extensions variable, populate it through
    addType and then later retrieve
    the document type for a given extension via getTypeForExtension.

    Is there anything "wrong" with this code? Would any experts out there
    recommend changes (i.e. adding methods(functions) to the prototype).
    While the code I'm writing always appears to work, I have to admit
    that with it's loose typing, I don't always know exactly what's going
    on behind the scenes with the code. It just works the way I expect it
    to, which I guess is better than the alternative :). I've heard the
    term closure thrown around, but I have not been able to fully
    understand the concept. I'm a java developer, so these things are a
    little foreign to me.

    From what I've written I expect that addType and getTypeForExtension
    are visible and can be called by any Extensions "instance", but
    containsExtension cannot:

    i.e.
    var extensionList = new Extensions();
    extensionList.addType(type, extensions); //this is visible, no
    problem...
    extensionList.getTypeForExtension(".gif"); //this is visible, no
    problem...
    extensionList.containsExtension(".gif", extensions); //this is NOT
    visible and throws an error that says
    //
    extensionList.containsExtension is not a function...
    Tom Cole, Apr 25, 2008
    #1
    1. Advertising

  2. Tom Cole

    Tom de Neef Guest

    "Tom Cole" wrote:
    > The code section works just fine, but I want to see if
    >
    > A) I'm writing this type of code correctly, and
    > B) I'm actually getting what I expect.
    >
    > The code looks like this (it is for mapping file extensions to the
    > document types recognized by SAP's DMS):
    >
    > var Extensions = function() {
    > this.types = new Array();
    > this.extensions = new Array();
    >
    > this.addType = function(type, ext) {
    > this.types.push(type);
    > this.extensions[type] = ext;
    > }
    >
    > this.getTypeForExtension = function(ext) {
    > for (var i = 0; i < this.types.length; i++) {
    > var type = this.types;
    > var extension = this.extensions[type];
    > if (containsExtension(ext, extension)) {
    > return type;
    > }
    > }
    > return null;
    > }
    >
    > var containsExtension = function(value, array) {
    > for (var i = 0; i < array.length; i++) {
    > if (array.toLowerCase() == value.toLowerCase()) {
    > return true;
    > }
    > }
    > return false;
    > }
    > }
    >
    > What I do is create an Extensions variable, populate it through
    > addType and then later retrieve
    > the document type for a given extension via getTypeForExtension.
    >


    I am certainly not an expert, but my comments may be of help nevertheless.
    Basically, what you want is an aray with elements of the kind:
    knownTypes["gif"] = "picture"
    knownTypes["doc"] = "text"
    So, adding is via knownTypes[extension] = type;
    Retrieval via type = knownTypes[extension];
    Note that the addition will overwrite an existing entry for this extension.

    You can package this in an object with methods add and retrieve, like you
    have aimed to do.
    But what is the purpose of your array types ? The various types are already
    in the array extensions.
    I don't think you need to loop thru the types array in containsExtension.
    Just try the retrieval I mentioned.
    (In a loop like that I would always move the value.toLowerCase() outside the
    loop for performance.)
    In getTypeForExtension, extension = this.extensions[type] is (most likely)
    just a string. But you pass it as an array argument to
    containsExtension(ext, extension). I don't understand that.
    Why do you use var Extensions = function() ... instead of function
    Extensions() ?
    Same with ContainsExtension.

    HTH
    Tom
    Tom de Neef, Apr 25, 2008
    #2
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Gene Gorokhovsky
    Replies:
    0
    Views:
    457
    Gene Gorokhovsky
    Jul 17, 2003
  2. inhahe
    Replies:
    3
    Views:
    2,349
    Diez B. Roggisch
    Jan 28, 2005
  3. Replies:
    18
    Views:
    490
    Richard Bos
    Feb 22, 2007
  4. Ryan Mitchley

    Pointer-to-member-function code okay?

    Ryan Mitchley, Mar 17, 2006, in forum: C++
    Replies:
    0
    Views:
    263
    Ryan Mitchley
    Mar 17, 2006
  5. Diwa
    Replies:
    8
    Views:
    497
    Daniel T.
    Dec 7, 2007
Loading...

Share This Page