Request - generate words starting with letter about random-words HOT 18 OPEN

How to work on this issue

1. Fork this repo into your own account
2. Make changes to the code - since it is in your account, you could work directly on main, but wouldn't you rather get some Git practice by working on a branch, and then merging to your own main?
3. Write one or more tests to make sure your feature works
4. Make sure your code changes don't break any existing tests.
5. Update the CHANGELOG.md file - since other people might contribute during the same sprint cycle, add your change log message under 'UNRELEASED', not a specific version. We will add the version number when everything is approved and published on npm.
6. When you are done, return to this repository and create a PR to pull code from your fork. Read more about this here. Make sure to fill out the PR template as best as you can.
7. Navigate to our Discord server here if you have already joined, or here if you need an invitation and post a message in the open-source contribution channel that you need a PR review.
8. After (hopefully) a short amount of time, one of our team engineers will review your PR and potentially advise you about things they want to see changed.
9. If you need to make changes, go back to your local fork, make changes, and contribute those back to the main repo. This will update the PR.
10. When you are done with changes and want a re-review, ask in Discord once again.
11. After your PR is accepted, celebrate!!! 🎉

Code suggestions for this issue

If you look at the generateWordWithMaxLength() function, this suggests a way to address this issue. In that function, a random word is generated and then checked against the size the user wants. If that size isn't correct, the function generates another. Not the most elegant, maybe, but very straightforward.

One edge case you need to worry about is if there aren't any words starting with that letter or if the user wants ten words starting with z and there are only four. Right now, there aren't any guardrails in place because the other options can almost always be fulfilled. Suggesting how you would take care of this either here on the issue or in the Discord would be great to get feedback and make sure you are on a good track.

Finally, please make use of the Discord to ask questions. Try to answer the questions yourself using internet resources, but don't be afraid to ask questions on the Discord about anything. We are here to help!

from random-words.

Hi @bhaveshittadwar,
Thanks for digging into this.

1 ) Yes, we want the user to be able to pass a key like startsWith: 'a' within the options object.
2) This new functionality should merge with the existing functionality. So, the user should be able to pass { minLength: 5, maxLength: 10, startsWith: 'c' }. You can check out the readme and the tests for the current options that are working.
3) As you saw in that code review, @boutell had concerns about the loop in general. While using a break to exit the while isn't bad, I would try to refactor to not have to either break or throw an error. As was said in that comment, it would be good to check if minLength, maxLength, and startsWith can all be satisfied out side of the possibility of an endless loop. You can then perform an early empty return if any can't be. I'm loath to throw an error. It would be up to the library user to check for an empty string and treat it accordingly.

There should be tests for any of those three returning early.
Does this all make sense?

from random-words.

@UnKnoWn-Consortium PRs always welcome! Thanks.

from random-words.

I would like that aswell.

from random-words.

I can do a pull request, but the gist of my change would be:

  function generateRandomWordStartingWith(startingLetter){
    const filteredList = wordList.filter(word => word.startsWith(startingLetter));
    return filteredList[randInt(filteredList.length)];
  }

Notes on code suggestions:

• unlike generateWordWithMaxLength, we can quickly figure out the start of each word in our wordList. and whether they would be a valid pick or not.
• the case where there aren't any words starting with that letter (e.g: ñ) is to be considered. The easiest solution is to return an empty list, but other alternatives do exist.
• the library already outputs the same word more than once occasionally
  (words({exactly: 3}) can already give as an output ["might", "might", "rubbed"] which has repeated words)
  so having only 4 words that start with z while the user wants 10 will repeat words, as asking for 3000 words regularly would (there are 1952 words in the wordList)

Let me know if it's better if we talk about this on discord or on an actual PR. Thanks for working on this awesome library btw
:D

from random-words.

Hi @ldd
"Let me know if it's better if we talk about this on discord or on an actual PR. " Why can't we do both!?😀
So a PR for this would be great. Overall, it seems like a good approach - how are you going to trigger the function?
What do you suggest for handling something like ñ?
I think the repeated words will work just fine.
Cheers,
Bob

from random-words.

Is this issue still open? Can I attempt it?

from random-words.

