3
\$\begingroup\$

// TODO: https://codereview.stackexchange.com/questions/240011/js-schedule-conflict-detection-algorithm-logic-length

// List with handle
Sortable.create(dashboard, {
  handle: '.fa-bars',
  animation: 150
});

// To Instantiate & Control the Modal 
var modal = document.getElementById("modal");
var addSchedule = document.getElementById('add');
var scheduleCount = 0;
var trackedSchedule;

addSchedule.addEventListener('click', function (event) {
  scheduleCount += 1;
  // console.log(event.target.id); // Anomoly Here
  trackedSchedule = `schedule-${scheduleCount}`;
  console.log(trackedSchedule);
  // Change modal to load with default values (prefill name with scheduleCount)
  // Change Text to Read "Create Schedule" instead of "Edit Schedule"
  modal.style.display = "block";  
});


var span = document.getElementsByClassName("close")[0];
span.onclick = function() {
  modal.style.display = "none";
  scheduleCount -= 1;
}

window.onclick = function(event) {
  if (event.target == modal) {
    modal.style.display = "none";
    scheduleCount -= 1;
  }
}

function getDate() {
  var today = new Date();
  var dd = String(today.getDate()).padStart(2, '0');
  var mm = String(today.getMonth() + 1).padStart(2, '0');
  var yyyy = today.getFullYear();
  today = `${yyyy}-${mm}-${dd}`;
  return today;
}

var savedSchedules = {};
var data = [];

var layout = {
  showlegend: false,
  xaxis: {range: ['2020-01-01 00:00:00', '2020-01-01 23:59:59'],
          showgrid: false,
          zeroline: false,
          showline: true,
          tickformat: '%H:%M:%S'
         },
  yaxis: {rangemode: 'tozero',
          range: [-0.75, 6.5],
          showline: true,
          zeroline: false,
          tickvals: [0, 1, 2, 3, 4, 5, 6],
          ticktext: ['Sunday', 'Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday']
         }
};

// To Save the Schedule Data from the Modal
var scheduleName = document.getElementById('schedule-name');
var warningMsg = document.getElementById('warning');
var saveButton = document.getElementById('submit-schedule');
var overwriteButton = document.getElementById('overwrite-schedule');
var sunday = document.getElementById('Sunday');
var monday = document.getElementById('Monday');
var tuesday = document.getElementById('Tuesday');
var wednesday = document.getElementById('Wednesday');
var thursday = document.getElementById('Thursday');
var friday = document.getElementById('Friday');
var saturday = document.getElementById('Saturday');
var daysOfWeek = {'Sunday': sunday,
                  'Monday': monday,
                  'Tuesday': tuesday,
                  'Wednesday': wednesday,
                  'Thursday': thursday,
                  'Friday': friday,
                  'Saturday': saturday}
var startTime;
var endTime;

document.getElementById('startTime').addEventListener("input", function (event) {
      startTime = event.target.value;
});

document.getElementById('endTime').addEventListener("input", function (event) {
      endTime = event.target.value;
});


function editSchedule(event) {
  console.log(event.target.id);
  // Init Modal from saved dict
  schedule = savedSchedules[event.target.id];
  Object.entries(schedule).forEach(function([key, value]) {
    if (value) {
      if (key != 'Name') {
         var day = document.getElementById(key);
         day.checked = true;
      }
      else {
        scheduleName.value = value;
      }
      var startTime_ = document.getElementById('startTime');
      var endTime_ = document.getElementById('endTime');
      startTime_.value = value.StartTime;
      endTime_.value = value.EndTime;
    }
  });
  modal.style.display = 'block';
};

// Need to validate logical comparators for provided time format?
function checkForConflict(c1s, c1e, c2s, c2e) { // case 1 start time, case 1 end time, case 2 start time, case 2 end time
  // if (c1 intersects c2) or (c2 intersects c1) or (c2 contained in c1) or (c1 contained in c2) or (anything else?)
  if ( (c1s >= c2s && c1e >= c2s) || (c1s <= c2s && c1e <= c2e) || (c1s <= c2s && c1e >= c2e) || (c1s >= c2s && c1e <= c2e) ) {
    return true;
  } else {return false};
}

saveButton.onclick = function() {
  // Disable save if not all reqd fields filled -> https://stackoverflow.com/questions/39880389/disable-button-until-fields-are-full-pure-js
  // while (!scheduleName || !startTime || !endTime || !(sunday.checked&&monday.checked&&tuesday.checked&&wednesday.checked&&thursday.checked&&friday.checked&&saturday.checked)) {
  //     saveButton.disabled = true;
  //   } 
  
  // Check that fields are filled
  if (!scheduleName || !startTime || !endTime || !sunday.checked&&!monday.checked&&!tuesday.checked&&!wednesday.checked&&!thursday.checked&&!friday.checked&&!saturday.checked) {
    warningMsg.textContent = "All required fields must contain data";
  } 
  
  // Try to Save the Schedule
  else {           
    // Check for Schedule Conflicts
    var conflictData;
    if (savedSchedules && Object.keys(savedSchedules).length >= 1) {
        console.log("At least one schedule detected");
        // Loop through saved schedules (key = 'schedule-x')
        Object.entries(savedSchedules).forEach(function([key, value]) {
          schedule_ = savedSchedules[key];
          // Loop through the schedule (key_ = 'Name', 'Sunday', ...)
          Object.entries(schedule_).forEach(function([key_, value_]) {
            // Only check if weekday has a time value and weekdays that are selected
            if (key_ != 'Name' && value_ && daysOfWeek[key_].checked) { 
                // if there is conflict -> store to variable
                var conflict = checkForConflict(value_.StartTime, value_.EndTime, startTime, endTime);
                if (conflict) {
                  console.log('Conflict Detected');
                  conflictData = [key_, `Start: ${value_.StartTime}/${startTime}`, `End: ${value_.EndTime}/${endTime}`];
                }
            }
          });
        });
    }
    // Alert User of Conflict
    if (conflictData) {
      console.log(conflictData);
      warningMsg.textContent = `There is a scheduling conflict ${conflictData}`;
      overwriteButton.style.display = 'block';
      // Handle Conflict Resolution -> Overwrite Schedule, etc
    } 
    
    // Save the Schedule
    else {
      var dict = {
        Name: scheduleName.value,
        Sunday: sunday.checked && startTime && endTime ? {StartTime: startTime, EndTime: endTime} : null,  
        Monday: monday.checked && startTime && endTime ? {StartTime: startTime, EndTime: endTime} : null,
        Tuesday: tuesday.checked && startTime && endTime ? {StartTime: startTime, EndTime: endTime} : null,
        Wednesday: wednesday.checked && startTime && endTime ? {StartTime: startTime, EndTime: endTime} : null,
        Thursday: thursday.checked && startTime && endTime ? {StartTime: startTime, EndTime: endTime} : null,
        Friday: friday.checked && startTime && endTime ? {StartTime: startTime, EndTime: endTime} : null,
        Saturday: saturday.checked && startTime && endTime ? {StartTime: startTime, EndTime: endTime} : null,
      }

      savedSchedules[trackedSchedule] = dict;
      console.log(savedSchedules);

      // Add to the graph
      var randomColor = Math.floor(Math.random()*16777215).toString(16); // Make color indicative of temperature
      Object.entries(dict).forEach(function([key, value]) {
        if (key != 'Name' && value) {

          data.push({
            x: [`${getDate()} ${value.StartTime}`, `${getDate()} ${value.EndTime}`], 
            y: [key, key], 
            hovertext: dict.Name,
            type: 'scatter',
            mode: 'markers',
            marker: {
              size: 30,
              color: [randomColor, randomColor],
            }
          });

          data.push({
            x: [`${getDate()} ${value.StartTime}`, `${getDate()} ${value.EndTime}`], 
            y: [key, key], 
            hovertext: scheduleName.value,
            type: 'scatter',
            mode: "lines",
            line: {
              width: 30,
              color: randomColor,
            },
          });

        };
      });

      Plotly.newPlot('graph', data, layout);


      // Create the Dashboard Entry
      var newLine = document.createElement('div');
      newLine.classList.add('list-group-item');
      var newButton = document.createElement('div');
      newButton.classList.add('add-button');
      newButton.id = trackedSchedule;
      newButton.addEventListener('click', editSchedule);    
      var newTemp = document.createElement('a');
      var newName = document.createElement('a');
      newName.textContent = scheduleName.value;
      newName.classList.add('name-font');
      var newWeekSet = document.createElement('div');
      newWeekSet.classList.add('right-align');
      Object.entries(dict).forEach(function([key, value]) {
        if (key != 'Name') {
           var newDay = document.createElement('label')
           // newDay.classList.add('day-font');
           newDay.style.display = 'inline-block';
           newDay.style.margin = '5px';
           newDay.style.fontStyle = 'italic';
           var newDayIcon;
           newDay.textContent = key.charAt(0);
           if (value) {
             newDayIcon = document.createElement('i');
             newDayIcon.classList.add('far', 'fa-check-circle', 'fa-xs');
             // Why doesn't class work for this?
             newDayIcon.style.display = 'block';
             newDayIcon.style.color = 'Blue';
           } 
           else {
             newDayIcon = document.createElement('i');
             newDayIcon.classList.add('far', 'fa-times-circle', 'fa-xs');
             // Why doesn't class work for this?
             newDayIcon.style.display = 'block';
             newDayIcon.style.color = 'Orange';
           }
           newDay.append(newDayIcon);
           newWeekSet.appendChild(newDay);
        };
      });
      var newMoveIcon = document.createElement('i');
      newMoveIcon.classList.add('fas', 'fa-bars');
      newMoveIcon.style.display = 'inline-block';
      newMoveIcon.style.paddingTop = '10px';
      newMoveIcon.style.paddingLeft = '20px'

      newLine.appendChild(newButton);
      newLine.appendChild(newName);
      newLine.appendChild(newWeekSet);
      newWeekSet.appendChild(newMoveIcon);
      dashboard.appendChild(newLine);

      modal.style.display = "none";
    }
  }
}
@import url(https://fonts.googleapis.com/css?family=Open+Sans:700,300);
 body {
	 background: white;
	 font-family: 'Open Sans', Helvetica, sans-serif;
	 -webkit-font-smoothing: antialiased;
	 -moz-osx-font-smoothing: grayscale;
}

.add-button {
  display: inline-block;
  height: 50px;
  width: 50px;
  color: orange;
  padding-left: 18px;
  padding-top: 10px;
  background: white;
  border: 2px solid orange;
  border-radius: 25px;
  cursor: pointer;
}

.name-font {
  font-size: 2em;
  padding-left: 35px;
}

.submit-button {
  display: inline-block;
  border: 1px solid orange;
  border-radius: 5px;
  height: 40px;
  background-color: white;
  cursor: pointer;
}

.submit-button:hover {
  background-color: #ffd796
}

.clickable {
  cursor: pointer;
}

.right-align {
  float: right;
}

/* The Modal (background) */
.modal {
  position: fixed;
  display: none;
  padding-left: 75%;
  left: 0;
  top: 0;
  width: 100%; 
  height: 100%; 
  overflow: auto; 
  background-color: rgba(255,255,255,0.5); 
}

/* Modal Content */
.modal-content {
  padding: 20px;
  border: 2px solid orange;
  height: 100%;
  overflow: auto;
}

/* The Close Button */
.close {
  display: inline;
  color: gray;
  float: right;
}

.close:hover,
.close:focus {
  color: #000;
  text-decoration: none;
  cursor: pointer;
}

/* Checkboxes */
.form-group {
  display: block;
  margin-bottom: 5px;
}

.form-group input {
  padding: 0;
  height: initial;
  width: initial;
  margin-bottom: 0;
  display: none;
  cursor: pointer;
}

.form-group label {
  position: relative;
  cursor: pointer;
}

.form-group label:before {
  content:'';
  -webkit-appearance: none;
  background-color: transparent;
  border: 2px solid orange;
  box-shadow: 0 1px 2px rgba(0, 0, 0, 0.05), inset 0px -15px 10px -12px rgba(0, 0, 0, 0.05);
  padding: 10px;
  display: inline-block;
  position: relative;
  vertical-align: middle;
  cursor: pointer;
  margin-right: 5px;
}

.form-group input:checked + label:after {
  content: '';
  display: block;
  position: absolute;
  top: 4px;
  left: 9px;
  width: 6px;
  height: 14px;
  border: solid orange;
  border-width: 0 2px 2px 0;
  transform: rotate(45deg);
}

/* Clocklet */
.labels {
  display: block;
}

 .clock {
  display: block;
  z-index: 10000;
}

 /* minute - dial, hand, selected tick, hovered tick */
.clocklet-color-example .clocklet-plate {
  box-shadow: 1px 2px 4px 0 rgba(0, 0, 0, 0.2);
  border: 1px solid gray;
}
.clocklet-color-example[data-clocklet-placement="bottom"] {
  background-color: transparent;
  border: none;
  box-shadow: none;
}
  .clocklet-color-example .clocklet-dial--minute { background-color: orange; color: white;}
  .clocklet-color-example .clocklet-hand--minute { background-color: white; border: 1px solid gray; width: 3px;}
  .clocklet-color-example .clocklet-tick--minute.clocklet-tick--selected { background-color: white; color: orange;}
  .clocklet-color-example.clocklet--hoverable:not(.clocklet--dragging) .clocklet-tick--minute:hover { background-color: white; color: orange;}

  /* hour - dial, hand, selected tick, hovered tick */
  .clocklet-color-example .clocklet-dial--hour { background-color: white; }
  .clocklet-color-example .clocklet-hand--hour { background-color: white; border: 1px solid gray; width: 3px;}
  .clocklet-color-example .clocklet-tick--hour.clocklet-tick--selected { background-color: orange; }
  .clocklet-color-example.clocklet--hoverable:not(.clocklet--dragging) .clocklet-tick--hour:hover { background-color: orange; }

  /* hand origin */
  .clocklet-color-example .clocklet-hand-origin { background-color: gray; }

  /* ampm */
  .clocklet-color-example .clocklet-ampm::before { background-color: orange; }
  .clocklet-color-example .clocklet-ampm:hover::before { background-color: white; color: orange; border: 1px solid orange;}
  .clocklet-color-example .clocklet-ampm[data-clocklet-ampm="pm"]::before { background-color: orange; }
  .clocklet-color-example .clocklet-ampm[data-clocklet-ampm="pm"]:hover::before { background-color: white; }
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0/css/bootstrap.min.css" integrity="sha384-Gn5384xqQ1aoWXA+058RXPxPg6fy4IWvTNh0E263XmFcJlSAwiGgFAW/dAiS6JXm" crossorigin="anonymous">

    <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.4.1/jquery.min.js"></script>

<script src="https://kit.fontawesome.com/6873aa3c17.js" crossorigin="anonymous"></script>

  <!-- Sortable -->
<script src="https://cdnjs.cloudflare.com/ajax/libs/Sortable/1.10.1/Sortable.min.js"></script>

<!-- Plotly -->
<script src="https://cdn.plot.ly/plotly-latest.min.js"></script>

<!-- Clocklet -->
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/clocklet@0.2.4/css/clocklet.min.css">
<script src="https://cdn.jsdelivr.net/npm/clocklet@0.2.4"></script>

  <!-- List with handle -->
<div id="dashboard" class="list-group">
    
    <div class="list-group-item">
      <div id='add' class='add-button'><i class="fas fa-plus fa-xs"></i></div>
      <a style="font-size: 1em; padding-left: 35px">Add Schedule</a>
    </div>
  
</div>

<div id="modal" class="modal">
  <div class="modal-content">
    <div>
      <h1 style='display: inline'>Edit Schedule</h1>
      <div class="close"><i class="fas fa-times"></i></div>
    </div>
    
    <form>
      <input id='schedule-name' type='text' value='Test'></input>
      <label class='labels' for='startTime'>Start Time</label>
      <input id='startTime' class="clock" value="12:34 A.M."
             data-clocklet="class-name: clocklet-color-example; format: h:mm AA; alignment: right; placement: bottom;"
             placeholder="h:mm AA">
      <label class='labels' for='endTime'>End Time</label>
      <input id='endTime' class="clock" value="1:35 P.M."
             data-clocklet="class-name: clocklet-color-example; format: h:mm AA; alignment: right; placement: bottom;"
             placeholder="h:mm AA">
      
      <br><br>
      <div class="form-group">
        <input type="checkbox" id="Sunday" checked='true'>
        <label for="Sunday">Sunday</label>
      </div>
      <div class="form-group">
        <input type="checkbox" id="Monday">
        <label for="Monday">Monday</label>
      </div>
      <div class="form-group">
        <input type="checkbox" id="Tuesday">
        <label for="Tuesday">Tuesday</label>
      </div>
      <div class="form-group">
        <input type="checkbox" id="Wednesday">
        <label for="Wednesday">Wednesday</label>
      </div>
      <div class="form-group">
        <input type="checkbox" id="Thursday">
        <label for="Thursday">Thursday</label>
      </div>
      <div class="form-group">
        <input type="checkbox" id="Friday">
        <label for="Friday">Friday</label>
      </div>
      <div class="form-group">
        <input type="checkbox" id="Saturday">
        <label for="Saturday">Saturday</label>
      </div>
    </form>
    
    <!--  Add temperature selector => slider? -->
    <!--  Add ability to select room/zones for schedule -->
    <a id='warning' style='color: Red'></a>
    <button id='submit-schedule' class="submit-button">Create Schedule</button>
  <button id='overwrite-schedule' class="submit-buttom" style='display: none'>Overwrite Schedule</button>
    
  </div>
</div>

<div id='graph'></div>

First post here - I hope I understand the purpose/intent of the community. I am trying to create a temperature scheduler for HVAC management WebUI using Js (although you wont find anything in the code currently about setting the temperature). The purpose of the code review is associated with the scheduling conflict algorithm & code. I.e. at no point should there be a duplicate temperature setting for a given day of the week or time (or room - to come later).

I am hoping to see if there are any suggestions for improving my schedule conflict detecting algorithm - I'm not entirely happy with it since the logic is difficult to read in my opinion. Find code (lots of omissions so if something is unclear let me know) and CodePen link (with no omissions) below - thanks in advance for any answers!


var scheduleName = document.getElementById('schedule-name');
var warningMsg = document.getElementById('warning');
var saveButton = document.getElementById('submit-schedule');
var sunday = document.getElementById('Sunday'); // checkbox
var monday = document.getElementById('Monday'); // checkbox
var tuesday = document.getElementById('Tuesday'); // checkbox
var wednesday = document.getElementById('Wednesday'); // checkbox
var thursday = document.getElementById('Thursday'); // checkbox
var friday = document.getElementById('Friday'); // checkbox
var saturday = document.getElementById('Saturday'); // checkbox
var daysOfWeek = {'Sunday': sunday,
                  'Monday': monday,
                  'Tuesday': tuesday,
                  'Wednesday': wednesday,
                  'Thursday': thursday,
                  'Friday': friday,
                  'Saturday': saturday}

function checkForConflict(c1s, c1e, c2s, c2e) { // case 1 start time, case 1 end time, case 2 start time, case 2 end time
  // if (c1 intersects c2) or (c2 intersects c1) or (c2 contained in c1) or (c1 contained in c2) or (anything else?)
  if ( (c1s >= c2s && c1e >= c2s) || (c1s <= c2s && c1e <= c2e) || (c1s <= c2s && c1e >= c2e) || (c1s >= c2s && c1e <= c2e) ) {
    return true;
  } else {return false};
}

saveButton.onclick = function() {

  // Check that fields are filled
  if (!scheduleName || !startTime || !endTime || !sunday.checked&&!monday.checked&&!tuesday.checked&&!wednesday.checked&&!thursday.checked&&!friday.checked&&!saturday.checked) {
    warningMsg.textContent = "All required fields must contain data";
  } 

  // Try to Save the Schedule
  else {           
    // Check for Schedule Conflicts
    var conflictData;
    if (savedSchedules && Object.keys(savedSchedules).length >= 1) {
        console.log("At least one schedule detected");
        // Loop through saved schedules (key = 'schedule-x')
        Object.entries(savedSchedules).forEach(function([key, value]) {
          schedule_ = savedSchedules[key];
          // Loop through the schedule (key_ = 'Name', 'Sunday', ...)
          Object.entries(schedule_).forEach(function([key_, value_]) {
            // Only check if weekday has a time value and weekdays that are selected
            if (key_ != 'Name' && value_ && daysOfWeek[key_].checked) { 
                // if there is conflict -> store to variable
                var conflict = checkForConflict(value_.StartTime, value_.EndTime, startTime, endTime);
                if (conflict) {
                  console.log('Conflict Detected');
                  conflictData = [key_, `Start: ${value_.StartTime}/${startTime}`, `End: ${value_.EndTime}/${endTime}`];
                }
            }
          });
        });
    }
    // Alert User of Conflict
    if (conflictData) {
      console.log(conflictData);
      warningMsg.textContent = `There is a scheduling conflict ${conflictData}:  - Cannot proceed...`;
      // Handle Conflict Resolution -> Overwrite Schedule, etc
    } 

    // Save the Schedule
    else {...}

The format of savedSchedules looks like this:

{
  schedule-1: {
    Name: <Some String>,
    Sunday: null,
    Monday: null,
    Tuesday: null,
    Wednesday: null,
    Thursday: null, 
    Friday: null,
    Saturday: {StartTime: 12:55 A.M., EndTime: 1:35 P.M.}
  },
  schedule-2: {...},
  ...
}

To me, the // Check for Schedule Conflicts block is pretty rough in terms of being concise both in logic and in length. I would appreciate a second set of more experienced eyes that might confirm that this is pretty sound logic/structure or provide a much better alternative.

Link to CodePen

\$\endgroup\$
  • \$\begingroup\$ Please can you add more description on what this is achieving. Currently it's unclear from the text what this is doing. Are you scheduling flights, a holiday, time out with friends, business meetings... How do the schedules interact with each other? \$\endgroup\$ – Peilonrayz Apr 6 at 20:46
  • 1
    \$\begingroup\$ @Graipher I added full code in snippet \$\endgroup\$ – Sterling Butters Apr 6 at 21:17
  • 1
    \$\begingroup\$ @Peilonrayz I added a little more to description in edit, let me know if still unclear \$\endgroup\$ – Sterling Butters Apr 6 at 21:17
  • \$\begingroup\$ It seems more clear. Whether others agree and don't add more close votes / retract existing ones I can't say. \$\endgroup\$ – Peilonrayz Apr 6 at 21:48
  • \$\begingroup\$ @Peilonrayz Thanks, I care less about my rep here thankfully - was just happy to get a good answer \$\endgroup\$ – Sterling Butters Apr 6 at 22:44
3
\$\begingroup\$

Your retrieval of checkboxes is a bit verbose. Consider using a single selector to get all of them instead, instead of having to select each one individually.

const daysOfWeek = {};
for (const input of document.querySelectorAll('.form-group > input[type="checkbox"]') {
  daysOfWeek[input.id] = input;
}

Then, rather than examining each separate variable to see if it's checked, iterate over that object to see if any values are checked.

To avoid indentation hell, consider returning early when an error is encountered, rather than a very large else block when there isn't an error.

Your savedSchedules is an object which is never reassigned:

var savedSchedules = {};

So it'll always be truthy - the if (savedSchedules && check is superfluous, since it'll always be true.

There's also no need to check how many keys the object has beforehand - just iterate through them all regardless. If there aren't any, then no conflicts will be detected. So the Object.keys(savedSchedules).length >= 1 can be removed.

Object.entries's callback accepts an entry array parameter of the key and the value. Since you already have the value as a variable, there's no need to select it again by going through the [key]:

Object.entries(savedSchedules).forEach(function([key, value]) {
  schedule_ = savedSchedules[key];

simplifies to

Object.entries(savedSchedules).forEach(function([key, schedule_]) {

But since you aren't actually using the schedule name (the key), you may as well remove it and use Object.values instead:

Object.values(savedSchedules).forEach(function(schedule_) {

But since you're trying to find whether there are any conflicts, it would be more appropriate to short-circuit when a problem is found. Consider using a for loop instead. When a problem is found, tell the user about it and return.

The underscores after the variable names are confusing - they don't match a convention I know of. It looks like they're meant to distinguish a single schedule's object from the day and object value for the day. I think it would be appropriate to note this in the variable names explicitly, eg schedule, day, and dayObj.

Also, since you're using Object.entries, your environment supports ES6 - in ES6, you should always use const to declare variables when possible, never var (and let only when you have to reassign).

The startTime variable name (which comes from the input) is a bit unclear once you're deep into the verification stage and are comparing against value_.StartTime variable name. Maybe call the input variable inputStartTime (and the same for endTime) to reduce possible confusion?

The conflict checker is a decent chunk of code that probably deserves to be in its own function. This allows you to see a decent overview of the whole click process from the click listener without having to scroll through large amounts of code to get a general idea.

Assigning to onclick should usually be avoided. It can work if done once, but if any other code follows the same bad practices and ever tries to do the same thing, your prior listener will be overwritten. Best to always use addEventListener instead.

In full:

const daysOfWeek = {};
for (const input of document.querySelectorAll('.form-group > input[type="checkbox"]')) {
    daysOfWeek[input.id] = input;
}
const getConflicts = () => {
    for (const schedule of Object.values(savedSchedules)) {
        // Loop through the schedule (key_ = 'Name', 'Sunday', ...)
        for (const [day, dayObj] of Object.entries(schedule)) {
            // Only check if weekday has a time value and weekdays that are selected
            if (day === 'Name' || !dayObj || !daysOfWeek[day].checked) {
                continue;
            }
            // if there is conflict, return it
            const conflict = checkForConflict(dayObj.StartTime, dayObj.EndTime, inputStartTime, inputEndTime);
            if (conflict) {
                console.log('Conflict Detected');
                const conflictData = [day, `Start: ${dayObj.StartTime}/${inputStartTime}`, `End: ${dayObj.EndTime}/${inputEndTime}`];
                return `There is a scheduling conflict ${conflictData}:  - Cannot proceed...`;
            }
        }
    }
};
saveButton.addEventListener('click', () => {
    // Check that fields are filled
    if (!scheduleName || !inputStartTime || !inputEndTime || Object.values(daysOfWeek).every(input => !input.checked)) {
        warningMsg.textContent = 'All required fields must contain data';
        return;
    }
    // Check for Schedule Conflicts
    const conflictsMessage = getConflicts();
    if (conflictsMessage) {
        // Handle Conflict Resolution -> Overwrite Schedule, etc
        // Probably CALL A FUNCTION HERE, don't write it all inside this click listener
        return;
    }
    // Save the Schedule

    // ...
});

The generation of the schedule object isn't shown, but it would be great not to have to do the if (day === 'Name' check - rather than combining the schedule name with the day keys, consider putting the days into a completely separate property, eg:

{
  scheduleName: 'schedule-1',
  days: {
    Sunday: ...
    Monday: ...
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Wow thank you for such a detailed answer! Definitely the kind of input I was hoping to find on this forum. Very very helpful - thanks again! \$\endgroup\$ – Sterling Butters Apr 6 at 0:54

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.