Skip to content
This repository has been archived by the owner on May 9, 2019. It is now read-only.

Improvements & Feature .. #28

Closed
wants to merge 2 commits into from
Closed

Conversation

cloned2k16
Copy link

proposing a re factory which includes some improvements
and pointing out a BUG which may be a feature but the have to be handled properly ...


// doesn't need to pad
function leftPad (str, len, ch) {
var pad = ''
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cloned2k16 did you mean to write the var definition like this?
var pad = '', cache;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to write it as I did .. which is the same ... isn't ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't the interpreter add a semicolon at the end of cache? So, you would end up with 2 semicolons:

var   pad     = ''
,     cache;
;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sort of ...
but I prefer to add the last one for clarity of the code and better portability ..

it doesn't harm having useless semicolons
(actually I dislike javascript ''''feature''' of ending line automatically )

@stevemao
Copy link
Member

The commit is huge. For people who are lazy to read the whole changes, The PR adds use-cases for

+  assert.strictEqual(leftPad('feature'  ,  8, 'bug or '), 'bug or feature' ); 
+  assert.strictEqual(leftPad('feature'  , 10, 'buggy ') , 'buggy buggy buggy feature' );

I don't know if anyone is using it.

@cloned2k16
Copy link
Author

cloned2k16 commented May 13, 2016

well , actually these two use case were supposed to highlight a BUG ..
which I didn't fix either ... because maybe can be converted in feature ..

the fact is .. as it is right now, is just a plain and simple BUG

because anyone going to what a function that pads a string with another,
expects it to count in a different (straightforward) way ..

eg ...

leftPad('feature' , 8, 'bug or ') ..

should be ..
leftPad('feature' , 1, 'bug or ')
instead ...

finally, If you just have a look to the whole file,
instead to just look to diff changes,
isn't so huge ...
while still true...
it changes a lot from original ...

whole file

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants