Gfrktyl

I've written an algo to compress a string

eg. aabcbbba to a2bcb3a

I'm pretty new to functional programming, I feel it's easier to debug and learn from functional code if you're good at it already, but total opposite if you're not.

I'm wondering how I can make this code cleaner and more functional.

I feel like there's gotta be a better way to do compressCharacters without the need of an array or a result variable (perhaps substituting forEach with something else) as well as reducing the lines of code in groupCharacters

const signature = `aabcbbba`;



const compressString = signature => {

  return compressCharacters(groupCharacters(signature));

}



const groupCharacters = signature => {

  let newSignature = "", arr = ;

  

  // convert signature to an array

  let result = [...signature].reduce((accumulator, element, index) => {

    

    // check if last letter in accumulator matches the current element

    if (accumulator[accumulator.length -1] !== element) {

      // add the accumulator string into an array

      arr.push(accumulator);

      // clear the newSignature string and set it to current element

      newSignature = element;

    } else {

      // if it doesn't match, add it to the accumulator

      newSignature = accumulator += element;

    }

    

    // check if it's the last item - add to array

    if (index === signature.length - 1) arr.push(element);

    

    // return newSignature to accumulator

    return newSignature;

  })



  return arr;

}



const compressCharacters = arr => {

  let newArray = ;



  let result = arr.forEach(e => e.length > 1 ? newArray.push(`${e[0]}${e.length}`) : newArray.push(e))



  return newArray.join("");

}



compressString(signature);

Style notes

• Not putting {, } around single line statement blocks is a bad habit. Always delimit statement code blocks.


• In compressCharacters newArray should be a constant. In groupCharacters arr should be a constant.


• Consistent naming is important. Your naming is all over the place. You call a string, characters (in compressCharacters), string (in compressString), signature, and e in the forEach. A character you call element. You abbreviate an array to arr in one function and call it newArray in another. Most of the names are describing the type and not the abstracted data that they hold.


• Don't add useless or redundant code. In compressCharacters you create a variable result that you do nothing with. Not to mention that forEach does not have a return defined. Also result in groupCharacters is never used.


• Don't declare variables outside the scope that they are to be used. newSignature is only used inside reduce but declared outside the callback.

Functional programing means that functions should not have side effect (change the state of anything outside the function's scope.) The reduce callback uses the array arr which breaks the no side effects rule. And the forEach pushes to arr which is also outside the forEach callbacks scope (use map or reduce in that case).

Applying the above you would get something like the following code.

const groupRuns = str => [...str].reduce((groups, char) => {

        const last = groups.length - 1;

        if (last < 0 || groups[last][0] !== char) { groups.push(char) }

        else { groups[last] += char }

        return groups;

    }, );



const concatGroups = groups => groups.reduce((str, g) => 

        str + (g.length > 2 ? g[0] + g.length : g)

    , "");



const compressString = str => concatGroups(groupRuns(str));



const signature = `aabcbbbaaaaaabcccccdddddddbbba`;

compressString(signature);

Note that rather than add numbers to groups of size 2 the group must be 3 or larger.

I am not sure which answer is better. but I would've done it this way:

var arrElement, arr= , tempElement = "", compressedStrng ="", counter = 1;



        function CompressString(strng) { 

        arr = strng.split(''); //splitting the string to an array of chars

                for (arrElement of arr) { //looping in this array

                if (tempElement == ""){ //if first element

                  tempElement = arrElement; 

                  compressedStrng += arrElement;

                }else{

                  if (tempElement == arrElement){ //if array element is repeating add one to the counter below

                    counter ++;

                  } else{

                    if (counter > 1){

                      compressedStrng += counter;

                      counter = 1;

                    }

                    tempElement = arrElement;

                    compressedStrng += arrElement;

                  }

                }

            }

            return compressedStrng;

          }



CompressString('aabcbbba');

console.log (CompressString('aabcbbba')); //It will display a2bcb3a

let me know if you need any clarifications