2
\$\begingroup\$

Is this code good practice? What kind of design pattern should I use for this specific situation?

The following function creates a new marker on a map. It sets a new image for it, except when the marker references the user location.

$scope.addMarker = function (lat, lng, isUserLocation) {

    if (typeof(isUserLocation) === undefined || isUserLocation === false) 
        var icon = "img/marker-map.png";

    $scope.markers.push({
        latitude: parseFloat(lat),
        longitude: parseFloat(lng),
        icon: icon
    });
}

Maybe just (!isUserLocation) is more readable - is it error prone?

Here's a call to the function to create the "user location" marker:

$scope.addMarker(position.coords.latitude, position.coords.longitude, true);

If someone reads that, he/she wouldn't be able to say what that true is for and would have to look for the addMarker() function, for example. But maybe that's no big deal or is it?

Here's the function that creates all other markers:

$scope.populateMarkers = function(locations) {
    for (var i = 0; i < locations.length; i++) {
        $scope.addMarker(locations[i].Latitude, locations[i].Longitude);
    }
}

This is the HTML / Angular directives that references the markers inside the map:

  <markers>
    <marker ng-repeat="marker in markers" coords="marker" icon="marker.icon">
    </marker>
  </markers>

It works, but since I'm very new to AngularJS and I also thought about many different ways to do just that, I'd like to know how people would do it and/or whether there is a recommended design pattern for it.

\$\endgroup\$
2
\$\begingroup\$

Interesting question,

I haven't found an obvious best way to do this.

For sure, sending true as a parameter should always be avoided, I'd rather see

var IS_USER_LOCATION = true;
$scope.addMarker(position.coords.latitude, position.coords.longitude, IS_USER_LOCATION);

though probably I would create 2 functions, 1 called addMarker and 1 called addUserMarker.

Finally, it amused me that the first 2 parameters in both calls are properties of the same object, so why not just ask for a coordinates object ?

$scope.addMarker = function ( coordinates ) {
    $scope.markers.push({
        latitude: parseFloat(coordinates.latitude),
        longitude: parseFloat(coordinates.longitude),
        icon: "img/marker-map.png";
    });
}

$scope.addUserMarker = function ( coordinates ) {
    $scope.markers.push({
        latitude: parseFloat(coordinates.latitude),
        longitude: parseFloat(coordinates.longitude),
    });
}

$scope.addUserMarker(position);

$scope.populateMarkers = function(locations) {
    for (var i = 0; i < locations.length; i++) {
        $scope.addMarker(locations[i]);
    }
}
\$\endgroup\$
1
\$\begingroup\$

Indeed

if (!isUserLocation)

is probably preferable to the more verbose

if (typeof(isUserLocation) === undefined || isUserLocation === false) 

In that case, isUserLocation is a function argument, so using it won't throw any error (in contrast to a variable) and hence there is no need for typeof. Also note that null === false evaluates to false, so the two expressions are different, strictly speaking.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.