p5.js icon indicating copy to clipboard operation
p5.js copied to clipboard

`nf` seems to have wrong prototypes

Open IronBlood opened this issue 1 year ago • 3 comments

Most appropriate sub-area of p5.js?

  • [ ] Accessibility
  • [ ] Color
  • [ ] Core/Environment/Rendering
  • [ ] Data
  • [ ] DOM
  • [ ] Events
  • [ ] Image
  • [ ] IO
  • [ ] Math
  • [ ] Typography
  • [X] Utilities
  • [ ] WebGL
  • [ ] Build process
  • [X] Unit testing
  • [ ] Internationalization
  • [ ] Friendly errors
  • [ ] Other (specify if possible)

p5.js version

main branch

Web browser and version

No response

Operating system

No response

Steps to reproduce this

This is generally a report about the documentation and test cases.

From the reference, the prototypes for nf are:

  • nf(num, [left], [right])
  • nf(nums, [left], [right])

However, from the source code, especially in doNf(num, left, right), there isn't check of type left === 'undefined', nor a default value for left, so when calling leftPart = leftPart.padStart(left, '0'), left cannot be undefined.

Also in the test suite of p5.prototype.nf, test cases are missing:

  • result = nf(1234), if left is optional
  • result = nf([1234, 3.141516, 3.141516e-2], 2) for an array of numbers to format

IronBlood avatar Apr 27 '24 01:04 IronBlood

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!

welcome[bot] avatar Apr 27 '24 01:04 welcome[bot]

Hey, I am intrested in this isssue. As you said here.

 However, from the source code, especially in doNf(num, left, right), there isn't check of type left === 'undefined', nor a default value for left, so when calling leftPart = leftPart.padStart(left, '0'), left cannot be undefined

did you meant that left can be undefined because that will make more sense if i am not wrong.

Akhilbisht798 avatar May 16 '24 10:05 Akhilbisht798

I can be wrong in this but i think p5._validateParameters('nf', arguments); Here the arguments are being checked so that they are always correct or something of that sort. But for test cases i think you are right we should have a test for array and test for only right argument.

Akhilbisht798 avatar May 16 '24 10:05 Akhilbisht798

I too agree with @Akhilbisht798, I think the same, only test cases should be updated , if no one working on it should I add missing test cases?

Rishab87 avatar Dec 22 '24 16:12 Rishab87

@Rishab87 hey this issue is resolved and merged.

Akhilbisht798 avatar Dec 23 '24 05:12 Akhilbisht798

@Akhilbisht798 thank you for letting me know

Rishab87 avatar Dec 24 '24 11:12 Rishab87