Comments on code layout requested

[Daniel Norden]

Hello.

Can you give me some feedback on my code layout?

I have everything organized in an object, "NRL", that's used as a namespace,
and there are other subobjects/modules below that, for example "dialog"
or "ajax". I'm interested in general comments, and I have some specific
questions, but first the (sample) code:

var NRL = {};
......

NRL.ajax = {};
......

NRL.ajax.ListControl = function (arg1, arg2) {

// first, some local "constants" (configuration)
var PAGER_BUFFER = 5,
EXPERIMENTAL_SORTING = false,
.....;

// next, the public members
this.paused = false;
this.currFoo = "";
.....

// public methods next
this.setTrigger = function (src) {
.....
};
this.fetchRows = function (argX, argY) {
.....
};

///// "private" after this line /////////////////////

var node, header, .....,
.....
spinner = NRL.ajax.Spinner.getInstance(),
that = this;

// private functions only accessible in/below this scope
function analyzeHeader () {
.....
}
function receiveUpdate (jsonData) {
.....
}
......

///// some initializations (not in all objects) //////////

node = document.createElement("div");
.....

};


It works well, as far as I can see, and I like the separation of "public"
and "private" areas in the source. When other people read the code, they
can immediately see where the public "interface" ends. Some questions:

1) I'm a little worried about the many closures... does that use up a huge
amount of memory? If so how can I improve the layout? Some of the functions
have other inner functions as well, like when I add event handlers or
callbacks for XMLHttpRequest, that also act as closures.

2) I have seen people using function expressions instead of function
statements for the "private" functions. Do function expressions have any
advantage in this case?

3) Most of the objects don't ever need it, but what would be the best way to
let other objects inherit from a structure like this?

4) Some of these objects are singletons, like the NRL.ajax.Spinner in the
example above. I currently handle them like this:

NRL.ajax.Spinner = function () {
..... same as above .....
};
NRL.ajax.Spinner.getInstance = function () {
var instance;
return function () {
instance = instance || new NRL.ajax.Spinner();
return instance;
}();
};

Now somebody could still use "new NRL.ajax.Spinner()" directly, but that
could be avoided by adding a check to the Spinner constructor. I don't want
to create an object literal for the singleton unless an object (instance)
is really required, and besides the constructor could have initialization
code, like creating elements.


Thanks everybody,
Dano

 

[Daniel Norden]

Can you give me some feedback on my code layout?

Okay... that didn't go as well as I'd hoped. I probably asked too many
questions at once. I'm going to give it another shot and just ask one short
question this time. I'd really appreciate some feedback from the JS pundits
in this group.... If you need more detail, my original post's still
there[0]; a reply to either one would be great.

Do you see any general problems with a constructor like this?

var ListControl = function (arg1, arg2) {
// public members
this.paused = false;
.....

// public methods
this.setTrigger = function (src) { ..... };
.....

///// "private" after this line /////////////////////
var node, header, that = this;

// private functions
function analyzeHeader () { ..... }
......

///// some initializations (not in all constructors) //////////
node = document.createElement("div");
.....
};

var ctrl = new ListControl(x,y);


Thanks in advance,
Dano


[0] msg id <[email protected]>
http://groups.google.com/group/comp.lang.javascript/msg/129ab340d8cd1d85

 

[Thomas 'PointedEars' Lahn]

Can you give me some feedback on my code layout?

Okay... that didn't go as well as I'd hoped. I probably asked too many
questions at once. [...]

Or maybe you did not wait long enough Weekend is *now*. You can expect
a more verbose reply, at least from me, later.


PointedEars

 

[Thomas 'PointedEars' Lahn]

I have everything organized in an object, "NRL", that's used as a namespace,
and there are other subobjects/modules below that, for example "dialog"
or "ajax". I'm interested in general comments, and I have some specific
questions, but first the (sample) code:

var NRL = {};

Since the identifier refers to an object that cannot be [[Construct]]ed, it
should not start with a capital letter.

.....

NRL.ajax = {};
.....

