Simplify this function which finds the coordinates of the neighbors of an isometric cell

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
0
down vote

favorite












I would like to simplify and optimize this function which finds me the neighbors of a cell thanks to cartesian coordinates of a cell Id



const getNeighborsCellId = cellId => 
const neighbors =
const x, y = getCoordinatesFromCellId(cellId)



// LOL ->

const isStaggered = y % 2
if (isStaggered)


neighbors.push(getCellIdFromCoordinates(x, y - 1))
neighbors.push(getCellIdFromCoordinates(x, y - 2))
neighbors.push(getCellIdFromCoordinates(x + 1, y - 1))
neighbors.push(getCellIdFromCoordinates(x - 1, y))
neighbors.push(getCellIdFromCoordinates(x + 1, y))
neighbors.push(getCellIdFromCoordinates(x, y + 1))
neighbors.push(getCellIdFromCoordinates(x, y + 2))
neighbors.push(getCellIdFromCoordinates(x + 1, y + 1))


else
neighbors.push(getCellIdFromCoordinates(x - 1, y - 1))
neighbors.push(getCellIdFromCoordinates(x, y - 2))
neighbors.push(getCellIdFromCoordinates(x, y - 1))
neighbors.push(getCellIdFromCoordinates(x - 1, y))
neighbors.push(getCellIdFromCoordinates(x + 1, y))
neighbors.push(getCellIdFromCoordinates(x - 1, y + 1))
neighbors.push(getCellIdFromCoordinates(x, y + 2))
neighbors.push(getCellIdFromCoordinates(x, y + 1))


return neighbors







