2
\$\begingroup\$

I wrote three versions of the same code using Reduce, a functional approach and also a regex pattern approach.

I'd like to get feedback, especially from the functional one since I'm learning functional programming and I'm sure my solution can be greatly improved.

const CHAR = 'X'
const BANNED = ['nice', 'is', 'stupid', 'potato']
const sentence = 'lorem ipsum is a nice and stupid sentence'


// VERSION 1 - REDUCE
const censor = (sentence) =>
  sentence.split(' ').reduce((acc, word) =>
    acc + ' ' + (BANNED.includes(word) ? CHAR.repeat(word.length) : word), '')

const censored = censor(sentence).trim()

console.log(censored)



// VERSION 2 - PIPE (functional?)
const pipe = (...fns) => fns.reduce((f, g) => (...args) => g(f(...args)))

const extractWords = (sentence) => sentence.split(' ')

const removeBanned = (arr) =>
  arr.reduce((acc, word) => acc + ' ' + replaceIfBanned(word), '')

const cleanUp = (arr) => arr.trim()

const replaceIfBanned = word =>
  BANNED.includes(word) ? replaceWithChar(word) : word

const replaceWithChar = (word) => CHAR.repeat(word.length)

const work = pipe(extractWords, removeBanned, cleanUp)(sentence)

console.log(work)



// VERSION 3 - REGEX
const styleHyphenFormat = (sentence) => {
  const replaceWithX = (match) => CHAR.repeat(match.length)
  const filter = new RegExp(BANNED.join('|'), 'gi')

  return sentence.replace(filter, replaceWithX)
}

console.log(styleHyphenFormat(sentence))

You can also check it out in JSFiddle.

\$\endgroup\$
2
\$\begingroup\$

The problem with the first two is that you are assuming separators are spaces but they could potentially be tabs, new lines or punctuation. Although it is easy to split on a non-word character joining them back in is tricky (even multiple spaces will fool the algorithm). Your regex solution has the added problem that it will convert words like kiss to kXXs.

I would stick to the regex, it is shorter and, imo, more readable. You can fix the issue by using a word boundary regex match:

const filter = new RegExp(`\\b(${BANNED.join('|')})\\b`, 'gi')

That said, I would also measure the performance of the three solutions as that is likely to be the biggest concern, especially with longer strings.

Edit 1

An alternative solution using split would be something like:

const censor = (sentence) =>
  sentence.split(/\b/).map(
    (word) => BANNED.includes(word.toLowerCase()) ? CHAR.repeat(word.length) : word
  ).join('')
\$\endgroup\$
2
  • \$\begingroup\$ I wouldn't worry about performance because the regular expression is the only solution that will actually work correctly :) \$\endgroup\$ – Sulthan Aug 6 '17 at 7:58
  • \$\begingroup\$ thank you for your input! Didn't think about word boundaries...I should've, thanks. \$\endgroup\$ – Adrià Fontcuberta Aug 6 '17 at 9:11
0
\$\begingroup\$

reduce looks like a very poor choice.

If you want to use your first solution, you should go for split(...).map(...).join(...).

const censored = sentence
   .split(' ')
   .map(word => BANNED.includes(word) ? CHAR.repeat(word.length) : word)
   .join(' ')

That will also remove the need for trim().

Of course, considering space character as the only word delimiter will become very problematic anyway. Also note that BANNED.includes will be a performance bottleneck once BANNED.length increase to higher numbers. Using Set or a simple javascript object with the banned words as keys would solve that.

\$\endgroup\$
3
  • \$\begingroup\$ thanks for your focus in performance! It actually make sense to use a Set to store banned words, so I switched that part. Just another question: why'd you say that reduce is a poor choice? Is it due to legibility? I can see that mapping might be cleaner, I'm just wondering if there's anything else to discard reduce. Thanks! \$\endgroup\$ – Adrià Fontcuberta Aug 6 '17 at 15:29
  • \$\begingroup\$ One issue with reduce is that you create a new string each time you add an item. Memory allocation and copying can become quite expensive. Using map/join means you only do that allocation once (hopefully). \$\endgroup\$ – Marc Rohloff Aug 7 '17 at 16:46
  • \$\begingroup\$ @AdriàFontcuberta You are using reduce for only one reason - to concat the strings. However, we already have a function for that and that's join. In (functional) programming you have to think how to split the split the functionality into steps. Some of the steps already have predefined functions. \$\endgroup\$ – Sulthan Aug 7 '17 at 19:17

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.