3
\$\begingroup\$

I am working on a university project that consists of creating a map application with openlayers as well as the necessary User-Interface. I am using jQuery, native JavaScript, PHP and of course HTML/CSS

The problem I have (mostly because I am not that experienced in web development with jQuery and JavaScript) is that I have lots of code inside a jQuery click callback function or an ajax success-callback . I have the feeling that this way is not the right way and also somehow overloaded. I did not find answers while googling around that deal with this specific problem and provide a better way.

To give you an example here is the code of one js-file I was currently working on. I want to post the full code to illustrate my dilemma.

The general purpose of this file is to load step by step UI elements with specific content fetched from a database, as well as updating or removing when certain contents are chosen.

var $info_panel = $("#route-update-info-panel"); //global -> namespace from sources.js

// eventually put html code in hidden divs and show them step by step?!

$("#route-update-get-dataset").click(function() {
    if ($("#update-routes-select-container").length) {
        $("#update-routes-select-container").remove();
        $("#route-update-get-data").remove();
        $("#update-slider-container").remove();
        $("#update-single-routes-container").remove();
        $("#route-update-button-container").remove();
    }

    var $route_type = $("#route-update-routelist").val();

    if (!$route_type) {
        $info_panel.html('<p>Routeklasse auswählen!</p>');
        return;
    } else {
        $info_panel.html('');
        $(`<div id="update-routes-select-container" class="input-field col s12">
                <select id="update-routes-select">
                    <option value="" disabled selected>zu bearbeitende Route auswählen</option>
                </select>
            </div>`)
            .appendTo('#update-routes');
    }

    $route_list = $("#update-routes-select");
    $route_list.on('contentChanged', function() {
        $(this).material_select();
    });
    // https://stackoverflow.com/questions/29132125/how-to-dynamically-modify-select-in-materialize-css-framework

    $.post('get_dataset.php', 
    {
        route_type: $route_type
    },function(data, status) {
        $(data).appendTo("#update-routes-select");
        $route_list.trigger('contentChanged');
        $(`<button id="route-update-get-data" class="btn waves-effect waves-light" type="submit" name="action">Bearbeiten
                <i class="material-icons right">create</i>
            </button>`)
                .appendTo("#update-routes");        
    });
});

var slider3; // bad globals -> namespace from sources.js