NRL.ajax.ListControl = function (arg1, arg2) {

You can write this more concise and easier to maintain, without drawbacks in
itself:

var NRL = {
ajax: {
ListControl: function(...) {
...
}
}
};

Note that you can create "namespaces" with prefixes as well.

[...]
// next, the public members

Those should usually be moved to and initialized through a prototype object.
Thus, on construction of the new object only properties need to be
initialized which value should differ from the inherited value, e.g.
where the former depends on the constructor arguments.

NRL.ajax.Spinner.prototype = {
paused: false
};

Especially with methods this improves memory efficiency since you have only
one function object that is used by all instances. However, it would also
slightly decrease runtime efficiency as for all calls the lookup along the
prototype chain requires more steps. Since memory is more expensive than
CPU speed with regard to programming nowadays, I recommend to use the
prototype chain pattern where applicable.

[...]
///// "private" after this line /////////////////////

var node, header, .....,
.....
spinner = NRL.ajax.Spinner.getInstance(),

What sense does a singleton pattern make if you cannot prevent the creation
of new objects that inherit from the same object?

that = this;

// private functions only accessible in/below this scope
function analyzeHeader () {
.....
}
function receiveUpdate (jsonData) {
.....
}

Those functions only become private in the classical sense if you use them
in public methods.

......

///// some initializations (not in all objects) //////////

node = document.createElement("div");
.....

I find it harder to maintain source code if the declaration and
initialization of a variable are not in the same statement.
(There are a few exceptions, of course.)

[...]
1) I'm a little worried about the many closures... does that use up
a huge amount of memory?

That depends on what you consider "huge" and how many objects are created
while the application is running.

If so how can I improve the layout?

You can try applying the suggestions on closures in the FAQ Notes.

Some of the functions
have other inner functions as well, like when I add event handlers or
callbacks for XMLHttpRequest, that also act as closures.

Those are not considered "inner functions", but you have to look out for
closures that involve DOM objects there as well, especially if this should
run in a MSHTML-based UA.

2) I have seen people using function expressions instead of function
statements for the "private" functions. Do function expressions have any
advantage in this case?

If the possibility of conditional execution (without dirty hacks) and to use
initialized values counts as an advantage, then yes. We have discussed this
before.

3) Most of the objects don't ever need it, but what would be the best way to
let other objects inherit from a structure like this?

Google this newsgroup for "inheritance". We have discussed this ad nauseam
as well.

4) Some of these objects are singletons, like the NRL.ajax.Spinner in the
example above. I currently handle them like this:

NRL.ajax.Spinner = function () {
..... same as above .....
};
NRL.ajax.Spinner.getInstance = function () {
var instance;
return function () {
instance = instance || new NRL.ajax.Spinner();
return instance;
}();
};

Now somebody could still use "new NRL.ajax.Spinner()" directly, but that
could be avoided by adding a check to the Spinner constructor. I don't want
to create an object literal for the singleton unless an object (instance)
is really required,

Of course you need another object to count the number of instances created.
Ideally that object would be the constructor's prototype object or the
constructor itself.

and besides the constructor could have initialization code, like creating
elements.

A singleton pattern is useful (if rather pointless in these dynamic
languages) iff you want to prevent having several objects of the same
inheritance. Why would you want to deny the user of this library
to create several spinners?


PointedEars

 

[Daniel Norden]

[many interesting points]

Thomas, thank you very much for your insightful comments. You have given me
a lot of food for thought and research. I will look up the topics you've
said have been discussed 'ad nauseam' here, and I'll see how I can
integrate all that in my project. I'll post again when I've had a chance to
try it all out... maybe I'll wait until next weekend, that seems to be the
best time to get a response =)

One quick question, before I dive into it..... you said,

  // next, the public members

Those should usually be moved to and initialized through a prototype
object. Thus, on construction of the new object only properties need to
be initialized which value should differ from the inherited value [....]

Yeah, that makes sense. But... if I do that, they won't have access to the
variables I defined in my constructor (the "private" variables). I guess I
could just pass some of them as arguments every time, but that won't work
for all of them. At one time, I had tried to use properties like
"this._myPrivateVar", but that would only make them private by convention,
and access to them would not be restricted.


Thanks again,
Dano

 

[Thomas 'PointedEars' Lahn]

[many interesting points]

Thomas, thank you very much for your insightful comments. [...]

You are welcome.

[...] I'll post again when I've had a chance to try it all out... maybe
I'll wait until next weekend, that seems to be the best time to get a
response =)

The best time to post questions is after they came up and sufficiently
extensive testing and research produced no (satisfying) solution. After you
posted you should wait a while (maybe including a weekend) before you start
wondering why so few people answered. Knowledgeable people have regular
jobs (at least that is what one should hope for); therefore, during the
working days they may not have enough time on their hands for providing
sufficiently helpful advice here. Especially when they are new or not
trivial questions that you are asking.

