mikehostetler / amplify

AmplifyJS
http://amplifyjs.com
GNU General Public License v2.0
1.45k stars 143 forks source link

Better documentation #56

Closed tommck closed 12 years ago

tommck commented 12 years ago

I was originally going to say "better documentation for timeout / cache parameters", but I really feel like this is a product-wide issue.

Going to the "Store" method's documentation (http://amplifyjs.com/api/store/), the entry for "expires" is

expires: Duration in milliseconds that the value should be cached

That's it!

Missing information:

  1. What happens when it's undefined/null?
  2. What is the maximum value?
  3. What happens when it's negative?

There also seems to be no information on how to clear the entire cache or what happens when you do explicit storage usage and the browser doesn't support it.

I know there are forums where this information may be found, but someone should be updating the documentation with this information so we don't have the "needle in the haystack" approach to finding documentation (searching through forum posts)

scottgonzalez commented 12 years ago

I don't feel like this is a problem. Passing undefined, null, negative values are all invalid uses. There's no maximum value, so there's no reason to say that. There's no API to clear the entire cache, so there's no documentation for it.

tommck commented 12 years ago

I understand that they're invalid... that doesn't mean that I understand what amplify is going to do with the invalid values!

"there is no maximum value for X" is always a false statement in programming.

The max might be "the largest integer possible" or "2 gagillion". In this case, you tweeted to me that it is "whatever you can send to setTimeout" - well, that is not obvious from the outside and also makes me go search for what the max is for "setTimeout"

Documentation should then say something like "the maximum value for 'expires' is whatever the JS engine's maximum for setTimeout is. Currenlty that is XYZ in most browsers"

The docs should also say something like "all null values and values < X (1?) will result in some action (throw?)"

There should also be a mention of "clear the cache: there's no API to clear the cache, but this could be accomplished by doing: { var hash = amplify.store(); for.... }"

It doesn't seem like it would be that hard to share this information.

tommck commented 12 years ago

Apparently, some implementations of setTimeout will have totally unexpected results when passing a number larger than expected,

http://stackoverflow.com/questions/3468607/why-does-settimeout-break-for-large-millisecond-delay-values

So, does amplify do any bounds checking, or does it just fire it on through to setTimeout getting unexpected behavior?

scottgonzalez commented 12 years ago

I really can't get behind this. There's no way I'm going to document what happens in all cases of invalid values.

You're getting really pedantic asking for a clarification on the order of "the largest integer possible."

If we were to document everything that you can do in more than one API call, our documentation would be ridiculous. API docs are absolutely not the place for every possible tutorial.

tommck commented 12 years ago

This isn't being pedantic, it's asking for basic behavior of the code!

You're saying that it's too much to ask for you to tell me how your code is going to handle error cases???

tommck commented 12 years ago

For anyone else who cares, as far as I can tell, the setTimeout() limit for the length of the "expires" period is only valid when you are using in-memory storage. Otherwise, it clears it out on next access to the object (this is on 1.0 code)

Additionally, request caching's "duration" option has the same limit

NOTE: Neither of these implementations does any error checking to make sure the value is valid, though you should get errors with null, undefined and < 0 (most likely)... really large numbers are the real problem

tommck commented 12 years ago

And for those who would like to clear everything from storage, you could use something like this:

function () {

    // get all the keys from the store (assumes Object.prototype.keys() is present)
    var keys = Object.keys(amplify.store());

    // iterate over them and clear it out.
    for(var keyIndex = 0; keyIndex < keys.length; keyIndex++) {
        var key = keys[keyIndex]
        amplify.store(key, null);
    }

}