Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I basically have a folder view structure, that each time I call grabs the value of the selected field and gets that specific folder data. I was wondering if there's a better way to do this by caching or something like that. So for example if I was to request that same folder it would be stored in memory, instead of having to make another get request.

var getFolder = function() {

    if ($('.list-group-item:hover').css('margin-left') === '0px') {
        $('.fold').remove();
    }

    var promises = [];

    $('.list-group-item').css("background-color", ""); // clear last selected

    $.get("/folder", {
        folder: $('.list-group-item:hover').attr('title') //.clone().children().text()
    }, function(data) {
        var folders = data.CommonPrefixes; // folders
        var jsonData = data.Contents; //json data

        var selected = $('.list-group-item:hover').css("background-color", "rgb(0, 196, 255)"); // new selected color



        var parFolder = parseInt($('.list-group-item:hover').css('margin-left'), 10);
        var add = parFolder + 15;
        var subFolder = add.toString() + 'px';

        if (folders.length > 0) {

            for (var k = 0; k < folders.length; k++) {

                var value = folders[k].Prefix;
                var displayVal = value.split('/').slice(-2).join("");


                var book = "<li class='fold list-group-item' onclick='getFolder()' title='" + value + "'>" + "<span class='glyphicon glyphicon-book'></span>" + displayVal + "<span class='glyphicon glyphicon-remove-sign' onclick='deleteFldr()'></span>" + "</li>"

                $(book).insertAfter(".list-group-item:hover").css('margin-left', subFolder);

            }
        }

        if (jsonData.length > 0) {
            for (var i = 0; i < jsonData.length; i++) {

                var req = $.get('/fldrContents', {
                    contents: jsonData[i].Key
                }, function(data) {
                    //var b = JSON.parse(data.body);
                });

                promises.push(req);

            }
        }

        $.when.apply(null, promises).done(function(d) {

            if ($('.dynamic')) {
                $('.dynamic').remove();
            }
            for (var i = 0; i < promises.length; i++) {

                var posTop;
                var posLeft;

                var z = JSON.parse(promises[i].responseText);
                console.log(z)

                var fileURL = z.path;

                var lastChar = fileURL.charAt(fileURL.length - 1);
                if (lastChar === '/') {
                    continue;
                }
                $('.description').css('display', 'none')

                var fileBody = JSON.parse(z.body);
                var website = fileBody.website;
                var title = fileBody.title;
                var description = fileBody.description;
                var created = fileBody.created;
                var canvasData = fileBody.canvas;

                // console.log(canvasData)

                if (fileBody.hasOwnProperty('description')) {

                    $('.description').css('display', 'block')
                }



                if (fileBody.hasOwnProperty('position')) {

                    var posTop = fileBody.position.top;
                    var posLeft = fileBody.position.left;

                    $('dynamic').css({
                        'top': posTop,
                        'left': posLeft
                    });



                    $(".static").append('<div class="dynamic col-md-6" style="top:' + posTop + 'px;left:' + posLeft + 'px">' +
                        '<div class="panel">' +
                        '<canvas id="canvas"></canvas>' +
                        '<div class="panel-body">' +
                        '<a href="' + website + '" target="_blank" class="title" title="Go to ' + website + '">' + title + '</a>' +
                        '<span class="website-edit"></span>' +
                        '<div class="description">' + description + '</div>' +
                        '</div>' +
                        '<div class="panel-footer">' +
                        '<span class="glyphicon glyphicon-bookmark" title="' + fileURL + '"></span>' +
                        '<span class="glyphicon glyphicon-time" title="' + created + '"></span>' +
                        '<span class="glyphicon glyphicon-edit" title="Edit this bookmark" onclick="edit()"></span>' +
                        '<span class="delete glyphicon glyphicon-remove-sign" title="Delete this bookmark" onclick="deleteBookmark()"></span>' +
                        '</div>' +
                        '</div>' +
                        '</div>');
                }

                if (!fileBody.hasOwnProperty('position')) {
                    $(".static").append('<div class="dynamic col-md-6">' +
                        '<div class="panel">' +
                        '<canvas id="canvas"></canvas>' +
                        '<div class="panel-body">' +
                        '<a href="' + website + '" target="_blank" class="title" title="Go to ' + website + '">' + title + '</a>' +
                        '<span class="website-edit"></span>' +
                        '<div class="description">' + description + '</div>' +
                        '</div>' +
                        '<div class="panel-footer">' +
                        '<span class="glyphicon glyphicon-bookmark" title="' + fileURL + '"></span>' +
                        '<span class="glyphicon glyphicon-time" title="' + created + '"></span>' +
                        '<span class="glyphicon glyphicon-edit" title="Edit this bookmark" onclick="edit()"></span>' +
                        '<span class="delete glyphicon glyphicon-remove-sign" title="Delete this bookmark" onclick="deleteBookmark()"></span>' +
                        '</div>' +
                        '</div>' +
                        '</div>')

                }

                if (fileBody.hasOwnProperty('canvas')) {

                    var canvas = document.getElementById('canvas');
                    var ctx = canvas.getContext('2d');
                    canvasData = new Image();
                    canvasData.src = promises[i];



                }



                $('.dynamic').draggable({
                    handle: "div",
                    stop: function(event, ui) {

                        var changedFile = $(this).children('.panel').children('.panel-footer').children('.glyphicon-bookmark').attr('title');

                        var pos = ui.position
                        var title = $(this).children('.panel').children('.panel-body').children('a.title').text();
                        var description = $(this).children('.panel').children('.panel-body').children('.description').text();
                        var website = $(this).children('.panel').children('.panel-body').children('a.title').attr('href');
                        var created = $(this).children('.panel').children('.panel-footer').children('.glyphicon-time').attr('title');
                        var canvas = $(this).children('.panel').children('#canvas') // fix this
                        var canvasData = canvas[0].toDataURL("image/png");

                        console.log(created + ' ' + title + ' ' + description + ' ' + canvasData)

                        $.post('/cssPosition', {
                            website: website,
                            title: title,
                            description: description,
                            position: pos,
                            folder: changedFile,
                            created: created,
                            canvas: canvasData
                        }, function() {

                        });
                        //location.reload();

                    }
                });



            }


        });


    });
}
share|improve this question
    
jQuery automatically caches stuff if you set cache: true in the ajax options. No need to roll your own –  Flambino Jun 28 at 15:17
    
i saw that before, but i was wondering if even before the get call you could store it, because jquery caches it but its still making another request –  Seeker Jun 28 at 16:22
    
First, check that the request actually goes anywhere. jQuery's cache: true basically allows the browser to use its cache. A request is still created (and shows up in the dev tools), but it may never leave your computer if the browser has a cache - and your server is saying that it's ok to cache responses. Again, the console/network log in the dev tools will tell you what the request did. You can still make your own cache if you need to, but that's a question for, say, StackOverflow. This site is for reviewing existing code, so we can't review code you haven't written. –  Flambino Jun 28 at 16:34

1 Answer 1

var selected = $('.list-group-item:hover').css("background-color", "rgb(0, 196, 255)");

rgb(0, 196, 255 tells me nothing, as I am not a computer and I can't visualize a color with just those numbers in my head.

Create a variable called with the name of the equivalent color in all caps, and set it to that rgb string.

For example, if you were changing the background to red, you would put:

var REDISH_ORANGE = "rgb(255, 68, 0)";

var selected = $('.list-group-item:hover').css("background-color", REDISH_ORANGE);

var parFolder = parseInt($('.list-group-item:hover').css('margin-left'), 10);
var add = parFolder + 15;
var subFolder = add.toString() + 'px';

This is a waste of memory; you are only using parFolder and add a single time, and that single time is to help create the variable right beneath it.

Just chain these together and stick them in subFolder:

var subFolder = (parseInt($('.list-group-item:hover').css('margin-left', 10) + 15).toString() + 'px';

Yes, you are only using subFolder once, but the name "subFolder" is describing what the value is. Names like "parFolder" (which I assume is short for "Parsed int folder", or something like that) and "add" sound like "filler words" and mean nothing to the variable or context.


You use

$('.list-group-item:hover')

quite a lot. Stick this into a variable so it is easier to access.


In your function, you should "use strict";.


This code was a little difficult to review without seeing the webpage, and without seeing the file structure/tree.

If you could add these to your post, I could edit my answer to add more improvements.

share|improve this answer
    
thanks for the review! I'm basically replicating a folder type structure for the UI and this particular function is getting all the contents of the folder and appending it to the view. –  Seeker Jul 15 at 20:12

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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