2

I am a beginner in coding and I can not figure out how to deal with a 'code/function structuring issue'. So when you write a function and the function is starting to have more subordinate functions... I mean it starts to be a multilevel function, I don't know how I should structure my code that it remains to be clean and readable.

Here is an example code, which is part of a tic-tac-toe game

function gameOver(gameWonObj) {

  if (gameWonObj === 'tie') {
    higlightAllFields();
    disableClickOnFields();
    declaireWinner('tie');
  } else {
    highlightWinningHits();
    disableClickOnFields();
    declaireWinner(gameWonObj.player);
  }

  function higlightAllFields() {
    allSquaresIds = ORIG_BOARD;

    for ([idOfSquare, currValue] of allSquaresIds.entries()) {
      currSquare = document.getElementById(idOfSquare);
      currSquare.style.backgroundColor = TIE_COLOR;
    }
  }

  function highlightWinningHits() {
    winningSquaresIds = WIN_COMBOS[gameWonObj.index];
    highlightColor = (gameWonObj.player === HU_PLAYERS_SIGN) ? WINNER_COLOR : LOOSER_COLOR;

    winningSquaresIds.forEach(currentId => {
      currentWinningSquare = document.getElementById(currentId);
      currentWinningSquare.style.backgroundColor = highlightColor;
    });
  }

  function disableClickOnFields() {
    CELLS.forEach(cell => {
      cell.removeEventListener('click', turnClick, false)
    })
  }

  function declaireWinner(player) {
    if (player === 'tie') {
      declaireWinnerOnModal('ITS A TIE GAME', 'blue')
    } else if (player === HU_PLAYERS_SIGN) {
      declaireWinnerOnModal('YOU WON', 'lightgreen')
    } else if (player === AI_PLAYERS_SIGN) {
      declaireWinnerOnModal('AI WON', 'red')
    }

    function declaireWinnerOnModal(message, textColor) {
      END_GAME_MODAL.style.display = 'block';
      END_GAME_MODAL.style.color = textColor;
      END_GAME_MODAL.innerHTML = `<p>${message}</p>`;
    }
  }

}

In this example, I have a main function: gameOver, and it goes a level deeper in functions: declaireWinner, disableClickOnFields, higlightAllFields, declaireWinnerOnModal.

So when you have lets say an additional layer of functions in one of the sub-functions in your main function the code really gets to be unreadable, too long and overwhelming.

When I started to write into my main app.js file, I was thinking about what should be the main constroller. Then I wouldn't go one level deeper and I would import all the necessary functions needed by my first level functions. Here, I would import all of the functions which are needed to the function gameOver.

But then I should pass into gameOver all of the global and every other variable which I have declared lexically above gameOver and then the function definitions and call would be really long and ugly: gameOver(global1,global2,global3,global4,...)

And the functions I have imported in, wouldn't have access to the variable object of the parent function and so I should pass again as parameters all variables which the second level of function - and there subordinated functions - need.

Ivan
  • 34,531
  • 8
  • 55
  • 100
Deli Sandor
  • 151
  • 1
  • 10
  • Since the code provided works and the original poster wants more guidance on how to structure his project, I believe this question belongs on another site in the Stack Exchange network: [Code Review](https://codereview.stackexchange.com/). This question will probably get closed here because it is either too primarily opinion-based or is too broad. I recommend you delete it here and post the same one on Code Review. – Ivan Jun 10 '18 at 12:01
  • thanks Ivan I do it – Deli Sandor Jun 10 '18 at 13:58

1 Answers1

0

But then I should pass into 'gameOver' all of the global and every other variables which I declaired lexically above gameOver and then the functions definition and call would be a really long and ugly: (gameOver(global1,global2,global3,global4,....))

You could introduce a game "state" that contains all that globals:

const state = { global1, global2, global3, global4 };

And then you only have to pass that state to the function:

gameOver(state);

The function could also destructure the object if it only needs one or two of the globals:

function gameOver({global1, global2 }) {
  console.log(global1);
}

So when you have lets say an additional layer of functions in one of the sub-functions in your main function the code really gets to be unreadable, too long and overwhelming.

You should not nest functions deeper than one level. Some of these functions are probably helpers, that only access one or two variables of thr outer function, so you can just pass them in as arguments. So for example you could turn this:

function disableClickOnFields() {
   CELLS.forEach(cell => { // Global?!
      cell.removeEventListener('click', turnClick, false) // another global?!
   })
}

Into:

/* Removes the listener on the specific event from all elements of thr collection */
function removeEventListener(collection, evt, listener) {
  for(const el of collection) {
    el.removeEventListener(evt, listener, false);
  }
}

Which then can be reused at multiple locations and the state can be passed in easily:

removeEventListener(state.cells, "click", handleClick);
Jonas Wilms
  • 132,000
  • 20
  • 149
  • 151