1
\$\begingroup\$

I am looking for a way to turn a list of words that may have duplicates into a dictionary/map that counts the number of occurrences of words. After spending some time with the problem, this seems to be one of the better ways, but maybe there are some downsides I am not seeing to this.

const magazine = "asdf ASDF wer wer";

This should produce a magazineMap of

const magazineMap = {
  asdf: 1,
  ASDF: 1,
  wer: 2
}

My solution to this was

function mapMagazine (magazine) {
    return magazine
        .split(' ')
        .reduce((initMap, word) => {
            return {
                ...initMap,
                [word]: (initMap[word] || 0) + 1
            }
        }, {});
}
\$\endgroup\$
  • \$\begingroup\$ Which language is it? Can you please retag your question? \$\endgroup\$ – Calak Nov 14 '18 at 8:03
  • \$\begingroup\$ @Calak javascript \$\endgroup\$ – TMB Nov 14 '18 at 19:18
  • \$\begingroup\$ Hello. What are your criteria for good code? Readable, fast...? Does your reduction create and fill a new dictionary (the returned value) at each iteration? I'm not familiar with modern Ecmascript. Cheers. \$\endgroup\$ – Elegie Nov 14 '18 at 22:11
1
\$\begingroup\$

Efficiency and potential problems

You could use a Map but as you are counting word occurrences using an object is a little more efficient.

Efficiency

Your code can be improved as it is highly inefficient with each word needing to create a new object and fill its properties from the previous one, then add or update then next word. You need only create one object and add properties to it as you find them.

function mapMagazine (text) {
    return text.split(' ')
        .reduce((map, word) => (
            map[word] = map[word] ? map[word] + 1 : 1, map            
        ), {});
}

or using forEach,

function mapMagazine (text) {
    const map = {};
    text.split(' ').forEach(w => map[w] = map[w] ? map[w] + 1 : 1);
    return map;
}

or the (slightly) more performant version using a for loop

function mapMagazine (text) {
    const map = {};
    for (const w of text.split(' ')) { map[w] = map[w] ? map[w] + 1 : 1 }
    return map;
}

Problems

I do however see that your code would suffer from problems with a sting such as "A is a, as an a at the beginning, a capitalised A." (Note it has a double space in it) It would return with some of the following properties...

{
    A : 1,
    "a," : 1,
    a : 2,
    "A." : 1,
    "beginning," : 1,
    "" : 1,  // empty string from splitting two spaces
     .. and the rest
 }

I would imagine you would want something a little more like.

{
    a : 5,
    beginning : 1,
     .. and the rest
 }

To do this you need a slight mod to the function. The text should be converted to lowercase, split can use a RegExp to split into words on white spaces or groups of white spaces, and a check for empty string that can result if the text is ended on a white space such as a full stop.

BTW does the function really map magazines?

function mapWords(text) {
    const map = {};
    for (const word of text.toLowerCase().split(/\W+/)) { 
        word !== "" && (map[word] = map[word] ? map[word] + 1 : 1);
    }
    return map;
}
|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Case is sensitive, characters are only alphabetic. Your catch about creating a new object every iteration was def good. Thank you for all the comments. Question about the first code block. map[word] = map[word] ? map[word] + 1 : 1, map What does the , map do? \$\endgroup\$ – TMB Dec 4 '18 at 23:49
  • \$\begingroup\$ I think I see, because of the lack of curly braces, the comma makes map the returned value of the expression. \$\endgroup\$ – TMB Dec 5 '18 at 0:09
  • 1
    \$\begingroup\$ @TMB Yes that is correct. You can combine many expressions in to one (compound expression) using commas, with the last item becoming the the result of the compound expression. I find them more readable. . Note that this is unique to JavaScript, most languages with C like syntax allow commas, but the order of execution and the resulting term is indeterminate. \$\endgroup\$ – Blindman67 Dec 5 '18 at 1:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.