Switching From Prototype Functions To Class Declarations

[RinkiyaKeDad]

Hey everyone! I'm new to Music Blocks development so was looking for some good first issues. I stumbled upon #2629 which mentions switching from prototype functions to class declarations for a bunch of files.

I was wondering if there is something which I can have a look at as an example so that I have a clear idea while porting files. Is the intended goal something like this.

Thanks.

[meganindya]

More like #2697. @ksraj123 wanna brief?

hoisting, closures, and binding are important behaviors to consider.

[ksraj123]

Hi @RinkiyaKeDad , here are a few points already laid out by @meganindya in various PRs related to this,

In my opinion the optimal order would be:

1. prototype function to class
2. function style callbacks to arrow functions
3. remove occurrences of that (CAREFUL!)
4. Use const over let wherever feasible ... no vars pls

In short:

1. Top-level variables go inside the constructor, e.g. const foo = 50; becomes this.foo = 50;.
2. If the top-level prototype function is parameterized, the constructor now becomes parameterized. Note that those parameters will now only be visible inside the constructor, so create new instance variables with those values, and replace occurrences of the parameter variables anywhere else by these new instance ones.
3. One level nested this.myfunction = function( ... ) { ... } or this.myfunction = ( ... ) => { ... } become myfunction( ... ) { ... }. For async the async keyboard precedes the method name.
4. If any top-level constants are defined, make them static, e.g. const MYCONST = 50; becomes static MYCONST = 50; and accessed like MyClass.MYCONST.
5. Nested functions inside one-level nested functions (see 3), are a bad convention. So, function myFunc( ... ) { ... } becomes const myFunc = ( ... ) => { ... }. Consider effects of hoisting. Move stuff around accordingly, if necessary.
6. Inside functions, un-identified variables (not specified using var/let/const) are implicitly identified as var. In class definition, they need to be explicitly identified, else it throws error.
7. VERY careful with this and that. Maybe read up on binding and closures.

We are porting the code the ES6 but while we are at it, it would be great if we could keep a few other things under consideration, like

• Its a standard convention to use _(underscore) before private properties but it is not followed properly in the codebase, try to fix this if you encounter properties from other classes that are being used publicly and start with an underscore.

• try to make the code leaner but keep it easy to understand. Prettier configuration is present, so format the file you are working on using Prettier.

• have a look at issue logo/turtles object not required to be passed as argument #2594 , for small files you could fix this in the same PR. It would be easy to remove redundant instances or logo/turtles while porting as we would be reading the code anyways.

• have a look at issue Add JSDoc style documentation #2630 - for small files you could do this as well in the same PR as it would be easy to do while porting.

Also, please test all aspects of the feature you are porting thoroughly before and after and try to adhere to best practices and conventions. It would be beneficial to go through the comments of all the PRs related to this. Thanks

[meganindya]

I suggest not prettifying changes like these until the review. Since these are scattered, it becomes difficult to notice the actual differences if you format the code. Maybe add a format commit at the end, after the review. Or the reviewer can do it for you.

[meganindya]

Keep your PRs short; the less the number of files per PR the better. Makes it easier to review.