This is my first attempt at object oriented javascript code. I have been writing JavaScript code for few years now but this is first time I am trying to write OO javascript code. I have read about OO javascript, but I am not still very confident about it.

Could you please review it and provide your comments about this implementation of OO javascript?

"use strict";

var maps_1, maps_2, maps_3, maps_4;
var markers_array1 = [], markers_array2 = [], markers_array3 = [], markers_array4 = [];


var google_map = function (x,y,z) {
    this.map = new google.maps.Map(document.getElementById(x), {
        zoom : parseInt(z),
        center : y,
        mapnameId : google.maps.MapnameId.ROADMAP,
        disableDefaultUI : true,
        styles : {
            featurename : "poi",
            elementname : "labels",
            stylers : [{
                    visibility : "off"
                }
            ]
        }
    });
};

var google_marker = function(x,map) {
    var icon_url =  'icons/' + x.dname + '_' + ((x.dname2 === 0) ? '0.png' : '1.png');
    var lat = parseFloat(x.lat), lng = parseFloat(x.lng);

    if(lat*lng > 0) {
        this.marker = new google.maps.Marker({
                icon : icon_url,
                position: {"lat":lat,"lng":lng},
                description : '<b>'+x.name+'</b><br>'+x.desc,
                title: x.dname,
                start: x.start,
                map: map
            });

        this.marker.addListener('click', infoWindow);
    }
}

function loadJson() {
    var xmlhttp = new XMLHttpRequest();
    xmlhttp.open("POST", "ajax.php", true);
    xmlhttp.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
    xmlhttp.send(data);

    xmlhttp.onreadystatechange = function () {
        if (xmlhttp.readyState == 4) {
            loadMarkers(JSON.parse(xmlhttp.response));
        }
    }
}

function init() {
    maps_1 = new google_map('maps_1',{"lat" : 32.93,"lng" : 85.06},12);
    maps_2 = new google_map('maps_2',{"lat" : 70.88,"lng" : 45.97},12);
    maps_3 = new google_map('maps_3',{"lat" : 60.49,"lng" : 38.00},12);
    maps_4 = new google_map('maps_4',{"lat" : 85.85,"lng" : 89.63},12);

    loadJson();
}

function loadMarkers(Markers) {

    for(var y in Markers) {
        var x  = Markers[y];
        markers_array1.push(new google_marker (x,maps_1.map));
        markers_array2.push(new google_marker (x,maps_2.map));
        markers_array3.push(new google_marker (x,maps_3.map));
        markers_array4.push(new google_marker (x,maps_4.map));
    }
}
init();
share|improve this question

put on hold as unclear what you're asking by Jamal Mar 21 at 9:08

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question.If this question can be reworded to fit the rules in the help center, please edit the question.

3  
Don't mean to be rude, but I didn't see anything of OO in your code. Could you explain what you mean with OO javascript? – Mario Santini Mar 21 at 8:10
    
@MarioSantini I am using the google_map and google_marker variable to create four instance of map and about 400 markers on each map. Ofcourse it might not be best/optimized way but this is what I have figured out so far. And I am looking for advice/suggestions or comments to improve. Thanks. by object oriented javascript I mean to reuse the code and use prototype and closure to improve code performance and reduce duplicate lines of code. – burgur Mar 21 at 8:19
1  
The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How to Ask for examples, and revise the title accordingly. – Jamal Mar 21 at 9:07
    
Please read the link in my comment for examples on ideal titles. That title update didn't improve anything. – Jamal Mar 21 at 9:38
up vote 1 down vote accepted

The problem I see on your code is that you try to be OO but still thinking in a procedural way.

for example, your code is full procedural, as you didn't design your class, instead you just wrote the code in the sequence it will be executed.

So far it is not so bad written for a procedural one.

Let's go on to explain the points.

What is the purpose to wrap a google map inside a function that contains only an attribute that is the map?

How do you use it:

