1

Like in the subject. I have a function like below and I have quite a bit of helping functions declared within a function (twice as much than in the example) because it's the only one using them.

My question is: should I extract those helping functions outside the function to maintain rule "Function should do one job and do it well" or it should be within? I also read about that higher level functions should be higher for better readability, but it somehow doesn't work (shouldn't hoisting make it work?).

const queryThings = async (body = defaultBody) => {
  try {
    (...)

    // helping functions
    const isNonTestDeal = obj => (...)
    const isNonEmpty = obj => (...)
    const replaceHTMLEntity = obj => (...)
    const extractCountries = o => (...)

    const queried = await query(...) // that one is outside this function

    const cases = queriedCases
      .filter(isNonTestDeal)
      .map(obj => {

        let countries = [(...)]
          .filter(isNonEmpty)
          .map(replaceHTMLEntity)
          .map(extractCountries)

        let data = {
          (...)
        }
       return data
      })
      .filter(obj => (...))
      .sort((a,b) => a.d - b.d)
      .slice(0, 45) // node has problem with sending data of more than 8KB

    return cases
  } catch (error) {
    console.log(error)
  }
}
jean d'arme
  • 4,033
  • 6
  • 35
  • 70
  • I don't think it really matters. If they are outside, then you can potentially reuse them. But if you don't need to, then you can just keep them inside. – VLAZ Apr 02 '20 at 17:10
  • That's what I thought. I try to extract just functions that I reuse. I read somewhere about similar things for `require` to not load things that are not used. – jean d'arme Apr 02 '20 at 17:14

2 Answers2

3

If you declare the function outside, and only use it in one function, then you cause namespace pollution. (What is namespace pollution?) Thus, I would recommend keeping it inside. Also, if you do so, it is easier to read as well, since it will be closer to the code where it is used.

To address your question about hoisting, it only works if you declare your function without assigning it to a variable.

collion
  • 31
  • 4
  • 3
    Nice first answer! Yes, this definitely helps answering my question. Found this little gem: `keeping the variables inside a closure ensures they are garbage collected` [link](https://stackoverflow.com/questions/8862665/what-does-it-mean-global-namespace-would-be-polluted#comment18226782_13352212) – jean d'arme Apr 02 '20 at 17:25
  • 1
    Thanks! Happy to be of help. :) – collion Apr 02 '20 at 17:26
0

i think when you write function in other function the memory use is better than write out of function but you can't use in another function it is local function and it isn't public function

MBadrian
  • 409
  • 3
  • 10
  • I think so too. Just want to make sure how to have clean AND performant code at the same time. And yeah, I know overusing `.map()` and `.filter()` is not helping performance, but at least it's clean :) – jean d'arme Apr 02 '20 at 17:16