processing / p5.js

p5.js is a client-side JS platform that empowers artists, designers, students, and anyone to learn to code and express themselves creatively on the web. It is based on the core principles of Processing. http://twitter.com/p5xjs —
http://p5js.org/
GNU Lesser General Public License v2.1
21.65k stars 3.32k forks source link

"static" methods are documented in a confusing way #1336

Closed toolness closed 6 years ago

toolness commented 8 years ago

Here's an example from p5.Vector.fromAngle():

static

I think this might be confusing for beginners. And even though I know what a static method is, I was actually confused by it at first myself.

I'm not sure what a better approach might be, though... Perhaps replacing it with the sentence "This is a static method.", and hyperlinking the word "static" to the glossary concept we discussed in #1330? (We could replace "method" with "function" too, if that makes things easier to understand.)

Another thing that might maybe make static methods easier to grasp is to change the Syntax section to actually include the fully-qualified name of the function, e.g. p5.Vector.fromAngle(angle), since that's always required to call the function.

Also curious if @iamjessklein has any thoughts on this.

shiffman commented 8 years ago

I would be happy to completely revisit how these functions are called. These static methods arrived in the p5.Vector object from being ported over from PVector. I think it's not too late to reinvent how this works in p5. For example:

p5.Vector.fromAngle(angle) --> createVector(angle) p5.Vector.add(v1, v2) --> addVector(v1, v2)

With Processing we didn't want to pollute the global functions with a zillion vector math functions, but maybe that's less of an issue here.

Thoughts? What is more native to JS? I guess there are things like Math.random() so static is a concept that exists.

toolness commented 8 years ago

Hmmm, good question... I would say that one thing I like about keeping the namespace "flat" (i.e. everything a global) is that it makes it easier for me to skim through p5js.org/reference/ and find the function I'm looking for: as the reference is currently structured, finding fromAngle might be difficult because it's "buried" to some extent in the reference page for p5.Vector. That said, the reference page could also potentially be restructured to list all the static p5.Vector methods directly on this page, in which case they wouldn't really be buried anymore.

But flattening everything is also consistent with the way most p5 functions are already named. For example, we have textAlign() and textLeading() rather than p5.Text.align() and p5.Text.leading().

I would also say that the "flatness" is something I think really distinguishes p5 from other JS libraries and makes it easier for beginners to grasp. If all of p5's API were refactored to follow the same approach as p5.Vector, I'm concerned that really simple, elegant sketches would suddenly look quite verbose and a bit intimidating to newcomers, but I could be wrong.

So I guess all this is to say that I'm cautiously in favor of the flattening :grin:

toolness commented 8 years ago

Oh! There's also a technical consideration here... Because p5.Vector.fromAngle() isn't called with an actual p5 instance as its this property--instead, this is set to p5.Vector--there's no way it can access p5 instance properties like the current angleMode! Which means that all those static functions have to accept and return radians as arguments, I think... At least, one would think that the following would work:

angleMode(DEGREES);
var v = p5.Vector.fromAngle(90);

Based purely on the way the rest of p5 works, one would expect v to be {x: 0, y: 1}, but it's not, because fromAngle can't access angleMode, so it interprets 90 in radians.

(Sorry, this might be totally obvious to others but I only just realized it now.)

There's some weird alternatives to this that I've implemented in p5.play, but I'm not a huge fan of them... I think the simplest way to fix this problem from a purely technical standpoint is just to make the static functions globals instead.

toolness commented 8 years ago

Yet another potential disadvantage of static methods: because of the current URL structure of the documentation, if a static method can also be used as an instance method--for example, p5.Vector.sub--we're forced to document both versions on the same page, which is rather confusing. This could potentially be fixed by restructuring the documentation, though.

indefinit commented 8 years ago

I personally really like having both static and instanced versions in the p5.Vector object, not the global namespace. I know we want the library to be as user (and beginner friendly) as possible, but vectors aren't exactly a beginner topic to begin with so simplifying the api with target usage for a beginner might be a little unnecessary. I like your idea about hyper linking "static" though. Another option is that this could be fixed by a simple static vs instanced tutorial.

Also just a heads up (not meant to persuade one way or the other) under the hood I'm making considerable use of both instanced and static vector methods in the various webgl objects so any changes to p5.Vector might be a breaking change for webgl.

shiffman commented 8 years ago

Good points @indefinit. With Processing, the static methods were also necessary b/c PVector is a stand-alone Java class that can be used on its own with no awareness of a PApplet instance. Does p5.Vector only makes sense in the context of p5 itself? We have createVector() for example which requires a p5 instance.

I'm really torn and could go either way. Perhaps best to stay as is for now.

Re: a tutorial I am currently writing a JS version of The Nature of Code book. I'll be adapting this "static vs. non-static" section for p5/JS and would be glad to include it on p5js.org or the p5.js wiki etc.

http://natureofcode.com/book/chapter-1-vectors/#19-static-vs-non-static-functions

Spongman commented 7 years ago

this is even more complicated when there are multiple overloads of the same method, some of which are instance methods, and others are static.

for example, https://p5js.org/reference/#/p5.Vector/add

the 'static' keyword isn't shown here simply because the first overload just happens to be an instance method. https://github.com/processing/p5.js/pull/2056/commits/73675821d88e9f591ed060c964920ca7be651230 made the syntax for static method explicit in the reference, so i think it's okay to just remove the 'static' keyword from the reference, now.

lmccart commented 6 years ago

thanks for all the thoughts here. I agree with @Spongman and @toolness let's remove the potentially cnofusing 'static' keyword, and just try to be really explicit in the documentation. I think p5.Vector does a good job, and this is potentially a place to link the word "static" in the description to a definition