// next, the public members

Those should usually be moved to and initialized through a prototype
object. Thus, on construction of the new object only properties need to
be initialized which value should differ from the inherited value
[....]

Yeah, that makes sense. But... if I do that, they won't have access to
the variables I defined in my constructor (the "private" variables). I
guess I could just pass some of them as arguments every time, but that
won't work for all of them. At one time, I had tried to use properties
like "this._myPrivateVar", but that would only make them private by
convention, and access to them would not be restricted.

Hence "usually". As for public properties, it makes no difference. As for
public methods, it depends on how important one deems information hiding to
be as compared to the advantage of saving memory in each case.


PointedEars

 

[dhtml]

Hello.

Can you give me some feedback on my code layout?

I have everything organized in an object, "NRL", that's used as a namespace,
and there are other subobjects/modules below that, for example "dialog"
or "ajax". I'm interested in general comments, and I have some specific
questions, but first the (sample) code:

It sounds like you're asking about structure.

var NRL = {};
.....

NRL.ajax = {};
.....

ListControl sounds like an object that would use a connection.

A good way to organize code is to have many small parts where the small
parts are strongly cohesive. What this means is that they do only one thing.

Cohesive design is easier to unit test and refactor. Custom builds of
the javascript can include only the things they need. So, if your app
needs a transport but not the ListControl, and the transport is 100 LOC,
then do not make the user DL a 500 LOC ListControl on a page that
doesn't need it.

The ListControl can be cohesive, can use the 'ajax' object, or can use
an object with an interface that it needs to 'getConnection' and
subscribe to events such as 'complete', 'done', 'progress' (see
ProgressEvents for ideas). The ListControl won't need to care about how
the 'ajax' object does its business; it will only care that there is an
object that has the methods it needs.


Is ListControl is a widget? A doc-style comment would help.

/**
* ListControl constructor
*/

NRL.ajax.ListControl = function (arg1, arg2) {

// first, some local "constants" (configuration)
var PAGER_BUFFER = 5,
EXPERIMENTAL_SORTING = false,
.....;

// next, the public members
this.paused = false;
this.currFoo = "";
.....

// public methods next
this.setTrigger = function (src) {
.....
};
this.fetchRows = function (argX, argY) {
.....
};

///// "private" after this line /////////////////////

var node, header, .....,
.....
spinner = NRL.ajax.Spinner.getInstance(),
that = this;

// private functions only accessible in/below this scope
function analyzeHeader () {
.....
}
function receiveUpdate (jsonData) {
.....
}
......

///// some initializations (not in all objects) //////////

node = document.createElement("div");
.....

};


It works well, as far as I can see, and I like the separation of "public"
and "private" areas in the source. When other people read the code, they
can immediately see where the public "interface" ends. Some questions:

1) I'm a little worried about the many closures... does that use up a huge
amount of memory?

You will likely have objects in the scope chain that you do not care
about. For example, |setTrigger| has a reference to |node| in the
enclosing scope.




If so how can I improve the layout? Some of the functions

have other inner functions as well, like when I add event handlers or
callbacks for XMLHttpRequest, that also act as closures.

I would recommend using fake private and fake final. If you use an
underscore convention, it might be more obvious. In most cases, it will
work. Just don't assign to the "_private" variable and it will be OK.

2) I have seen people using function expressions instead of function
statements for the "private" functions. Do function expressions have any
advantage in this case?

Did you get that term 'Function statement' from 'The Good Parts'?

That really is the wrong term for it.

3) Most of the objects don't ever need it, but what would be the best way to
let other objects inherit from a structure like this?

4) Some of these objects are singletons, like the NRL.ajax.Spinner in the
example above. I currently handle them like this:

NRL.ajax.Spinner = function () {
..... same as above .....
};

NRL.ajax.Spinner.getInstance = function () {
var instance;
return function () {
instance = instance || new NRL.ajax.Spinner();
return instance;
}();
};

Now somebody could still use "new NRL.ajax.Spinner()" directly, but that
could be avoided by adding a check to the Spinner constructor. I don't want
to create an object literal for the singleton unless an object (instance)
is really required, and besides the constructor could have initialization
code, like creating elements.

Spinner constructor could be called twice, but the getInstance method
would seem to be there to help prevent that by encouraging clients to
use that method. However, the getInstance method does not do that. In
fact, the getInstance method will declare a local - instance - variable,
then return the return value of a call an anonymous function. The -
instnace - variable is then eligible for collection.

