2
\$\begingroup\$

I am retrieving data from a json object and even though code works I am not sure if I am following best practices.

In particular some of the objects seems too deep (like value.metadata.connections.comments.total) and not sure if there is a better way to avoid creating all my markup in JS.

(function getData() {

    $.ajax({
        url: 'data.json',
        type: 'GET',
        dataType: 'json',
        success: function(response) {
            console.log('success!');
            var fixtureHTML = '';

           for(var i = 0; i < 5; i++){
                var value = response.data[i];   

                fixtureHTML += '<div class="media"><div class="media-left">';
                //Author avatar + link
                if(value.user.pictures !== null) {
                    fixtureHTML += '<a href="' + value.user.link +'"><img class="media-object" src="' + value.user.pictures.sizes[2].link + '" alt="' + value.user.name + '"></a><p>'+ value.user.name + '</p>';
                } else {
                    fixtureHTML += '<a href="' + value.user.link +'"><img class="media-object no-thumb" src=""></a><p>'+ value.user.name + '</p>';
                }
                fixtureHTML += '</div><div class="media-body">';
                fixtureHTML += '<h4 class="media-heading"><a href="' + value.link +'">'+ value.name + '</a></h4>' + '<span class="more">' + textFormatter(value.description) + '</span>';
                console.log(value.user.name);

                //Number of plays + comments + likes inside meta-info
                fixtureHTML += '<div class="meta-info">';
                fixtureHTML +=  '<span class="comments">' + value.metadata.connections.comments.total + '</span>';
                fixtureHTML += '<span class="likes">' + abbreviateNumber(value.metadata.connections.likes.total) + '</span>';
                fixtureHTML += '<span class="plays">' + abbreviateNumber(value.stats.plays) + '</span>';
                fixtureHTML += '</div></div></div>';

            }

            // Append generated HTML to DOM
            $('.js-load').append(fixtureHTML);

            // Show more-less  content

            var showChar = 300;  // How many characters are shown by default
            var ellipsestext = "...";
            var moretext = "Show more +";
            var lesstext = "Show less -";


            $('.more').each(function() {
                var content = $(this).html();

                if(content.length > showChar) {

                    var c = content.substr(0, showChar);
                    var h = content.substr(showChar, content.length - showChar);
                    var html = c + '<span class="moreellipses">' + ellipsestext+ '&nbsp;</span><span class="morecontent"><span>' + h + '</span>&nbsp;&nbsp;<a href="" class="morelink">' + moretext + '</a></span>';
                    $(this).html(html);
                }

            });

            $(".morelink").click(function(){
                if($(this).hasClass("less")) {
                    $(this).removeClass("less");
                    $(this).html(moretext);
                } else {
                    $(this).addClass("less");
                    $(this).html(lesstext);
                }
                $(this).parent().prev().toggle();
                $(this).prev().toggle();
                return false;
            });



        }, // End of Success
        error: function() {
            console.log('errror');
        }
    });



    // Format string of text

    function textFormatter(str) {
      return str.replace(/\r?\n/g, '<br />')
        .replace(/((https?|ftp):\/\/[^\s/$.?#].[^\s\\\<]*)/g, "<a href='$1'>$1</a>");
    }

    // Format number to abbreviation
    function abbreviateNumber(value) {
        var newValue = value;
        if (value >= 1000) {
            var suffixes = ["", "k", "m", "b","t"];
            var suffixNum = Math.floor( (""+value).length/3 );
            var shortValue = '';
            for (var precision = 2; precision >= 1; precision--) {
                shortValue = parseFloat( (suffixNum != 0 ? (value / Math.pow(1000,suffixNum) ) : value).toPrecision(precision));
                var dotLessShortValue = (shortValue + '').replace(/[^a-zA-Z 0-9]+/g,'');
                if (dotLessShortValue.length <= 2) { break; }
            }
            if (shortValue % 1 != 0)  shortNum = shortValue.toFixed(1);
            newValue = shortValue+suffixes[suffixNum];
        }
        return newValue;
    }






    // Filter by user that have more than 10 likes

    var checkbox = $('.js-check');

    checkbox.on('click',function(){
        if($(this).is(':checked'))
            alert('ciao');
        else
            alert('unchecked'); // unchecked
    });


    // Filter search by text functionality

    $('.js-filter').keyup(function () {

        var value = $(this).val();

            $('.media-body').each(function() {
                if ($(this).text().search(value) > -1) {
                    $(this).parent().show();
                }
                else {
                    $(this).parent().hide();
                }
            });
    });


})(); //end of main function
\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Code Review! I hope you'll have good reviews of your code! \$\endgroup\$
    – Marc-Andre
    Commented Jun 30, 2016 at 12:43

2 Answers 2

1
\$\begingroup\$

Be Consistent

  • Quotes used for strings. var fixtureHTML = ''; vs var moretext = "Show more +";.
  • Else-Statement Formatting. } else { vs putting the else { on its own line.
  • Spacing. function () { vs function (){. Sure, it's nit-picking but consistency adds significant amounts of readability. And being able to read code quickly is half the battle.
  • If-Statement Brackets. I would suggest always using them. It'll save you (or someone else) from a rookie mistake when maintaining the code. It adds consistency. etc.

Other Suggestions

  • Regex. I would suggest adding a bit of a descriptive comment above each one as to what they're doing. Regex's readability is near rock-bottom. Someone else or even you a few weeks from now might have to maintain this code and then you're going to have to spend some amount of time re-figuring out what these regex expressions do.

I know these suggestions aren't much, but every little bit helps right?

\$\endgroup\$
0
\$\begingroup\$

Minor comments:

  • CSS actually handles the display of text overflow with ellipsises https://developer.mozilla.org/en-US/docs/Web/CSS/text-overflow

  • I would use some debug function that takes and evaluates a severity before writing to console.log ( like in console.log('success!');)

  • When there is no-thumb you should still provide an alt text.

  • From a naming perspective, c and h are pretty terrible

  • Check out $.toggleClass() for the $(".morelink").click handler

  • function textFormatter could use a tad more commenting, I have no idea what it does

\$\endgroup\$

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.