Skip to main content
Code Review

Return to Answer

Added suggestion for restructuring the code
Source Link
200_success
  • 145.5k
  • 22
  • 190
  • 479

To start with, unpronouncable should be spelled unpronounceable, or better yet, generate_unpronounceable().

I'm not convinced that this is a valid strategy. If it is a valid approach, then your lists of pronounceable syllables is incomplete. For example, unpronouncable(4) is capable of returning "test", which I consider to be a pronounceable word.

Your two-letter combinations are listed almost alphabetically, which makes the list hard to maintain. For example, "so" is out of place, "os" and "xi" are listed twice, and eight elements are haphazardly added at the end. The three-letter list doesn't appear to be in any recognizable order.

When you reject a candidate word, you use recursion to retry. In JavaScript, you should avoid using recursion when a simple while loop will do — especially when there is no bound to the depth of the recursion. The code should be reorganized along these lines:

function generate_unpronounceable(length) {
 var LETTERS = "abcdefghijklmnopqrstuvwxyz";
 do {
 var word = "";
 for (var i = 0; i < length; i++) {
 word += LETTERS[Math.floor(Math.random() * LETTERS.length)];
 }
 } while (is_pronounceable(word));
 return word;
}
function is_prounounceable(word) {
 ...
}

Decomposing the problem into generate_unpronounceable() and is_pronounceable() is also better because is_pronounceable() can be unit-tested more easily.

To start with, unpronouncable should be spelled unpronounceable.

I'm not convinced that this is a valid strategy. If it is a valid approach, then your lists of pronounceable syllables is incomplete. For example, unpronouncable(4) is capable of returning "test", which I consider to be a pronounceable word.

Your two-letter combinations are listed almost alphabetically, which makes the list hard to maintain. For example, "so" is out of place, "os" and "xi" are listed twice, and eight elements are haphazardly added at the end. The three-letter list doesn't appear to be in any recognizable order.

When you reject a candidate word, you use recursion to retry. In JavaScript, you should avoid using recursion when a simple while loop will do — especially when there is no bound to the depth of the recursion.

To start with, unpronouncable should be spelled unpronounceable, or better yet, generate_unpronounceable().

I'm not convinced that this is a valid strategy. If it is a valid approach, then your lists of pronounceable syllables is incomplete. For example, unpronouncable(4) is capable of returning "test", which I consider to be a pronounceable word.

Your two-letter combinations are listed almost alphabetically, which makes the list hard to maintain. For example, "so" is out of place, "os" and "xi" are listed twice, and eight elements are haphazardly added at the end. The three-letter list doesn't appear to be in any recognizable order.

When you reject a candidate word, you use recursion to retry. In JavaScript, you should avoid using recursion when a simple while loop will do — especially when there is no bound to the depth of the recursion. The code should be reorganized along these lines:

function generate_unpronounceable(length) {
 var LETTERS = "abcdefghijklmnopqrstuvwxyz";
 do {
 var word = "";
 for (var i = 0; i < length; i++) {
 word += LETTERS[Math.floor(Math.random() * LETTERS.length)];
 }
 } while (is_pronounceable(word));
 return word;
}
function is_prounounceable(word) {
 ...
}

Decomposing the problem into generate_unpronounceable() and is_pronounceable() is also better because is_pronounceable() can be unit-tested more easily.

Source Link
200_success
  • 145.5k
  • 22
  • 190
  • 479

To start with, unpronouncable should be spelled unpronounceable.

I'm not convinced that this is a valid strategy. If it is a valid approach, then your lists of pronounceable syllables is incomplete. For example, unpronouncable(4) is capable of returning "test", which I consider to be a pronounceable word.

Your two-letter combinations are listed almost alphabetically, which makes the list hard to maintain. For example, "so" is out of place, "os" and "xi" are listed twice, and eight elements are haphazardly added at the end. The three-letter list doesn't appear to be in any recognizable order.

When you reject a candidate word, you use recursion to retry. In JavaScript, you should avoid using recursion when a simple while loop will do — especially when there is no bound to the depth of the recursion.

default

AltStyle によって変換されたページ (->オリジナル) /