Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default values of arguments #6721

Closed
mohitbalwani opened this issue Jan 9, 2024 · 15 comments · Fixed by #6807
Closed

Default values of arguments #6721

mohitbalwani opened this issue Jan 9, 2024 · 15 comments · Fixed by #6807

Comments

@mohitbalwani
Copy link
Contributor

Topic

I noticed that the default values for arguments are being set using if ladders. I think we can pass the defaults in the function's parameters itself to save lots of lines of code. But before jumping into making those changes I wanted to know the reason behind setting them like that. What do you think about changing the way of setting the default values?

p5.prototype.cone = function(radius, height, detailX, detailY, cap) {
  this._assert3d('cone');
  p5._validateParameters('cone', arguments);
  if (typeof radius === 'undefined') {
    radius = 50;
  }
  if (typeof height === 'undefined') {
    height = radius;
  }
  if (typeof detailX === 'undefined') {
    detailX = 24;
  }
  if (typeof detailY === 'undefined') {
    detailY = 1;
  }
  if (typeof cap === 'undefined') {
    cap = true;
  }
Copy link

welcome bot commented Jan 9, 2024

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!

@GregStanton
Copy link
Collaborator

Great find @mohitbalwani!

This code was introduced in commit 4102551, which dates to 2017. Setting default values in this way was common until ECMASCRIPT 2015 (aka ES6), which was the specification that introduced many of the features we now refer to as "modern JavaScript," including default parameters. It took developers some time to adopt the new features. For example, here's an article from 2018 that still refers to these features as "new." So, I suspect this usage is a result of developers not being familiar with the new syntax, or wanting to maintain consistency with the old approach.

We're just starting work toward p5.js 2.0, and one of the goals is to thoroughly modernize the p5.js codebase. The issue you found would likely be a part of that effort. I'll go ahead and link to your issue from the main discussion about p5.js 2.0, which is happening in #6678, so that we can figure out the best way to proceed.

@mohitbalwani
Copy link
Contributor Author

Thanks for the detailed explanation @GregStanton! I'm excited about the ongoing work on p5.js 2.0 and would love to contribute to the modernization effort. If there's any specific guidance or tasks related to this issue that I can help with, please let me know. Looking forward to being a part of the progress!

@GregStanton
Copy link
Collaborator

I'm glad to help @mohitbalwani!

To get started, you could read my informal modernization proposal and then indicate whether you're interested in leading that effort or whether you'd like to work on specific tasks like the one you identified.

If you want to lead the effort and help volunteers to contribute, I suggest that you write up a detailed proposal indicating the types of modernizations you propose and how you would break up the work into manageable tasks. However, do note that we're currently figuring out a new issue template for the p5.js 2.0 proposals, and it will be easier to make the proposal once that template is available. If you prefer to start immediately, you're still welcome to submit a proposal now; in that case, I would use the Existing Feature Enhancement issue template.

@mohitbalwani
Copy link
Contributor Author

Thank you, @GregStanton! I'm excited about the idea of leading the effort to modernize p5.js codebase. I'll start by reading your informal proposal and then work on a detailed proposal outlining the modernization plan. I agree that breaking it down into sub-issues will make it more manageable. I'll keep an eye on the new issue template for p5.js 2.0 proposals and ensure that the proposal aligns with the community's goals. Looking forward to contributing to this important initiative!

@GregStanton
Copy link
Collaborator

That's fantastic @mohitbalwani! I'm looking forward to seeing what you put together!

@GregStanton
Copy link
Collaborator

Hi @mohitbalwani! The new issue form for p5.js 2.0 proposals is live!

@vaishnavi192
Copy link

Hery @GregStanton I wanted to know what skills should I have in order to contribute in something as big as changing from p5.js to p5.js 2.0 ?

@GregStanton
Copy link
Collaborator

GregStanton commented Feb 6, 2024

Hi @vaishnavi192! Thanks for your question!

The same skills that are usually helpful will also be helpful for p5.js 2.0. No special expertise is required. Here are some skills that are generally helpful:

  1. For code contributions, a working knowledge of JavaScript, as well as Git and GitHub, is helpful. Here's a nice tutorial series on Git and GitHub for anyone who is new to those things. For details about contributing to p5.js specifically, you can check out the p5.js contributor docs.
  2. For other kinds of contributions, like outreach, the main thing that's needed is a desire to help!

Here a couple of specific ways to help:

  1. Inform others that proposals are being accepted for p5.js 2.0. Priority will be given to proposals that would change existing features in a way that's difficult to do under normal circumstances (breaking changes, systemic changes, or overdue changes). For example, a breaking change to p5.js will make people's sketches work differently even if their own code stays the same; here's an example of a proposal that involves breaking changes. We need to give people a lot of advance notice before making these kinds of changes, so we can only make them as part of a major release like 2.0. However, that doesn't mean that making these changes requires any special skills.
  2. Provide feedback on existing proposals. For example, if you're not a total expert in JavaScript, that can actually be helpful, since proposals like #6767 depend on whether the changes will be understandable to beginners.

Note that the list of current proposals is maintained at the top of issue #6678. I expect more proposals will get added as people continue to use the p5.js 2.0 proposal form.

I hope that helps!

@GregStanton
Copy link
Collaborator

Hi @mohitbalwani! Are you still planning to submit a proposal to modernize syntax in the p5.js codebase? As a reminder, the informal idea is outlined in this comment. It's totally fine if you're too busy or if you need some more time. I just thought I'd check in, since if you won't be able to work on this, I can submit the proposal or else find another contributor who'd like to do it.

@vaishnavi192
Copy link

@GregStanton Thanks for the great explanation! Rn I have medium level proficiency in js. Currently gaining more understanding about p5.js and js. So, like as you mentioned in #6767 issue above in comments, how can I help them as a beginner, I have to tell them my reviews as a beginner?

@mohitbalwani
Copy link
Contributor Author

@GregStanton I will submit proposal very soon. I apologize for the delay in my response, I was traveling/vacationing and just returned back.

@GregStanton
Copy link
Collaborator

Hi @vaishnavi192! That's a good question. If you read over issue #6767 and some of the comments, you'll see that "beginners" are mentioned quite frequently. That's because we're trying to guess what will be hard for beginners to understand.

So, one way to help is to make a comment saying something like "Hi! I noticed you've all been trying to determine what's easiest for beginners. I'm kind of a beginner and know some JavaScript, but I don't have experience with async/await. Whenever you all narrow down the options, I'd be happy to to let you know which ones I think are easiest to understand!" They're all very friendly, and I'm sure they'd appreciate your effort.

You might try something similar with a couple of other issues and see if anyone takes you up on your offer. A quick way to figure out if you can help in this way is to use CTRL+F/CMD+F to quickly find if a discussion includes the word "beginner." If so, you can read the comments to see if it seems like they'd benefit from a beginner's perspective.

I hope that helps! Let me know if you have any other questions :)

@GregStanton
Copy link
Collaborator

Hi @mohitbalwani! No worries! Take your time. I just wanted to ask in case you had moved on to something else.

@vaishnavi192
Copy link

@GregStanton Yes I got it coz of your detailed explanation. Thanks, will try to help for sure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Completed
Development

Successfully merging a pull request may close this issue.

4 participants