Google Maps marker management for games

Question

Is there a better way to design this code? I have read that too many function parameters is a sign of poorly design code, and would love some feedback on this code.

The functions Google Maps markers and update the state.

Also any other feedback unrelated is welcome.

Functions:

// Initialising map and markers
export const handleApiLoaded = (
  games,
  map,
  setGameMarkers,
  setInfoWindow,
  handleMarkerClick
) => {
  const infoWindow = new google.maps.InfoWindow({});

  const gameMarkers = [];
  for (const game of games) {
    const newMarker = createGameMarker(
      map,
      game,
      infoWindow,
      gameMarkers,
      handleMarkerClick
    );

    newMarker ? gameMarkers.push(newMarker) : null;
  }

  // Update state
  setGameMarkers(gameMarkers);
  setInfoWindow(infoWindow);
};


// Create marker object for game
export const createGameMarker = (
  map,
  game,
  infoWindow,
  gameMarkers,
  handleMarkerClick
) => {
  const [lat, lng] = [parseFloat(game.latitude), parseFloat(game.longitude)];

  if (locationIsUnoccupied(lat, lng, gameMarkers)) {
    var icon = {
      url: GAME_ICON_URL,
      scaledSize: new google.maps.Size(30, 45),
    };

    var marker = new google.maps.Marker({
      position: {
        lat: lat,
        lng: lng,
      },
      icon: icon,
      map,
    });

    marker.addListener("click", () => {
      infoWindow.close();
      map.panTo(marker.getPosition());
      handleMarkerClick(marker, map);
      infoWindow.setContent(createGameInfoWindow(game));
      infoWindow.open(map, marker);
    });

    return marker;
  }
};

Answer 1

From a short review;

• locationIsUnoccupied and createGameInfoWindow are mysterious, where are they defined?
• newMarker ? gameMarkers.push(newMarker) : null; should trigger jshint, I would just go for the if statement
• I would structure all the code and functions in to MVC (model view controller)
  • So GAME_ICON_URL becomes model.GAME_ICON_URL
  • So createGameInfoWindow becomes ui.createGameInfoWindow
  • So locationIsUnoccupied becomes model.locationIsUnoccupied

This could make your code look like;

// Initialising map and markers
export function handleApiLoaded(model, ui, controller){
  ui.infoWindow = new google.maps.InfoWindow({});

  model.gameMarkers = [];
  for (const game of games) {
    addGameMarker(game, model, ui, controller);
  }

  //Not sure the below would still make sense?
  model.setGameMarkers(gameMarkers);
  ui.setInfoWindow(ui.infoWindow);
};


// Create marker object for game
export function addGameMarker(game, model, ui, controller){

  const [lat, lng] = [parseFloat(game.latitude), parseFloat(game.longitude)];

  if (model.locationIsUnoccupied(lat, lng, model.gameMarkers)) {
    var icon = {
      url: model.GAME_ICON_URL,
      //Why 30 and 45, a comment would be good here
      scaledSize: new google.maps.Size(30, 45),
    };

    var marker = new google.maps.Marker({
      position: {
        lat: lat,
        lng: lng,
      },
      icon: icon,
      map: model.map,
    });

    marker.addListener("click", () => {
      ui.infoWindow.close();
      //Maybe `map` should be part of `ui`, deep thoughts to be had..
      model.map.panTo(marker.getPosition());
      ui.handleMarkerClick(marker, map);
      ui.infoWindow.setContent(createGameInfoWindow(game));
      ui.infoWindow.open(map, marker);
    });

    model.gameMarkers.push(marker);
  }
};