I haven't heard anything from @idd since the last message, so I guess they are not going to work on it. Read the notes and give it a try! Let me know in our Discord or here when you need a code review.

from random-words.

Are the users supposed to specify the amount of words to be generated along with the starting letter? Or should I print all the letters starting with the specified letter?

from random-words.

This would be an add-on option. So a user should be able to specify that they want 5 words all starting with 'a', for example. But, they should also be able to leave off how many need to be returned if they only want one.
I don't know the current library coverage, so you will also have to deal with there not being any words that fit the criteria in the library.

from random-words.

Hi @BoDonkey , here's the link to index.js file of my attempt at the issue: https://github.com/Suyash699/random-words/blob/main/index.js

from random-words.

Hi @Suyash699, Thanks for trying this! At first glance, your function to filter the words looks reasonable. Although I would say that charAt(0) is likely more efficient. The only thing is, what if the user wants 3 words, each 6 letters, that start with 'c'? Your code needs to take this into account. Right now it looks like you are returning every word that starts with that letter in the dictionary.
The other thing that I'm not sure about is this line: if(options[0].toLowerCase>= "a" && options[0].toLowerCase<= "z") What are you trying to test here? What do you expect options[0] to return?
Cheers,
Bob

from random-words.

One thing I can see is that "toLowerCase" is a method, but you are not invoking it, so you are comparing functions themselves, not strings.

I suggest that you open a PR with your current attempt and mark it as draft if it's not ready, so it's easier to give feedback.

from random-words.

Hi @BoDonkey, will this work? :

function generateRandomWordStartingWith(startingLetter){
     const filteredList = startingLetter.charAt(0).toLowerCase().charCodeAt(0) >= 97 && startingLetter.charAt(0).toLowerCase().charCodeAt(0) <= 122 ? 
  wordList.filter(word => word.charAt(0).toLowerCase() === startingLetter.charAt(0).toLowerCase())
  : "error";

return filteredList.length === 0 ? null : filteredList[randInt(filteredList.length)];
 }

So basically, added the previous function in a ternary operator that checks if the given letter falls in the unicode range of a-z or not, and if yes, then checks whether it exists in the filteredList or not. I am still a bit confused as to how the "options" parameters work, so still haven't been able to solve the "how many words will you print?" issue.

As for the "toLowerCase" problem, I have corrected that as well. I'm considering making a PR as well @boutell

from random-words.

@Suyash699 - Check out how the other options in the repo work. Basically, the user is passing parameters to the main function. Those parameters dictate what will be returned to the user. For example, if the user makes a request like randomWords(5) then they will get back five random words from the dictionary. You can look through the tests and the README.md to learn a little more. Looking at the index.js file, at around line 2013 there is a conditional that says

if(options === undefined) {
  return word();
}

That conditional is looking at whether the user passed any arguments. If they didn't then return a single random word. You need to fashion your code in a similar way. You need to change what is returned to the user after checking if the option you designate is there and has a certain value. Take a look at the word() method. It keeps generating words until it has one that is the rightSize. You could do the same - keep generating new words until it has one with the correct first letter. However, you have to make sure that if there aren't any words that start with a particular letter that match the other parameters, like word length, the program doesn't try forever. Make sense?
Cheers,
Bob

from random-words.

Hi @BoDonkey , I have raised a PR - #42
And Need a PR Review.

from random-words.

Hey @BoDonkey,

I have been playing around with this module for a while now & I have a few questions:

1. Do we want to get words by just matching the first letter and not a multi-letter prefix subsequence? (This comment indicates YES, although it's never harmful to re-affirm)

2. I gather from this comment that we want to be able to gather the exact amount of words for the criteria mentioned in the options object (minLength, maxLength, join, formatter, etc). Do you know if we need that functionality in this ticket itself or was it an open discussion and nothing was fixed yet?

3. Is throwing an error to escape the while loop the way to go(ref: https://github.com/apostrophecms/random-words/pull/42/files#r1303500077) or can I just put a regex/character check over characters like 'ñ' to see if they are valid English alphabet? If not, I could just break the while loop.

from random-words.

@BoDonkey I see there is a stale #42. Mind if I submit an alternative?

from random-words.