ClosestStorm / v8cgi

Automatically exported from code.google.com/p/v8cgi
BSD 3-Clause "New" or "Revised" License
0 stars 1 forks source link

Do not print 'undefined' when there is no value in templates #13

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I reckon it is better to output nothing instead of 'undefined' when there
is no value in the Template module's $() operator.

I guess it can be done by replacing line 48 in template.js
(Template.prototype._tokenCommand)
       return "__output += "+str+";";
with the following:
       if (str) return "__output += "+str+";";
       else return "";

Original issue reported on code.google.com by rand0mbond@gmail.com on 20 Jun 2009 at 9:58

GoogleCodeExporter commented 9 years ago
Yes, good idea. Your proposed fix is not sufficient, though (because this will 
also
apply to 0, "", false etc.).

Original comment by ondrej.zara on 21 Jun 2009 at 9:54

GoogleCodeExporter commented 9 years ago
Yeah, surely it should be:
 if (str!==undefined) return "__output += "+str+";";
       else return "";

Original comment by rand0mbond@gmail.com on 22 Jun 2009 at 7:06

GoogleCodeExporter commented 9 years ago
Actually, I spent some time thinking about this issue and I decided that current
behavior is correct. User should have maximum control about the output and 
"hiding"
his mistakes (such as passing undefined variable to template) in fact makes 
debugging
a lot harder (there is no "undefined" keyword in output which points to a 
problematic
place).

Original comment by ondrej.zara on 22 Jun 2009 at 7:44

GoogleCodeExporter commented 9 years ago
Hmm, well, you are right that it is wrong hiding errors, but well, sometimes it 
is
much easier to have that kind of behaviour, so that there is no need to check 
every
variable manually.

In my opinion this behaviour should be configurable somehow. 

And yes, there is no any "undefined" keyword, but it is comfortable to use it 
since
its value is indeed undefined if you didn't define id :D

This code should 100% correct:
if (typeof str != "undefined") return "__output += "+str+";";
       else return "";

Original comment by rand0mbond@gmail.com on 22 Jun 2009 at 7:57

GoogleCodeExporter commented 9 years ago
Actually, there *is* a keyword "undefined" (without quotes) in Javascript. 
Experiment
with it a bit in Firebug's console to see how it works...

However, I strongly believe that one should be 100% responsible for the data 
sent to
template. Transparently (automagically) fixing undefined variables encourages 
sloppy
programming, at least in my humble opinion.

Original comment by ondrej.zara on 22 Jun 2009 at 9:01

GoogleCodeExporter commented 9 years ago
Yeah, probably, I didn't read the spec... but
this code
    var undefined = 123;
    alert(undefined);
shows 123 in Firefox and Opera.

This code:
        undefined = 123; console.debug(undefined);
prints 123 in Firebug,

and finally, this code:
        var undefined = 123;
        response.write(undefined);
outputs 123 in v8cgi :)

Original comment by rand0mbond@gmail.com on 22 Jun 2009 at 9:21

GoogleCodeExporter commented 9 years ago
Yes, you are simply overwriting the "undefined" variable with your own data. 
You can
do the same with other builtin objects - "eval" etc.

Try this (prior to overwriting undefined):

- alert(x); /* error */
- alert(undefined); /* works, e.g. undefined exists */
- x = 3; /* x is now defined */
- x = y; /* error, y is not defined */
- x = undefined; /* works, x is now undefined */

Original comment by ondrej.zara on 22 Jun 2009 at 9:28

GoogleCodeExporter commented 9 years ago
Well, but anyway it is a variable which is _defined_ with the "undefined" 
value, but
in the case of alert(x) - x _was not defined_ previously elsewhere. 

Okay, but this actually doesn't matter so much here :)

What you do think about making a parameter in templates, that would not display
undefined variables?

Original comment by rand0mbond@gmail.com on 22 Jun 2009 at 9:51

GoogleCodeExporter commented 9 years ago
Not sure. One can ask if this could be also done for null values, for false 
values,
for zeros.. in my opinion, this just encourages bad coding.

Are you able to show an example where this option would be actually useful? 
Where
there *is* a reason to pass undefined value to template and expect empty string 
in
result?

Original comment by ondrej.zara on 22 Jun 2009 at 9:55

GoogleCodeExporter commented 9 years ago
Here is an example, currently I need to write like this:

   $code( if (data.something) {) $(data.something) (})
or
   $( data.something ? data.something : "")

when it would be much easier to have just
   $(data.something)

Of course, I can check if this variable is not set and then set it at some other
place, but this just will include a lot of unnecessary code, which is 
completely useless.

There is abolutely no reason to display "undefined", except for debuging 
purposes,
however, if the value is not displayed - then it will also indicate that there 
is no
value.

It can be just a simple parameter to the template constructor, which is by 
default
can be false, so that it needs to be explicitly set for not displaying 
"undefined"s :)

Original comment by rand0mbond@gmail.com on 22 Jun 2009 at 10:27

GoogleCodeExporter commented 9 years ago
Well, technically, there is no problem with implementing the suggested change. 

The main reason why I don't want it there is that in my opinion, one should not
expect having undefined data in template. It is not a responsible coding: you 
prepare
a data object (to be passed to template) and you set its properties because you
_know_ that they will be used in template. Similarly, template accesses only 
those
properties which should have been previously prepared by the data provider.

Can you also show the code that builds the "data" object, so I can see the need 
for a
non-deterministic property?

BTW, you can even simplify your expression by typing $(data.something || 
"defaultValue").

Original comment by ondrej.zara on 22 Jun 2009 at 10:39

GoogleCodeExporter commented 9 years ago
I use the MVC model, and I pass the controller itself as a parameter to the 
template.
And since different actions might occur - different variables may be set or not.

Thanks for the tip!

Original comment by rand0mbond@gmail.com on 22 Jun 2009 at 12:26

GoogleCodeExporter commented 9 years ago
I believe that is is not architecturally correct to pass Controller itself to 
the
View. Controller should request relevant data from Model and let View visualize 
them
(quoting wikipedia MVC article: "The view gets its own data from the model").

That said, a common MVC-based workflow should be:
a) controller performs necessary logic and retrieves data from model,
b) view uses received data to create the resulting page.

However, we are getting a bit offtopic here. I discussed this stuff with several
other people and the overall conclusion is that template should act 
transparently,
outputting exactly the same data that it receives. Therefore, this issue will 
remain
in its current state.

Original comment by ondrej.zara on 22 Jun 2009 at 12:41

GoogleCodeExporter commented 9 years ago
Yes, you are absolutely right! 

In my case it is just much easier to do so, but I definitely need to think more 
on
this case.

Anyway, your tip $(data.something || "defaultValue") will do almost what is 
need, I
think you should mention it in the wiki

Original comment by rand0mbond@gmail.com on 22 Jun 2009 at 12:50