基于字母频率数组生成随机抽取字母列表的算法失效问题排查

Let's break down the issues in your code one by one and fix them to meet your requirements:

Issues in Your Code #

• Duplicate variable declaration: You’ve declared const letters twice in the same scope. This will throw a syntax error since const variables can’t be redeclared.
• Incorrect array iteration: Using for...in on an array loops over index keys (like "0", "1") instead of the actual elements. So letter here isn’t the object you expect, and letter.frequency will return undefined.
• Off-by-one repetition error: The condition i <= letter.frequency * 100 adds one extra occurrence of the letter. For example, if frequency * 100 equals 50, this loop runs 51 times (from 0 to 50). Use i < letter.frequency * 100 to get the exact number of repetitions.
• slice() doesn’t modify the original array: The slice() method returns a new sliced array, but you aren’t assigning this result back to freqLetters. This means you’re still returning the full array instead of the first n elements.
• Incorrect empty rows check: rows from db.all() will never be undefined on success—it’ll be an empty array if no records exist. Checking rows.length === 0 is more accurate.
• Pushing the wrong value: Your code pushes the entire letter object into freqLetters, but based on your requirement, you should push the letter string (e.g., "A") instead of the object.

Fixed Code #

exports.listLetters = (n) => { 
  return new Promise((resolve, reject) => { 
    const sql = 'SELECT * FROM letters'; 
    db.all(sql, [], (err, rows) => { 
      if (err) { 
        reject(err); 
        return; 
      } 
      if (rows.length === 0) { 
        resolve({ error: "No letters retrieved." }) 
      } else { 
        // Only declare the letters array once
        const letters = rows.map((e) => ({ letter: e.letter, frequency: e.frequency })); 
        let freqLetters = []; 
        // Use for...of to iterate over array elements directly
        for (const letterObj of letters) { 
          const repeatCount = Math.floor(letterObj.frequency * 100); // Ensure integer count
          // Loop the exact number of times needed
          for (let i = 0; i < repeatCount; i++) { 
            // Push the actual letter string, not the object
            freqLetters.push(letterObj.letter); 
          } 
        }; 
        shuffleArray(freqLetters); 
        // Assign the sliced result back to keep only the first n elements
        freqLetters = freqLetters.slice(0, n); 
        resolve(freqLetters); 
      }
    });
  });
}

// Your shuffle function works correctly (modifies array in place)
function shuffleArray(array) { 
  for (let i = array.length - 1; i > 0; i--) { 
    const j = Math.floor(Math.random() * (i + 1)); 
    [array[i], array[j]] = [array[j], array[i]]; 
  }
}

Additional Notes #

• Added Math.floor(letterObj.frequency * 100) to ensure we’re using an integer count (critical if frequency is a float like 0.35, which would translate to 35 repetitions).
• The shuffle function is correct because it modifies the array in place—no need to assign its return value.
• Updated the empty rows message to be more precise about what’s happening.

内容的提问来源于stack exchange，提问作者Paganini