Return the first word with the greatest number of repeated letters

Question

I'm learning JavaScript and have written a function that accepts a string and returns the first word with the greatest number of repeated letters. For example:

Input:"Hello apple pie" - Output:"Hello"
Input:"Hello apple pie yelllow" - Output:"yelllow"

I've tried to keep it as concise as possible. I wonder whether later on in the code it starts to become more verbose. The answer it returns seems to be correct for all tests.

Could someone give some advice about improving my code? Or is it not bad?

function whichWord(str) {

// 1. split into array with words - EG if var str = "wodrd sy hello";
var arr = str.split(' '); // puts str into array ["wodrd", "sy", "hello"]
var mdArr = arr.map(function(el){ return el.split('') }); // puts str into MD array [["w","o","d","r","d"],["s","y"],["h","e","l","l","o"]]

// 2. get each unique letter (remove duplicates)
function removeDuplicates(a) {
 return Array.from(new Set(a)) 
}
var letters = mdArr.map(removeDuplicates);

// 3. count # of each letter occurrences
var temp = [];
var numbers = [];

for(i=0;i < mdArr.length;i++) {

for(k=0;k < letters[i].length;k++) {temp.push(mdArr[i].filter(function(c2){return c2==letters[i][k]}).length)}

numbers.push(temp);
temp=[];

}

var letterCount = letters.map((innerArr, i) => innerArr.map((letter, ii) => ({letter: letter, instances: numbers[i][ii]}))); // puts into MD array with objects:
/* returns: [
    [{letter: "w", instances: 1},{letter: "o", instances: 1},{letter: "d", instances: 2},{letter: "r", instances: 1}],
    [{letter: "s", instances: 1},{letter: "y", instances: 1}],
    [{letter: "h", instances: 1},{letter: "e", instances: 1},{letter: "l", instances: 2},{letter: "o", instances: 1}]
] */

// 4. return word from index with largest number, & first instance if matching numbers
var highestNumbers = letterCount.map((outerArr,i) => Math.max.apply(Math,outerArr.map((innerArr,ii) => innerArr.instances)));

var highestNumber = Math.max.apply(Math,highestNumbers);

var indexToReturn = highestNumbers.findIndex(function(c) { return c == highestNumber } );

return arr[indexToReturn];

}

Answer 1

This approach seems a bit overly-complicated - there are multiple loops over the word just to remove duplicates, count the letters, etc. I would suggest a simple loop over each word to count the number of repeated numbers, maintaining the max count so far, and then using a simple object mapping (or a Set could be used) to look up the max counts for each word. See the rewritten code below for an example of this.

Additionally, the indentation could be greatly improved. Perhaps the first level inside whichWord in your code was lost because of pasting it into the question. However, most JavaScript code is indented by 2 or 4 spaces inside each block.

I noticed that arrow functions are being used (e.g. letters.map((innerArr, i) => innerArr.map((letter, ii).... This is a feature of ecmascript-6 and thus other features from that standard could be used, such as const for any variable that is never re-assigned and let for any variable that needs to be reassigned (e.g. iterator variables, cumulative counts, etc.), as well as the spread syntax instead of using function.apply().

Rewritten code

Below is my first take at simplifying the code. It can be compared for performance in this jsPerf.

function maxMode(word) {
  if (word.length == 1) {
    return 0;
  }
  const counts = {};
  for (const letter of word) {
    counts[letter] = (counts[letter] ?? 0) + 1;
  }
  return Math.max(...Object.values(counts));
}
const whichWord = str => {
  const arr = str.split(' ');
  const mappings = arr.reduce((mapping, word) => {
    mapping[word] = maxMode(word);
    return mapping;
  }, {});
  const mappingCounts = Object.values(mappings);
  const highestNumber = Math.max(...mappingCounts);
  const indexToReturn = mappingCounts.findIndex(c => c == highestNumber);
  return Object.keys(mappings)[indexToReturn];
};
const strings = ['Hello a apple pie yellow', 'axaya zza'];
for (const string of strings) {
  console.log('whichWord('+string+'):', whichWord(string));
}

¹_{https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String#Character_access} ²_{https://stackoverflow.com/a/4051431/1575353}

Answer 2

I decided to take a crack at simplifying this problem myself as it sounded like a fun challenge.

@SᴀᴍOnᴇᴌᴀ already offered a bunch of good advice. The main thing I want to add is how important it is to be able to split up the logic into well-named and easily understandable helper functions. You did have one small helper function in there, but one or two more would have helped to divide up the logic further.

Here's my shot at implementing it:

function firstWithHighestScore({ array, scoreFn }) {
  const scores = array.map(scoreFn);
  const highScore = Math.max(...scores);
  return array[scores.indexOf(highScore)];
}

function countLetters(text) {
  const count = {};
  for (const char of text) {
    count[char] = (count[char] ?? 0) + 1;
  }
  return count;
}

const whichWord = text => firstWithHighestScore({
  array: text.split(' '),
  scoreFn: word => Math.max(...Object.values(countLetters(word)))
});

console.log(whichWord('Hello apple    pie'));
console.log(whichWord('Hello apple pie yelllow'));
console.log(whichWord('xwxyx zz'));

You'll notice that the main function, whichWord, is higher level. It's stating that we need to find the "firstWithHighestScore" from a list word words (text.split(' ')), scored by a function that counts letters, and finds the most of a single occurrence.

The helper functions help fill in the details.