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
}
}, {});
}
javascript ecmascript-6 hash-map
add a comment |
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
}
}, {});
}
javascript ecmascript-6 hash-map
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
add a comment |
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
}
}, {});
}
javascript ecmascript-6 hash-map
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
javascript ecmascript-6 hash-map
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
add a comment |
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
add a comment |
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;
}
add a comment |
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;
}
add a comment |
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;
}
add a comment |
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;
}
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;
}
edited 9 hours ago
answered 9 hours ago
Blindman67
6,3691521
6,3691521
add a comment |
add a comment |
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
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
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
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