well, i can't tell you exactly what's going wrong, because it
looks like you have all the right operations in all the right
places.   i *can* nit-pick your code a little, though.  ;-)

the way you have the function laid out now:


> function getCookie(name){
> var cname = name + "=";
> var dc = document.cookie;
> var a = " ";
>     if (dc.length > 0) {
>     begin = dc.indexOf(cname);
>         if (begin != -1) {
>         begin += cname.length;
>         end = dc.indexOf(";", begin);
>             if (end == -1) end = dc.length;
>             return unescape(dc.substring(begin, end));
>         }
>     }
> return a;
> }


is concise, but a bit hard to read.   for one thing, you use a
different format for indenting code beneath an if() statement
than i'm used to.   that's not right or wrong per se.. pick a
style you're comfortable with and stick with it.. but you've
also used a trick that's technically legal, but generally
discouraged.

officially, the statement:

    if (some_condition)
        do_something();

works.   a single, atomic statement which follows a conditional
is executed if the condition is true, and ignored if the
condition is false.   the problem is that it becomes easy to
forget where you are, so the more formal way of writing the
statement:

    if (some_condition) {
        do_something();
    }

is preferred.   the braces aren't technically necessary in this
case, but they make the code easier to read.   as a matter of
fact, from what i can understand of your indenting style, you've
made an indenting error at that point in the function:

>         if (begin != -1) {
>         begin += cname.length;
>         end = dc.indexOf(";", begin);
>             if (end == -1) end = dc.length;
>             return unescape(dc.substring(begin, end));
>         }

should read:

>         if (begin != -1) {
>         begin += cname.length;
>         end = dc.indexOf(";", begin);
>             if (end == -1) end = dc.length;
>         return unescape(dc.substring(begin, end));
>         }

because the return() statement doesn't have anything to do with
the second if().. in my style, that part of the code would look
like so:

>         if (begin != -1) {
>             begin += cname.length;
>             end = dc.indexOf(";", begin);
>             if (end == -1) {
>                 end = dc.length;
>             }
>             return unescape(dc.substring(begin, end));
>         }

or, if i was concerned with compression (usually, i'm not):

>         if (begin != -1) {
>             begin += cname.length;
>             end = dc.indexOf(";", begin);
>             if (end == -1) { end = dc.length; }
>             return unescape(dc.substring(begin, end));
>         }




converting the whole function over to my style, with a couple
small changes in organization, makes it look like so:


function getCookie( name ) {
    var cname   = name + "=";
    var dc      = document.cookie;
    var a       = " ";

    begin = dc.indexOf (cname);

    if ((dc.length <= 0) || (begin == -1)) {        //  -1-
        return (a);
    }

    begin += cname.length;
    end = dc.indexOf (";", begin);

    if (end == -1) {
        end = dc.length;
    }
    a = dc.substring (begin, end);                  //  -2-
//  document.writeln("The substring is " + a);
    a = unescape (a);
//  document.writeln("The unescaped value is " + a);

    return (a);
}


//  1:  handling the exception cases first allows you to move the
//      main logic out to the top level of function scope.   it looks a
//      bit cleaner, but the real payoff is that it becomes much easier
//      to go back and play with the code.

//  2:  it's generally a good idea to put each operation on its own
//      line.   it's absolutely impossible to say whether consecutive
//      assignments or fall-through return values are more efficient
//      unless you have a deep, meaningful relationship with your
//      compiler's optimization code, and the difference is negligible
//      in any case.   the primary value is that it lets you toss in
//      debugging code at each stage of the process, to see just where
//      things start going wrong.



i haven't tested the code to see if it works the way it's
supposed to, but it should make the glitch easier to track down.







mike stone  <[EMAIL PROTECTED]>   'net geek..
been there, done that,  have network, will travel.

Reply via email to