Breaking up methods leading to more operations

Question

Breaking up methods/functions to do one thing only is considered a good practice as it leads to more maintainable and readable code but it increase number of operations in many cases . How to achieve both ie. optimum number of operations while still keeping readability/maintainability ?

As illustrated in this dummy example . I want to find count of Js in a string but also want to check if addition of all positive integers is even or odd . Two separate methods for these 2 different tasks leads to excellent readability but increases number of operations by n where n is size of input string . Combining them in a method leads to very low readability but decreases number of operations.

class UserText{ 
    constructor(userProvidedText){
     this.content = userProvidedText;
    }
    _findCharCount(char){
      let count = 0;
      for (let i = 0; i < this.content.length; i++) {
        if (this.content[i] === char) {
          count++;
        }
    }   
    return count
    }

    _isDigit(char) {
        return !isNaN(parseInt(char));
      }

    // Excellent Readability and maintainability 
    findLetterCount(char){
        return this._findCharCount(char)
        }
    
    // Excellent Readability and maintainability 
    CheckIfDigitsAdditionEven(){ 
        let count = 0;
        for (let i = 0; i < this.content.length; i++) {
          console.log(this.content[i])
          if (!this._isDigit(this.content[i])){ 
            continue
          }
          count+=parseInt(this.content[i]); 
        }   
        if (count % 2 === 0){ 
            return true
        } 
        return false
    }

    // Combining functionalities in 1 function . Not readable. Not maintainable But time complexity is decreased. 
    CheckIfDigitsAdditionEvenAndCountLetter(char){ 
        let count1 = 0;
        let count2 = 0;

        let res1 = false;

        for (let i = 0; i < this.content.length; i++) {
          if (this.content[i] === char) {
            count2++;
          }
          if (this._isDigit(this.content[i])){ 
            count1+=parseInt(this.content[i]); 
          }
        }   
        if (count1 % 2 === 0){ 
            res1 = true
        } 
        return [res1, count2]

    }
} 

ut = new UserText("SOME..STRING..12311.HHIIJJJJKKKLL ")

/*
Breaking up functions and using them one at a time . One method , one purpose.. 
Maintainable and readable but iterates by 2n.  
*/

JCount=ut.findLetterCount("J") 
evenCheck=ut.CheckIfDigitsAdditionEven()


/*
Using 1 function for the task  . One method , multi-purposed.. 
Maintainable and readable is very low but  iterates by "n" only .  
*/

cr=ut.CheckIfDigitsAdditionEvenAndCountLetter("J") 

// Some custom logic here . 
console.log(JCount, evenCheck)
console.log(cr[0], cr[1])

Answer 1

First of all, thinking about performance issues while designing the model is good, but Premature Optimization should be avoided.

Second, the OP's code in my opinion is neither clean/lean nor maintainable, where the OP claims it to be. And the implementation with all the pseudo-privately annotated prototype methods already starts as a big mess.

Third, the OP's CheckIfDigitsAdditionEvenAndCountLetter method intermingles two tasks which anyhow have to be implemented each separately ... e.g. as hasEvenTotalDigitCount and getTotalLetterCountFor.

Thus said, the OP should write a lot of helper/utility functions which each do one thing a time. These helpers then get re/used by the very few and only really necessary methods, the OP wants the model to feature.

A much better readable, maintainable version of what the OP did provide could look like the following example code ...

// e.g. ... `UserText` module with ...
//
//  - locally scoped helper functions
//  - and a single class export.
// 
//  - each helper function might/should
//    get exported as well for testing.

function isEven(value) {
  return (value % 2 === 0);
}

function isDigit(value) {
  return (/^\p{N}$/u).test(String(value));
}
function isLetter(value) {
  return (/^\p{L}$/u).test(String(value));
}

function getTotalDigitCount(text) {
  return text.match(/\p{N}/gu).length;
}
function getTotalLetterCount(text) {
  return text.match(/\p{L}/gu).length;
}

function getTotalMatchCount(text, query) {
  return text.match(
    RegExp(
      query.replace(/[/\-\\^$*+?.()|[\]{}]/g, '\\$&'), 'g'
    )
  )?.length ?? 0;
}