$("#update-routes").on('click', '#route-update-get-data', function(evt) {
    $id = $("#update-routes-select").val();
    $route_type = $("#route-update-routelist").val();
    if (!$id) {
        $info_panel.html('<p>Route auswählen!</p>');
        return;
    } else {
        $info_panel.html('');
    }

    $.post('get_data.php',
    {
        id: $id,
        route_type: $route_type
    }, function(data, status) {
        removeFromMap([
            {
                map_identifier: 'name',
                val: 'updateRouteLayer'
            }
        ]);

        var geojson = new ol.format.GeoJSON();
        var features = geojson.readFeatures(data, { 
            defaultDataProjection: 'EPSG:4326',
            featureProjection:'EPSG:3857' }
        );

        var updateRouteVectorSource = new ol.source.Vector({
            features: features
        });

        var updateRouteVectorLayer = new ol.layer.Vector({
            name: 'updateRouteLayer',
            source: updateRouteVectorSource,
            style: new ol.style.Style({
                stroke: new ol.style.Stroke({
                    color: 'tomato',
                    width: 4
                })
            })
        });

        var feature = features[0];
        var extent = updateRouteVectorSource.getExtent();
        map.addLayer(updateRouteVectorLayer);
        map.getView().fit(extent, {
            size: map.getSize(),
            duration: 500
        });

        var s_pav = parseInt(feature.get('s_pav')*100);

        if (!$("#update-slider-container").length) {
            $(`<div id="update-slider-container">
                <p>Anteil von befestigtem und unbefestigtem Belag wählen</p>
                <div id="update-route-surface-choice-slider"></div> 
                <div id="update-route-surface-choice-vals"></div>
                </div>`)
                    .appendTo("#update-routes");

            slider3 = document.getElementById('update-route-surface-choice-slider');

            noUiSlider.create(slider3, {
                start: [s_pav],
                connect: [true, true],
                behaviour: 'drag',
                step: 1,
                tooltips: false,
                orientation: 'horizontal', // 'horizontal' or 'vertical'
                range: {
                    'min': 0,
                    'max': 100
                },
                format: wNumb({
                    decimals: 0
                })
            });
        } else {
            slider3.noUiSlider.set(s_pav);
        }

        var update_pav, update_unpav;

        slider3.noUiSlider.on('update', function() {
            update_pav = slider3.noUiSlider.get();
            update_unpav = 100-slider3.noUiSlider.get();
            $('#update-route-surface-choice-vals').html(`<p>befestigt: ${update_pav} % | unbefestigt: ${update_unpav} %</p>`);
        });

        var art = parseInt(feature.get('art'));

        if (!$("#update-single-routes-container").length) {
            $(`<div id="update-single-routes-container" class="input-field col s12">
                    <select id="route-update-routetype" multiple>
                    <option id="update-routetype-select-heading" value="" disabled selected>Routentyp wählen</option>
                    </select>
                </div>`)
                    .appendTo("#update-routes");

            $update_route_type = $("#route-update-routetype");
            $update_route_type.on('contentChanged', function() {
                $(this).material_select();
            });
        }    

        $("#update-routetype-select-heading")
            .nextAll()
            .each(function(i, elem) {
                    elem.remove();
                });

        // very ugly but until now it does the job :)
        switch(art) {
            case 1:
                $(`<option value="001" selected>naturnah</option>
                    <option value="010">stadtnah</option>
                    <option value="100">innerstädtisch</option>`)
                        .appendTo("#route-update-routetype");
                $update_route_type.trigger('contentChanged');
                break;
            case 2:
                $(`<option value="001">naturnah</option>
                    <option value="010" selected>stadtnah</option>
                    <option value="100">innerstädtisch</option>`)
                    .appendTo("#route-update-routetype");
                $update_route_type.trigger('contentChanged');
                 break;
            case 3:
                $(`<option value="001" selected>naturnah</option>
                    <option value="010" selected>stadtnah</option>
                    <option value="100">innerstädtisch</option>`)
                        .appendTo("#route-update-routetype");
                $update_route_type.trigger('contentChanged');
                break;
            case 4:
                $(`<option value="001">naturnah</option>
                    <option value="010">stadtnah</option>
                    <option value="100" selected>innerstädtisch</option>`)
                        .appendTo("#route-update-routetype");
                $update_route_type.trigger('contentChanged');
                break;
            case 5:
                $(`<option value="001" selected>naturnah</option>
                    <option value="010">stadtnah</option>
                    <option value="100" selected>innerstädtisch</option>`)
                        .appendTo("#route-update-routetype");
                $update_route_type.trigger('contentChanged');
                break;
            case 6:
                $(`<option value="001">naturnah</option>
                    <option value="010" selected>stadtnah</option>
                    <option value="100" selected>innerstädtisch</option>`)
                        .appendTo("#route-update-routetype");
                $update_route_type.trigger('contentChanged');
                break;
            case 7:
                $(`<option value="001" selected>naturnah</option>
                    <option value="010" selected>stadtnah</option>
                    <option value="100" selected>innerstädtisch</option>`)
                        .appendTo("#route-update-routetype");
                $update_route_type.trigger('contentChanged');
                break;
            default:
                $(`<option value="001">naturnah</option>
                    <option value="010">stadtnah</option>
                    <option value="100">innerstädtisch</option>`)
                        .appendTo("#route-update-routetype");
                $update_route_type.trigger('contentChanged');
                break;
        }  

        if (!$("#route-update-button-container").length) {
            $(`<div id="route-update-button-container">
                <button id="update-route-send" class="btn waves-effect waves-light" type="submit" name="action">Abschicken
                    <i class="material-icons right">send</i>
                </button>
                &nbsp;
                <button id="update-route-reset" class="btn waves-effect waves-light" type="submit" name="action">Zurücksetzen
                    <i class="material-icons right">delete</i>
                </button>
                </div>`)
                    .appendTo("#update-routes");
    });
});

At the moment this file is not finished at all but while writing the code that feeling mentioned above grew bigger and bigger. Any help or advice, criticism is greatly appreciated on how to write not such overloaded callback functions...

\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

Bit field

Just a quick note on the large and hacky switch statement you use to create selected options.

The variable art is a bit field, with the first bit for the top selection and to the third bit the bottom. The whole switch can be simplified with a bit of binary logic and math

art = art < 8 ? art : 0; // to make art does not contain values out of range.

$(["naturnah","stadtnah","innerstädtisch"].reduce((str, name, i) => {
    return `${str}<option 
      value="${(1 << i).toString(2).padStart(3,"0")}" 
      ${art & (1 << i) ? " selected" : ""}>${name}</option>`;
  },""
).appendTo("#route-update-routetype");

$update_route_type.trigger('contentChanged');

To check a bit you use bitwise & (and) to mask (remove) all unwanted bits. If the result is not zero the bit is on. To create the mask shift 1 left to the bit position you want.

const isBitOn = (bitField, bit) => !(bitField & (1 << bit));
//                                 ^          ^    ^^ ^^^ bit number
//                                 |          |    ||
//                                 |          |    || shift left
//                                 |          |   
//                                 |          | bitwise & to mask out unwanted bits
//                                 |
//                                 | Not to check result is not zero
\$\endgroup\$
3
  • \$\begingroup\$ Thanks a lot! Just yesterday I read something about these bitwise operators in more detail and thought maybe I could solve this large switch statement with that. But surely I would not have come as far as your solution. I will try to understand it in detail! \$\endgroup\$
    – phloose
    Aug 5, 2018 at 6:52
  • \$\begingroup\$ thanks for this nice solution! i got it :) do you have some advice where i can learn more about this bitwise stuff? (i know, google ;) but maybe you have a good tip...) \$\endgroup\$
    – phloose
    Aug 5, 2018 at 8:28
  • \$\begingroup\$ @moekloar MDN has a section on bitwise operators that includes an example illustrating bit fields developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/… \$\endgroup\$
    – Blindman67
    Aug 5, 2018 at 10:36
