search

apostrophecms / random-words

Generate one or more common English words. Intended for use as sample text, for example generating random blog posts for testing
MIT License
249 stars 72 forks source link

Addition of dictionarySize option #37

Closed prateek-budhiraja closed 1 year ago

prateek-budhiraja commented 1 year ago

Summary

What are the specific steps to test this change?

Use the below code for testing

import randomWords from "random-words";

console.log(randomWords({ dictionarySize: true }));
console.log(randomWords({ dictionarySize: true, exactly: 5, join: " " }));
console.log(randomWords({ dictionarySize: false, exactly: 5, join: " " }));

What kind of change does this PR introduce?

(Check at least one)

Make sure the PR fulfills these requirements:

If adding a new feature without an already open issue, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

BoDonkey commented 1 year ago

Three quick thoughts from glancing at your PR:

  1. Is there a reason not to pass back the dictionary size as the first value and then the words in whatever form the user requested them?
  2. The typescript declaration file must also be updated for your new option.
  3. Your code needs a little linting; the spacing is off compared to the rest of the code.

You can make changes, push them to the repo, and then let me know again in Discord. Thanks! Great job.

boutell commented 1 year ago

Alternatively, we could export two named functions, generate and count, which would probably call a shared implementation function inside the module. That would be more natural syntax I think.

nellfs commented 1 year ago

Alternatively, we could export two named functions, generate and count, which would probably call a shared implementation function inside the module. That would be more natural syntax I think.

I had doubts about count and generate, could you help me understand this better?

So count would only return the number of words in the dictionary?

What would generate do? Would it be like count, but with options such as minLength and maxLength, and then return a word value based on the options, like a word quantity filter? The name "generate" makes me think I am wrong about this question.

boutell commented 1 year ago

What I am saying is that if the module will be able to do more than one thing, that is either generate words or give you a count, then it should have two named exports, and no default export. It is often a mistake to assume there will only ever be one export from a module and now we have a chance to address that because this will be a new major version number anyway.

On Tue, May 9, 2023 at 10:22 AM nellfs @.***> wrote:

Alternatively, we could export two named functions, generate and count, which would probably call a shared implementation function inside the module. That would be more natural syntax I think.

I had doubts about count and generate, could you help me understand this better?

So count would only return the number of words in the dictionary?

What would generate do? Would it be like count, but with options such as minLength and maxLength, and then return a word value based on the options, like a word quantity filter? The name "generate" makes me think I am wrong about this question.

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/random-words/pull/37#issuecomment-1540227021, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27MAOXVXBHU4JFZWYA3XFJHJRANCNFSM6AAAAAAXMYTRV4 . You are receiving this because you commented.Message ID: @.***>

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his