Turn a list of words into a dictionary











up vote
1
down vote

favorite












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
}
}, {});
}









share|improve this question
























  • Which language is it? Can you please retag your question?
    – Calak
    yesterday










  • @Calak javascript
    – TMB
    20 hours ago










  • 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.
    – Elegie
    17 hours ago















up vote
1
down vote

favorite












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
}
}, {});
}









share|improve this question
























  • Which language is it? Can you please retag your question?
    – Calak
    yesterday










  • @Calak javascript
    – TMB
    20 hours ago










  • 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.
    – Elegie
    17 hours ago













up vote
1
down vote

favorite









up vote
1
down vote

favorite











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
}
}, {});
}









share|improve this question















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
}
}, {});
}






javascript ecmascript-6 hash-map






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited 20 hours ago

























asked yesterday









TMB

1876




1876












  • Which language is it? Can you please retag your question?
    – Calak
    yesterday










  • @Calak javascript
    – TMB
    20 hours ago










  • 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.
    – Elegie
    17 hours ago


















  • Which language is it? Can you please retag your question?
    – Calak
    yesterday










  • @Calak javascript
    – TMB
    20 hours ago










  • 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.
    – Elegie
    17 hours ago
















Which language is it? Can you please retag your question?
– Calak
yesterday




Which language is it? Can you please retag your question?
– Calak
yesterday












@Calak javascript
– TMB
20 hours ago




@Calak javascript
– TMB
20 hours ago












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.
– Elegie
17 hours ago




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.
– Elegie
17 hours ago










1 Answer
1






active

oldest

votes

















up vote
0
down vote













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;
}





share|improve this answer























    Your Answer





    StackExchange.ifUsing("editor", function () {
    return StackExchange.using("mathjaxEditing", function () {
    StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
    StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
    });
    });
    }, "mathjax-editing");

    StackExchange.ifUsing("editor", function () {
    StackExchange.using("externalEditor", function () {
    StackExchange.using("snippets", function () {
    StackExchange.snippets.init();
    });
    });
    }, "code-snippets");

    StackExchange.ready(function() {
    var channelOptions = {
    tags: "".split(" "),
    id: "196"
    };
    initTagRenderer("".split(" "), "".split(" "), channelOptions);

    StackExchange.using("externalEditor", function() {
    // Have to fire editor after snippets, if snippets enabled
    if (StackExchange.settings.snippets.snippetsEnabled) {
    StackExchange.using("snippets", function() {
    createEditor();
    });
    }
    else {
    createEditor();
    }
    });

    function createEditor() {
    StackExchange.prepareEditor({
    heartbeatType: 'answer',
    convertImagesToLinks: false,
    noModals: true,
    showLowRepImageUploadWarning: true,
    reputationToPostImages: null,
    bindNavPrevention: true,
    postfix: "",
    imageUploader: {
    brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
    contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
    allowUrls: true
    },
    onDemand: true,
    discardSelector: ".discard-answer"
    ,immediatelyShowMarkdownHelp:true
    });


    }
    });














     

    draft saved


    draft discarded


















    StackExchange.ready(
    function () {
    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f207609%2fturn-a-list-of-words-into-a-dictionary%23new-answer', 'question_page');
    }
    );

    Post as a guest
































    1 Answer
    1






    active

    oldest

    votes








    1 Answer
    1






    active

    oldest

    votes









    active

    oldest

    votes






    active

    oldest

    votes








    up vote
    0
    down vote













    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;
    }





    share|improve this answer



























      up vote
      0
      down vote













      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;
      }





      share|improve this answer

























        up vote
        0
        down vote










        up vote
        0
        down vote









        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;
        }





        share|improve this answer














        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;
        }






        share|improve this answer














        share|improve this answer



        share|improve this answer








        edited 9 hours ago

























        answered 9 hours ago









        Blindman67

        6,3691521




        6,3691521






























             

            draft saved


            draft discarded



















































             


            draft saved


            draft discarded














            StackExchange.ready(
            function () {
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f207609%2fturn-a-list-of-words-into-a-dictionary%23new-answer', 'question_page');
            }
            );

            Post as a guest




















































































            Popular posts from this blog

            Quarter-circle Tiles

            build a pushdown automaton that recognizes the reverse language of a given pushdown automaton?

            Mont Emei