maps_1 = new google_map('maps_1',{"lat" : 32.93,"lng" : 85.06},12);
maps_2 = new google_map('maps_2',{"lat" : 70.88,"lng" : 45.97},12);
maps_3 = new google_map('maps_3',{"lat" : 60.49,"lng" : 38.00},12);
maps_4 = new google_map('maps_4',{"lat" : 85.85,"lng" : 89.63},12);

So why you just don't directly create the google map instead?

What mean thinking in a object orientation way. you should first think about the object you use.

In your case you have a Map, that have reference to a DOM element, a configuration, and a list of markers.

// Please provide meaningful names for parameter
var Map = function (elementId, mapConf) {
    this.markers = []; //  Will use later
    this.elementId = elementId;
    var domElement = document.getElementById(this.elementId);
    this.map = new google.maps.Map(domElement, {
        zoom : parseInt(mapConf.zoom),
        center : mapConf.center,
        mapnameId : google.maps.MapnameId.ROADMAP,
        disableDefaultUI : true,
        styles : {
            featurename : "poi",
            elementname : "labels",
            stylers : [{
                    visibility : "off"
                }
            ]
        }
    });
};

// Now you need some methods
Map.prototype.addMarkers = function(markers) {
    markers.forEach(function (marker) {
        this.markers.push(new Marker(marker, this.map, infoWindow));
    }.bind(this));
};

Now you need a Marker class:

var Maker = function (markerConf, map, clickHandler) {
    var icon_url =  'icons/' + markerConf.dname + '_' + ((markerConf.dname2 === 0) ? '0.png' : '1.png');
    var lat = parseFloat(marketConf.lat), lng = parseFloat(marketConf.lng);

    if(lat*lng > 0) {
        this.marker = new google.maps.Marker({
            icon : icon_url,
            position: {"lat":lat,"lng":lng},
            description : '<b>'+x.name+'</b><br>'+x.desc,
            title: marketConf.dname,
            start: marketConf.start,
            map: map
        });

        // You should handle this by your class (the Marker)
        // or by configuration, like shown here 
        // so it's flexible
        if (clickHandler) {
            this.marker.addListener('click', clickHandler);
        }
    }
};

So your init function will be:

function init() {
    maps_1 = new Map('maps_1',{"lat" : 32.93,"lng" : 85.06},12);
    maps_2 = new Map('maps_2',{"lat" : 70.88,"lng" : 45.97},12);
    maps_3 = new Map('maps_3',{"lat" : 60.49,"lng" : 38.00},12);
    maps_4 = new Map('maps_4',{"lat" : 85.85,"lng" : 89.63},12);

    performPostAjaxRequest("ajax.php", function (data) {
        maps_1.addMarkers(data);
        maps_2.addMarkers(data);
        maps_3.addMarkers(data);
        maps_4.addMarkers(data);
    });
}

And loadJson will change to be more general purpose:

function performPostAjaxRequest(url, cb) {
    var xmlhttp = new XMLHttpRequest();
    xmlhttp.open("POST", url, true);
    xmlhttp.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
    xmlhttp.send(data);

    xmlhttp.onreadystatechange = function () {
        if (xmlhttp.readyState == 4) {
            cb(JSON.parse(xmlhttp.response));
        }
    }
}

This is just a first step, a lot more could be done, the purpose of OO is to design the objects you need as classes, and put all the logic that is connected with those objects inside the class. You should not have a function that know how to attach a Marker object to a Map object. Instead, the Map object should know how to do that. You just need to know that Maps need Markers. When you define a class it's important to start the name with a capital letter, to distinguish a class constructor from a normal function.

share|improve this answer
    
Thank you very much for putting in efforts to rewrite and explain all codes. This is what I was looking for, I mean after reading the examples (mostly cars and animals) for OOP I was still not sure how I could I modify it for my needs (maps and markers). And overall in terms of object. Now I have some more ideas thanks a lot. – burgur Mar 21 at 9:33

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