share|improve this question



























    up vote
    0
    down vote

    favorite












    I would like to simplify and optimize this function which finds me the neighbors of a cell thanks to cartesian coordinates of a cell Id



    const getNeighborsCellId = cellId => 
    const neighbors =
    const x, y = getCoordinatesFromCellId(cellId)



    // LOL ->

    const isStaggered = y % 2
    if (isStaggered)


    neighbors.push(getCellIdFromCoordinates(x, y - 1))
    neighbors.push(getCellIdFromCoordinates(x, y - 2))
    neighbors.push(getCellIdFromCoordinates(x + 1, y - 1))
    neighbors.push(getCellIdFromCoordinates(x - 1, y))
    neighbors.push(getCellIdFromCoordinates(x + 1, y))
    neighbors.push(getCellIdFromCoordinates(x, y + 1))
    neighbors.push(getCellIdFromCoordinates(x, y + 2))
    neighbors.push(getCellIdFromCoordinates(x + 1, y + 1))


    else
    neighbors.push(getCellIdFromCoordinates(x - 1, y - 1))
    neighbors.push(getCellIdFromCoordinates(x, y - 2))
    neighbors.push(getCellIdFromCoordinates(x, y - 1))
    neighbors.push(getCellIdFromCoordinates(x - 1, y))
    neighbors.push(getCellIdFromCoordinates(x + 1, y))
    neighbors.push(getCellIdFromCoordinates(x - 1, y + 1))
    neighbors.push(getCellIdFromCoordinates(x, y + 2))
    neighbors.push(getCellIdFromCoordinates(x, y + 1))


    return neighbors







    share|improve this question























      up vote
      0
      down vote

      favorite









      up vote
      0
      down vote

      favorite











      I would like to simplify and optimize this function which finds me the neighbors of a cell thanks to cartesian coordinates of a cell Id



      const getNeighborsCellId = cellId => 
      const neighbors =
      const x, y = getCoordinatesFromCellId(cellId)



      // LOL ->

      const isStaggered = y % 2
      if (isStaggered)


      neighbors.push(getCellIdFromCoordinates(x, y - 1))
      neighbors.push(getCellIdFromCoordinates(x, y - 2))
      neighbors.push(getCellIdFromCoordinates(x + 1, y - 1))
      neighbors.push(getCellIdFromCoordinates(x - 1, y))
      neighbors.push(getCellIdFromCoordinates(x + 1, y))
      neighbors.push(getCellIdFromCoordinates(x, y + 1))
      neighbors.push(getCellIdFromCoordinates(x, y + 2))
      neighbors.push(getCellIdFromCoordinates(x + 1, y + 1))


      else
      neighbors.push(getCellIdFromCoordinates(x - 1, y - 1))
      neighbors.push(getCellIdFromCoordinates(x, y - 2))
      neighbors.push(getCellIdFromCoordinates(x, y - 1))
      neighbors.push(getCellIdFromCoordinates(x - 1, y))
      neighbors.push(getCellIdFromCoordinates(x + 1, y))
      neighbors.push(getCellIdFromCoordinates(x - 1, y + 1))
      neighbors.push(getCellIdFromCoordinates(x, y + 2))
      neighbors.push(getCellIdFromCoordinates(x, y + 1))


      return neighbors







      share|improve this question













      I would like to simplify and optimize this function which finds me the neighbors of a cell thanks to cartesian coordinates of a cell Id



      const getNeighborsCellId = cellId => 
      const neighbors =
      const x, y = getCoordinatesFromCellId(cellId)



      // LOL ->

      const isStaggered = y % 2
      if (isStaggered)


      neighbors.push(getCellIdFromCoordinates(x, y - 1))
      neighbors.push(getCellIdFromCoordinates(x, y - 2))
      neighbors.push(getCellIdFromCoordinates(x + 1, y - 1))
      neighbors.push(getCellIdFromCoordinates(x - 1, y))
      neighbors.push(getCellIdFromCoordinates(x + 1, y))
      neighbors.push(getCellIdFromCoordinates(x, y + 1))
      neighbors.push(getCellIdFromCoordinates(x, y + 2))
      neighbors.push(getCellIdFromCoordinates(x + 1, y + 1))


      else
      neighbors.push(getCellIdFromCoordinates(x - 1, y - 1))
      neighbors.push(getCellIdFromCoordinates(x, y - 2))
      neighbors.push(getCellIdFromCoordinates(x, y - 1))
      neighbors.push(getCellIdFromCoordinates(x - 1, y))
      neighbors.push(getCellIdFromCoordinates(x + 1, y))
      neighbors.push(getCellIdFromCoordinates(x - 1, y + 1))
      neighbors.push(getCellIdFromCoordinates(x, y + 2))
      neighbors.push(getCellIdFromCoordinates(x, y + 1))


      return neighbors









      share|improve this question












      share|improve this question




      share|improve this question








      edited Jan 6 at 17:23









      janos

      95.6k12120343




      95.6k12120343









      asked Jan 6 at 15:26









      ken

      325




      325




















          1 Answer
          1






          active

          oldest

          votes

















          up vote
          3
          down vote



          accepted










          First off, I'm not a game developer, and this is a fundamental point, but wouldn't it be easier to use "regular" coordinates (for example: https://i.stack.imgur.com/XpFSQ.png) instead of "staggered" ones (https://i.stack.imgur.com/Mt5Mw.png)?




          Anyway, aside from that, in my opinion there are two major problems about your your function: A) Is doing too much and B) repeating itself.



          The main function of calculating the neighbor coordinates should be separate and take an coords object and return an array of coordinates objects. It should use a list of (either pre-calculated or hardcoded) relative coordinates of the neighbors. If you have, for example, functions to calculete the explicit north, north-east, east, etc. neighbors, then they should be sharing this data.



          const CELL_NEIGHBORS = [
          // Even:
          [dx: 0, dy: -1, dx: 0, dy: -2, dx: +1, dy: -1 /* etc. */ ],
          // Odd:
          [ /* TODO */ ]
          ];

          function getAllNeighborCoordinates(x, y)
          const evenOdd = y % 2;

          return CELL_NEIGHBORS[evenOdd].map(dx, dy => x: x + dx, y: y + dy);



          Now if you change getCellIdFromCoordinates to also take an coordinates object, the final function would be:



           function getNeighborsCellId(cellId) 
          return getAllNeighborCoordinates(getCoordinatesFromCellId(cellId))
          .map(getCellIdFromCoordinates);



          (DISCLAMER: I'm not that into ES2015 yet, so there may be errors in my code.)






          share|improve this answer





















            Your Answer




            StackExchange.ifUsing("editor", function ()
            return StackExchange.using("mathjaxEditing", function ()
            StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
            StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
            );
            );
            , "mathjax-editing");

            StackExchange.ifUsing("editor", function ()
            StackExchange.using("externalEditor", function ()
            StackExchange.using("snippets", function ()
            StackExchange.snippets.init();
            );
            );
            , "code-snippets");

            StackExchange.ready(function()
            var channelOptions =
            tags: "".split(" "),
            id: "196"
            ;
            initTagRenderer("".split(" "), "".split(" "), channelOptions);

            StackExchange.using("externalEditor", function()
            // Have to fire editor after snippets, if snippets enabled
            if (StackExchange.settings.snippets.snippetsEnabled)
            StackExchange.using("snippets", function()
            createEditor();
            );

            else
            createEditor();

            );

            function createEditor()
            StackExchange.prepareEditor(
            heartbeatType: 'answer',
            convertImagesToLinks: false,
            noModals: false,
            showLowRepImageUploadWarning: true,
            reputationToPostImages: null,
            bindNavPrevention: true,
            postfix: "",
            onDemand: true,
            discardSelector: ".discard-answer"
            ,immediatelyShowMarkdownHelp:true
            );



            );








             

            draft saved


            draft discarded


















            StackExchange.ready(
            function ()
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f184450%2fsimplify-this-function-which-finds-the-coordinates-of-the-neighbors-of-an-isomet%23new-answer', 'question_page');

            );

            Post as a guest






























            1 Answer
            1






            active

            oldest

            votes








            1 Answer
            1






            active

            oldest

            votes









            active

            oldest

            votes






            active

            oldest

            votes








            up vote
            3
            down vote



            accepted










            First off, I'm not a game developer, and this is a fundamental point, but wouldn't it be easier to use "regular" coordinates (for example: https://i.stack.imgur.com/XpFSQ.png) instead of "staggered" ones (https://i.stack.imgur.com/Mt5Mw.png)?




            Anyway, aside from that, in my opinion there are two major problems about your your function: A) Is doing too much and B) repeating itself.



            The main function of calculating the neighbor coordinates should be separate and take an coords object and return an array of coordinates objects. It should use a list of (either pre-calculated or hardcoded) relative coordinates of the neighbors. If you have, for example, functions to calculete the explicit north, north-east, east, etc. neighbors, then they should be sharing this data.



            const CELL_NEIGHBORS = [
            // Even:
            [dx: 0, dy: -1, dx: 0, dy: -2, dx: +1, dy: -1 /* etc. */ ],
            // Odd:
            [ /* TODO */ ]
            ];

            function getAllNeighborCoordinates(x, y)
            const evenOdd = y % 2;

            return CELL_NEIGHBORS[evenOdd].map(dx, dy => x: x + dx, y: y + dy);



            Now if you change getCellIdFromCoordinates to also take an coordinates object, the final function would be:



             function getNeighborsCellId(cellId) 
            return getAllNeighborCoordinates(getCoordinatesFromCellId(cellId))
            .map(getCellIdFromCoordinates);



            (DISCLAMER: I'm not that into ES2015 yet, so there may be errors in my code.)






            share|improve this answer

























              up vote
              3
              down vote



              accepted










              First off, I'm not a game developer, and this is a fundamental point, but wouldn't it be easier to use "regular" coordinates (for example: https://i.stack.imgur.com/XpFSQ.png) instead of "staggered" ones (https://i.stack.imgur.com/Mt5Mw.png)?




              Anyway, aside from that, in my opinion there are two major problems about your your function: A) Is doing too much and B) repeating itself.



              The main function of calculating the neighbor coordinates should be separate and take an coords object and return an array of coordinates objects. It should use a list of (either pre-calculated or hardcoded) relative coordinates of the neighbors. If you have, for example, functions to calculete the explicit north, north-east, east, etc. neighbors, then they should be sharing this data.



              const CELL_NEIGHBORS = [
              // Even:
              [dx: 0, dy: -1, dx: 0, dy: -2, dx: +1, dy: -1 /* etc. */ ],
              // Odd:
              [ /* TODO */ ]
              ];

              function getAllNeighborCoordinates(x, y)
              const evenOdd = y % 2;

              return CELL_NEIGHBORS[evenOdd].map(dx, dy => x: x + dx, y: y + dy);



              Now if you change getCellIdFromCoordinates to also take an coordinates object, the final function would be:



               function getNeighborsCellId(cellId) 
              return getAllNeighborCoordinates(getCoordinatesFromCellId(cellId))
              .map(getCellIdFromCoordinates);



              (DISCLAMER: I'm not that into ES2015 yet, so there may be errors in my code.)






              share|improve this answer























                up vote
                3
                down vote



                accepted







                up vote
                3
                down vote



                accepted






                First off, I'm not a game developer, and this is a fundamental point, but wouldn't it be easier to use "regular" coordinates (for example: https://i.stack.imgur.com/XpFSQ.png) instead of "staggered" ones (https://i.stack.imgur.com/Mt5Mw.png)?




                Anyway, aside from that, in my opinion there are two major problems about your your function: A) Is doing too much and B) repeating itself.



                The main function of calculating the neighbor coordinates should be separate and take an coords object and return an array of coordinates objects. It should use a list of (either pre-calculated or hardcoded) relative coordinates of the neighbors. If you have, for example, functions to calculete the explicit north, north-east, east, etc. neighbors, then they should be sharing this data.



                const CELL_NEIGHBORS = [
                // Even:
                [dx: 0, dy: -1, dx: 0, dy: -2, dx: +1, dy: -1 /* etc. */ ],
                // Odd:
                [ /* TODO */ ]
                ];

                function getAllNeighborCoordinates(x, y)
                const evenOdd = y % 2;

                return CELL_NEIGHBORS[evenOdd].map(dx, dy => x: x + dx, y: y + dy);



                Now if you change getCellIdFromCoordinates to also take an coordinates object, the final function would be:



                 function getNeighborsCellId(cellId) 
                return getAllNeighborCoordinates(getCoordinatesFromCellId(cellId))
                .map(getCellIdFromCoordinates);



                (DISCLAMER: I'm not that into ES2015 yet, so there may be errors in my code.)






                share|improve this answer













                First off, I'm not a game developer, and this is a fundamental point, but wouldn't it be easier to use "regular" coordinates (for example: https://i.stack.imgur.com/XpFSQ.png) instead of "staggered" ones (https://i.stack.imgur.com/Mt5Mw.png)?




                Anyway, aside from that, in my opinion there are two major problems about your your function: A) Is doing too much and B) repeating itself.



                The main function of calculating the neighbor coordinates should be separate and take an coords object and return an array of coordinates objects. It should use a list of (either pre-calculated or hardcoded) relative coordinates of the neighbors. If you have, for example, functions to calculete the explicit north, north-east, east, etc. neighbors, then they should be sharing this data.



                const CELL_NEIGHBORS = [
                // Even:
                [dx: 0, dy: -1, dx: 0, dy: -2, dx: +1, dy: -1 /* etc. */ ],
                // Odd:
                [ /* TODO */ ]
                ];

                function getAllNeighborCoordinates(x, y)
                const evenOdd = y % 2;

                return CELL_NEIGHBORS[evenOdd].map(dx, dy => x: x + dx, y: y + dy);



                Now if you change getCellIdFromCoordinates to also take an coordinates object, the final function would be:



                 function getNeighborsCellId(cellId) 
                return getAllNeighborCoordinates(getCoordinatesFromCellId(cellId))
                .map(getCellIdFromCoordinates);



                (DISCLAMER: I'm not that into ES2015 yet, so there may be errors in my code.)







                share|improve this answer













                share|improve this answer



                share|improve this answer











                answered Jan 8 at 11:18









                RoToRa

                6,0121236




                6,0121236






















                     

                    draft saved


                    draft discarded


























                     


                    draft saved


                    draft discarded














                    StackExchange.ready(
                    function ()
                    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f184450%2fsimplify-this-function-which-finds-the-coordinates-of-the-neighbors-of-an-isomet%23new-answer', 'question_page');

                    );

                    Post as a guest













































































                    Popular posts from this blog

                    Greedy Best First Search implementation in Rust

                    Function to Return a JSON Like Objects Using VBA Collections and Arrays

                    C++11 CLH Lock Implementation