Does this code snippet look okay?

T

Tom Cole

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...
 
T

Tom de Neef

Tom Cole said:
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
 

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,990
Messages
2,570,211
Members
46,796
Latest member
SteveBreed

Latest Threads

Top