The problem is that each time Spinner.getInstance is called, instance is
declared as local variable.


var Spinner = function () {
alert('in constructor');
};

/**
* FIXME: Calls the Spinner constructor every time.
*/
Spinner.getInstance = function () {
var instance; // local variable.

return function () {
instance = instance || new Spinner();
return instance;
}();
};

What you want instead is for the getInstance function to save a private
copy of the instance var. If - instance - is undefined, getInstance
should assign instance to a new Spinner.

Spinner.getInstance = (function () {
var instance;
/**
* @name Spinner.getInstance
* @private
*/
return function () {
instance = instance || new Spinner();
return instance;
};
}());

But might seem clearer if written as:-


(function () {
var instance;
/**
* @name Spinner.getInstance
* @private
*/
Spinner.getInstance = function () {
instance = instance || new Spinner();
return instance;
};
}());


If you want to make the Spinner constructor private, add it to the
anonymous function.

var NRL = { };
NRL.Spinner = {};

(function () {

var instance;
NRL.Spinner.getInstance = function (i) {
instance = instance || new Spinner(i);
return instance;
};

function Spinner(i) {
this.name = "Spinner " + i;
}
})();
NRL.Spinner.getInstance(1);

// Always returns the first Spinner.
NRL.Spinner.getInstance(2);
NRL.Spinner.getInstance("failure!");

(the above code is just drafts of ideas; not tested).

Garrett

 

[Dr J R Stockton]

In comp.lang.javascript message <[email protected]>

Cohesive design is easier to unit test and refactor.

<FAQENTRY> Suggestion :

---------------------------------
Jargon Explained
---------------------------------

Seek the following with the <http://en.wikipedia.org/wiki/Main_Page> Go
button : "Jargon" "Unit Testing" "Refactoring" ...

// That's perhaps the shortest way to indicate what might be needed.

 

[dhtml]

In comp.lang.javascript message <[email protected]>


<FAQENTRY> Suggestion :

---------------------------------
Jargon Explained
---------------------------------

Seek the following with the <http://en.wikipedia.org/wiki/Main_Page> Go
button : "Jargon" "Unit Testing" "Refactoring" ...

// That's perhaps the shortest way to indicate what might be needed.

Are you suggesting an FAQ for various terms, like a glossary, but with
links to Wikipedia instead of a definition?

Garrett

 

[Dr J R Stockton]

Are you suggesting an FAQ for various terms, like a glossary, but with
links to Wikipedia instead of a definition?

Yes. The FAQ should use no terms other than those which everybody
understands and those for which a meaning is indicated; and it should
give some help for terms not in the FAQ but often used in the newsgroup.
The shortest way of explaining is to let someone else do it; and,
assuming that several terms need an explanation, giving a tested Wiki
"Go" string in that manner seems briefest.

I'd not forge a question for Sections 4/5 (as last seen); if wanted,
drop it into Sections 2/3.


Web 2008-10-13 FAQ 2.4 contains an incomplete entry "You ignored the".
"W3C" is used before its first explanation.

 

[dhtml]

I always thought "Jargon" was a common word. It doesn't have to do with
programming.

Terms to define:

"Unit Testing"
"Refactoring"
"Host Object"

Yes. The FAQ should use no terms other than those which everybody
understands and those for which a meaning is indicated; and it should
give some help for terms not in the FAQ but often used in the newsgroup.
The shortest way of explaining is to let someone else do it; and,
assuming that several terms need an explanation, giving a tested Wiki
"Go" string in that manner seems briefest.

I'd rather put up an actual glossary in the FAQ and link to Wikipedia as
necessary.

I'd not forge a question for Sections 4/5 (as last seen); if wanted,
drop it into Sections 2/3.

I think a glossary would be appropriate right after "Language Overview"
(used to be "Javascript tips").

Web 2008-10-13 FAQ 2.4 contains an incomplete entry "You ignored the".
"W3C" is used before its first explanation.

Blah. That xml conversion is weird in places. I added a link that was
just dropped out. Its back in. I'll have to update the one for M/W
postings as well.

Garrett

 

[Dr J R Stockton]

In comp.lang.javascript message <[email protected]>

I'd rather put up an actual glossary in the FAQ and link to Wikipedia
as necessary.

That means more argument here. We should be wary of lengthening
Sections 1, 2, and 6 of the FAQ.