0
\$\begingroup\$

It's always a good idea to decouple low-level code, like the one jQuery is intended for (show a div, a click event) from high-level code (an action about your businness, in this case, the map).

A simple example is: imagine that the logic you have inside that click event callback should be triggered each 5 minutes... Would you fake the click event to execute that logic? Or imagine that this action can be triggered by another button too, what would you do?

Wouldn't it be better to have it in a method like updateRoutes() and inside it, methods like addDefaultOption(), getRouteInfo()...? You should try to structure your code as much as independent from your tools as possible, and jQuery is a tool.

Another good piece of advice would be: find duplications and extract them to variables, functions... and give them a proper name that express their responsibility.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for your suggestions! I also have some functions for actions I use a lot, like removing elements from the map or creating markers but some actions I dont want to put into functions because I just need them once for a specific meaning. E.g. In the code above I have a lot of DOM action/appending html to elements. For me it seems very bloated. Would it maybe better to create empty but hidden divs and all the necessary elements and show/hide them when I need them? Or does something like html templates exist that I can load via js/jQuery? \$\endgroup\$
    – phloose
    Aug 4, 2018 at 12:24
  • \$\begingroup\$ You can have functions inside functions, for the ones you only use once. For templating, there are handful options. One of the simple ones is jquery-template. \$\endgroup\$ Aug 4, 2018 at 12:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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