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

Added strings/wordCount #377

Closed
wants to merge 1 commit into from
Closed

Added strings/wordCount #377

wants to merge 1 commit into from

Conversation

dshubhadeep
Copy link
Contributor

No description provided.

@CLAassistant
Copy link

CLAassistant commented May 3, 2019

CLA assistant check
All committers have signed the CLA.


const cleanedInput = cleanString(input);

return cleanedInput.split(" ").length;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does (input.trim().match(/\s+/g) || []).length also work here?
(Not sure which performs better.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hayd Yeah it works, but your code is returning the (no. of words - 1). So, incrementing by 1 will result in the correct output.
I compared both the methods using Performance API. There is a negligible difference in the performance

@ry
Copy link
Member

ry commented May 6, 2019

Thank you for the contribution but this functionality is too specific for deno_std. The criteria I'm using is whether similar functionality is found in Go's standard library.

Rather than having a wordCount, I would accept strings.fields() which should operate like this:
https://golang.org/pkg/strings/#Fields

The tests and documentation should be ported from Go: https://github.com/golang/go/blob/b41eee4c8a2fe692c1d9fb46972b9047b5dc02b7/src/strings/strings_test.go#L462-L493

@ry ry closed this May 6, 2019
@dshubhadeep
Copy link
Contributor Author

@ry No problem. I will start working on fields implementation.

@ry
Copy link
Member

ry commented May 6, 2019

@dshubhadeep actually I forgot we have String.split already... so I guess fields isn't necessary either.

@j-f1
Copy link

j-f1 commented May 6, 2019

They behave differently, though.

// input:
const s = "  foo bar  baz   "
// Go:
import "strings"
strings.Fields(s) => ["foo" "bar" "baz"]

// JS
s.split()      => ["  foo bar  baz   "]
s.split("")    => [" ", " ", "f", "o", "o", " ", ..., "z", " ", " ", " "]
s.split(" ")   => ["", "", "foo", "bar", "", "baz", "", "", ""]
s.split(/\s+/) => ["", "foo", "bar", "baz", ""]

@dshubhadeep
Copy link
Contributor Author

@ry Oh. Is there anything that needs to be added to the strings module ?

@ry
Copy link
Member

ry commented May 6, 2019

@j-f1 @dshubhadeep Do you think it's worth it to add fields() as an alias for s.split(/\s+/).filter(x => x.length > 0) ?

@dshubhadeep
Copy link
Contributor Author

@ry No, it just feels like an unnecessary abstraction.

inverted-capital pushed a commit to dreamcatcher-tech/napps that referenced this pull request Oct 24, 2024
closes denoland#377.  

this disallows users from submitting duplicate entries.  
upon submission all user entries are checked for uniqueness on either
`title` or `url`.

---------

Co-authored-by: Asher Gomez <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants