5
\$\begingroup\$

I wanted to get some eyes on some code that I wrote, and see if anyone can help improve what I did. Just looking for some constructive feedback to help make my code more performant, and more elegant.

From a high level my code needs to accomplish the following:

Create a function that takes an array of objects that each represent an event. Each object in the array has a start and end time represented in minutes. The start and end times are represented as minutes after 9am. For example {start:30, end: 60} represents an event that starts at 9:30am and ends at 10am. The function takes the objects, and plots them as events on a vertical timeline. Events that have crossover will split the container in half, while events that don’t crossover will take the whole container width.

Here is the JS that I wrote to accomplish the above task:

//sample data - array of objects that represent events
var myArray = [
{start: 30, end: 130},
{start: 140, end: 147},
{start: 145, end: 155},
{start: 150, end: 175},
{start: 200, end: 250},
];

var myFunc = function(a) {
  //sort the input array from earliest start time to latest. Items with identical start times will compare end times - the longer duration item resolves with lower index. Assumes no exact duplicate items.
  var sortedDates = a.sort(function(a,b) {
    var caseOne= (a.end>b.end) ? -1 : 1,
        caseTwo= a.start - b.start;

    return (a.start===b.start) ? caseOne : caseTwo;
  });

  for(var i=0; i<sortedDates.length; i++) {
    var currentItem = sortedDates[i],
        itemDuration = currentItem.end-currentItem.start,
        itemTop = currentItem.start,
        prevItem = sortedDates[i-1],
        nextItem = sortedDates[i+1],
        newDiv = document.createElement('div');

    //set a default direction to each item
    currentItem.direction = 'left';

    //determine which items overlap and set a property to indicate
    if(nextItem !== undefined && (currentItem.end - nextItem.start > 0)) {
      currentItem.overlap = true;
      nextItem.overlap = true;
    }

    //ensure items flow in UI by staggering overlapping items
    if(prevItem !== undefined && (prevItem.direction === 'left')) {
      currentItem.direction = 'right';
    }

    //set class names on new DOM element based on overlap
    if(currentItem.overlap === true) {
      if(currentItem.hasOwnProperty('direction')) {
        newDiv.setAttribute('class', 'split '+currentItem.direction);
      }
      else {
        newDiv.setAttribute('class', 'split');
      }
    }

    //set the size and position based on computed duration and start time
    newDiv.setAttribute('style', 'height:'+itemDuration+'px; top:'+itemTop+'px');

    //insert new element into DOM
    document.getElementById('stuff').appendChild(newDiv);
  }
};

Here is a rough working example.

\$\endgroup\$
1
\$\begingroup\$

Comments are in the code:

var myArray = [
    { start: 30, end: 130 },
    { start: 140, end: 147 },
    { start: 145, end: 155 },
    { start: 150, end: 175 },
    { start: 200, end: 250 }
];

var myFunc = function (dates) {

    // sort the input array from earliest start time to latest. Items with
    // identical start times will compare end times - the longer duration
    // item resolves with lower index. Assumes no exact duplicate items.
    var sortedDates = dates.sort(function (a, b) {
            // replaced the 
            // var caseOne = (a.end > b.end) ? -1 : 1,
            //     caseTwo = a.start - b.start;
            // return (a.start===b.start) ? caseOne : caseTwo
            // with a shorter short cut, considering the start value are the same,
            // it evaluates to falsy (0) and the other part is taken, if evaluated 
            // to truthy, the value of the first difference is taken.
            return a.start - b.start || b.end - a.end;
        }),
        factor = 2; // for some more space

    // i change the for (...) loop to the more handy Array.forEach loop
    sortedDates.forEach(function (item, i, o) {

        var itemDuration = factor * (item.end - item.start), // applied factor
            itemTop = factor * item.start,                   // applied factor
            prevItem = o[i - 1],
            nextItem = o[i + 1],
            newDiv = document.createElement('div');

        // set a default direction to each item
        item.direction = 'left';

        // determine which items overlap and set a property to indicate
        // changed the comparison nextItem !== undefined to simply nextItem
        // and the subtraction to a direct comparison
        if (nextItem && item.end > nextItem.start) {
            item.overlap = true;
            nextItem.overlap = true;
        }

        // ensure items flow in UI by staggering overlapping items
        // changed the comparison prevItem !== undefined to simply prevItem
        if (prevItem && prevItem.direction === 'left') {
            item.direction = 'right';
        }

        // set class names on new DOM element based on overlap
        // replaced item.overlap === true with item.overlap because if not set
        // then it evaluates to false and if set, but false the it stays false, 
        // otherwise if set and set to true, it evaluates true, then no furter
        // comparison is needed

        // the other change is the comparison of item.hasOwnProperty('direction')
        // here logical or is applied, because item.direction evaluates to false
        // if not set, with the || '' we get an empty string, but if set, then
        // only this result becomes the value
        item.overlap && newDiv.setAttribute('class', 'split ' + item.direction || '');

        //set the size and position based on computed duration and start time
        newDiv.setAttribute('style', 'height:' + itemDuration + 'px; top:' + itemTop + 'px');
        // to see some information
        newDiv.innerText = i + ': ' + item.start + ' - ' + item.end;

        //insert new element into DOM
        console.log(newDiv);
        document.getElementById('calendar').appendChild(newDiv);
    });

};

myFunc(myArray);
#calendar {
    height: 720px;
    width: 100%;
    position: relative;
}
#calendar div {
    position: absolute;
    width: 100%;
    min-height: 5px;
    background: brown;
}
#calendar .split {
    width: 50%;
    background: pink;
}
.right {
    right: 0;
}
.left {
    left: 0;
}
<div id="calendar"></div>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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