4
\$\begingroup\$

I have a monolithic script that powers this form:

Screenshot of form

Here it is:

$(function() {

  // Functions
  function attainDistinctTags(value) {
    var tags = value.split(',');
    tags = tags.filter(function(tag) { return /\S/.test(tag) });
    tags = tags.filter(function(item, pos, self) { return self.indexOf(item) == pos; });
    return tags;
  }
  function parseVideoId(url) {
    var pattern = /^.*(youtu.be\/|v\/|u\/\w\/|embed\/|watch\?v=|\&v=)([^#\&\?]*).*/;
    var match = url.match(pattern);
    if (match && match[2].length == 11) {
      return match[2];
    }
  }
  function buildVideoApiUrl(id) {
    var base = "https://www.googleapis.com/youtube/v3/videos";
    var parts = "snippet,contentDetails";
    var key = "AIzaSyCKQFYlDRi5BTd1A-9rhFjF8Jb_Hlfnquk";
    return base + "?part=" + parts + "&id=" + id + "&key=" + key;
  }

  // Validation
  $.validator.addMethod("youtubeVideoUrl", function (value, element) {
    return /^(https?\:\/\/)?(www\.)?(youtube\.com|youtu\.?be)\/.+$/.test(value);
  }, "Please enter a valid YouTube video url.");
  $.validator.addMethod("maximumOf2Tags", function (value, element) {
    var tags = attainDistinctTags(value);
    return tags.length <= 2;
  }, "You cannot enter more than two tags.");
  $("#submitForm").validate({
    ignore: [],
    errorElement: "span",
    errorClass: "help-block",
    highlight: function(element) {
      $(element)
        .closest('.form-group')
        .addClass('has-error')
        .removeClass('has-success');
    },
    unhighlight: function(element) {
      $(element)
        .closest('.form-group')
        .addClass('has-success')
        .removeClass('has-error');
    },
    rules: {
      url: {
        required: true,
        youtubeVideoUrl: true,
        remote: function () {
          return {
            url: buildVideoApiUrl(parseVideoId($("#url").val())),
            dataFilter: function(response) {
              var json = JSON.parse(response);
              return json.items.length !== 0;
            }
          };
        }
      },
      tags: {
        required: true,
        maximumOf2Tags: true
      }
    },
    messages: {
      url: {
        required: "Please enter a screencast link.",
        remote: "This video does not exist."
      },
      tags: {
        required: 'Please enter at least one tag.'
      }
    }
  });

  // Automatic title and description loading
  $("#url").change(function() {
    var videoUrl = $(this).val();
    // if the input is not a valid YouTube url, return.
    if(!$(this)[0].checkValidity()) {
      return;
    }
    var id = parseVideoId(videoUrl);
    var apiUrl = buildVideoApiUrl(id);
    $.get(apiUrl, function(data) {
      var item = data.items[0];
      // if the video does not exist
      if (item == undefined) {
        $("#title").val('');
        $("#description").val('');
        $("#channelName").val('');
      } else {
        $("#title").val(item.snippet.title);
        $("#description").val(item.snippet.description);
        $("#channelName").val(item.snippet.channelTitle);
      }
    });
  });

  // Tag input control
  $("#tags-input").keyup(function() {
    var tags = attainDistinctTags($(this).val());
    $("#tag-list").empty();
    tags.forEach(function(tag) {
      $("#tag-list").append("<li> <span class=\"fa fa-tag\"></span>" + tag + "</li>");
    });
  });

  // Tag autocomplete
  function split(val) {
    return val.split(/,\s*/);
  }
  function extractLast(term) {
    return split(term).pop();
  }
  $('#tags-input').bind('keydown', function(event) {
    if ( event.keyCode === $.ui.keyCode.TAB &&
      $( this ).autocomplete( "instance" ).menu.active ) {
        event.preventDefault();
      }
  }).autocomplete({
    source: function( request, response ) {
      $.getJSON( "/api/tags", {
        term: extractLast( request.term )
      }, response );
    },
    focus: function() {
      return false;
    },
    select: function( event, ui ) {
      var terms = split( this.value );
      terms.pop();
      terms.push( ui.item.value );
      terms.push( "" );
      this.value = terms.join( ", " );
      return false;
    }
  });

});

It has 4 distinct responsibilites:

  1. Validation
  2. Autocomplete
  3. Automatically populating the disabled fields when the user enters a YouTube link
  4. Rendering a tag list

At the moment I use comments to make the code a bit easier to read but I would never do this on the server - monolithic code is bad.

How can break this code down into smaller parts?

\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

You're splitting a comma separated string and getting only the first occurrence of a string. Two filters are costly and a regular expression created on each iteration is too. The following might be better. Instead, we'll use trim to see if there is a word and at the same time, check the occurrence.

function attainDistinctTags(value) {
  return value.split(',').filter(function(tag, position, tags){
    return tag.trim().length && tags.indexOf(tag) === pos;
  });
}

You might want to move out to "constants" values that could possibly be configurable. In your case, you can move out the , in the split, the 11 of parseVideoId, the base url, and the various regular expressions you have. If you want to keep them "functional", your callers can pass them in:

var TAG_DELIMITER = ',';

function attainDistinctTags(value, delimiter) {
  return (value || []).split(delimiter).filter(function(tag, position, tags){
    return tag.trim().length && tags.indexOf(tag) == pos
  });
}

attainDistinctTags(tags, TAG_DELIMITER);

Avoid modifying the DOM per iteration. I suggest you remove the element from the DOM first, append the contents, then append it back to the DOM. Perf gain may be negligible, so its up to you. The following code is terrible, but the idea is there.

$("#tags-input").keyup(function() {
  var tags = attainDistinctTags($(this).val());
  var tagList = $("#tag-list");
  var tagListContainer = tagList.parent();
  tagList.detach().empty();
  tags.forEach(tag => tagList.append("..."));
  tagListContainer.append(tagList);
});
\$\endgroup\$

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.