/*export */class UserText{
  #text;

  constructor(value) {
    // - always ensure a protected string value.
    this.#text = String(value);
  }
  get value() {
    // - helps ensuring the immutability of
    //   the initially passed user text value.
    return this.#text;
  }

  getTotalDigitCountFor(value) {
    value = String(value);

    return isDigit(value) ? getTotalMatchCount(this.#text, value) : 0;
  }
  getTotalLetterCountFor(value) {
    value = String(value);

    return isLetter(value) ? getTotalMatchCount(this.#text, value) : 0;
  }

  hasEvenTotalDigitCount() {
    const totalCount = getTotalDigitCount(this.#text);

    return (totalCount !== 0) && isEven(totalCount);
  }
  hasEvenTotalLetterCount() {
    const totalCount = getTotalLetterCount(this.#text);

    return (totalCount !== 0) && isEven(totalCount);
  }
}
// end of `UserText` module.



// e.g.

/* import { UserText } from './UserText.js'; */
const userText = new UserText("SOME..STRING..12311.HHIIJJJJKKKLL ");

const totalCountOfUpper_J = userText.getTotalLetterCountFor('J');
const hasEvenTotalDigitCount = userText.hasEvenTotalDigitCount();

console.log({
  totalCountOfUpper_J,
  hasEvenTotalDigitCount,
});

// attempt of manipulating the text value.
userText.value = 'foo, bar, baz';

console.log({
  userText,
  'userText.value': userText.value,
});

.as-console-wrapper { min-height: 100%!important; top: 0; }

Edit according to the OP's comment ...

1. "Premature Optimization should be avoided." Could you please expand ? What if I have a str which consumes 500MB in RAM . Now If I process it twice , my performance is going to take a big hit . Same issue If I have a 1MB string but the 2 operations are heavy . I will have to plan beforehand . 2. If I want to run hasEvenTotalDigitCount() and getTotalLetterCountFor() . We will be running 2 regex meaning 2 iterations of the same string instead of 1 , which is what my problem was in the first place . 3. I wanted check if addition of all digits was even not digit count

If there is a 500MB String in RAM, it depends on the target platform. A browser might not be the best environment for running an application which has such requirements whereas a highly performant node server environment might not even getting close to being in trouble.

In addition, taking into account the scenario described by the OP, the question arises whether a class based design like the one introduced with the original question does really suite the OP's needs.

But what could be done is the introduction of lazy computation and memoization techniques, in order do the heavy lifting only when it's needed and never twice for one and the same time consuming operation.

And of cause nothing prevents the OP from implementing a function which does all the necessary computation within a single full string iteration when there is no other option. But then again, the implementation of additional functionality which each computes just a fraction of it is entirely obsolete.

// e.g. ... `UserText` module with ...
//
//  - locally scoped helper functions
//  - and a single class export.
// 
//  - each helper function might/should
//    get exported as well for testing.

function isEven(value) {
  return (value % 2 === 0);
}

function isDigit(value) {
  return (/^\p{N}$/u).test(String(value));
}
function isLetter(value) {
  return (/^\p{L}$/u).test(String(value));
}

function sumUp(summation, value) {
  return summation + Number(value);
}

function getDigits(text) {
  return text.match(/\p{N}/gu);
}
function getLetters(text) {
  return text.match(/\p{L}/gu);
}

function computeMatchCount(text, query) {
  return text.match(
    RegExp(
      query.replace(/[/\-\\^$*+?.()|[\]{}]/g, '\\$&'), 'g'
    )
  )?.length ?? 0;
}
function getMatchCount(text, query, matchCountLookup) {
  // - lookup the `query` specifc, possibly already memoized match-count.
  matchCount = matchCountLookup.get(query) ?? null;

  if (matchCount === null) {
    // - compute and memoize the `query` specifc match-count.
    matchCount = computeMatchCount(text, query);

    matchCountLookup.set(query, matchCount);
  }
  return matchCount;
}

/*export */class UserText{
  #text;

  #digitSummation;
  #matchCountLookup = new Map;

  constructor(value) {
    // - always ensure a protected string value.
    this.#text = String(value);
  }
  get value() {
    // - helps ensuring the immutability of
    //   the initially passed user text value.
    return this.#text;
  }

  get digitSummation() {
    // - lazy computation and memoization.
    return (this.#digitSummation ??= getDigits(this.#text).reduce(sumUp, 0));
  }
  get isDigitSummationEven() {
    // - either uses the memoized `digitSummation` value or triggers its computation.
    return isEven(this.digitSummation);
  }

  getDigitCountFor(value) {
    value = String(value);

    return isDigit(value)
      ? getMatchCount(this.#text, value, this.#matchCountLookup)
      : 0;
  }
  getLetterCountFor(value) {
    value = String(value);

    return isLetter(value)
      ? getMatchCount(this.#text, value, this.#matchCountLookup)
      : 0;
  }
}
// end of `UserText` module.



// e.g.

/* import { UserText } from './UserText.js'; */

// - an around 10MB messuring test string.
const hugeTestString = Array
  .from(
    { length: 300_000 }, crypto.randomUUID.bind(crypto)
  )
  .join(' ');

console.log({
  'hugeTestString.length': hugeTestString.length,
});
const userText = new UserText(hugeTestString + ' SOME..STRING..12311.HHIIJJJJKKKLL ');

// attempt of manipulating the text value.
userText.value = 'foo, bar, baz';

console.log({
  userText,
  // 'userText.value': userText.value,
});

// - any of the next same operations will
//   each return an already memoized value.
const lower_a_count = userText.getLetterCountFor('a');
const lower_f_count = userText.getLetterCountFor('f');

const upper_J_count = userText.getLetterCountFor('J');
const upper_S_count = userText.getLetterCountFor('S');

const digit_7_count = userText.getDigitCountFor('7');
const digit_0_count = userText.getDigitCountFor('0');

console.log({
  lower_a_count,
  lower_f_count,

  upper_J_count,
  upper_S_count,

                 // - returns an already memoized value.
  digit_7_count: userText.getDigitCountFor('7'),

                 // - returns an already memoized value.
  digit_0_count: userText.getDigitCountFor('0'),

  digitSummation: userText.digitSummation,
  isDigitSummationEven: userText.isDigitSummationEven,
});

.as-console-wrapper { min-height: 100%!important; top: 0; }