Javascript Object Oriented Planner App

Question

I am creating a planner/scheduling app using objects. I am just a beginner with object oriented programming, so any tips on how to better structure my classes would be very helpful.

---

Program Basics

The program consists of "objectives" and "checkpoints." An objective is essentially a goal, and checkpoints are smaller steps that must be completed to reach that goal. Objectives and checkpoints all have "deadlines" (due dates) and are part of the calendar object. Eventually, I'd like to make a calendar display that shows the objectives and checkpoints.

---

Code

// GUID generator
function uuidv4() {
 return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function(c) {
 var r = Math.random() * 16 | 0, v = c == 'x' ? r : (r & 0x3 | 0x8);
 return v.toString(16);
 });
}
// Adds add day feature to Date prototype
Date.prototype.addDays = function(days) {
 var date = new Date(this.valueOf());
 date.setDate(date.getDate() + days);
 return date;
}
// Parent object for all data
var calendar = {
 // Contains all objectives
 objectives: new Array(),
 // Checkpoints must be completed in order?
 strict: true,
 // Create objective, add to list, and sort list by deadline
 addObjective: function(name, deadline, description) {
 this.objectives.push(new Objective(name, deadline, description));
 this.sortObjectives();
 },
 getObjectives: function() {
 return this.objectives;
 },
 setStrict: function(strict) {
 this.strict = strict;
 },
 // Sort objectives by deadline, most recent last
 sortObjectives() {
 this.objectives.sort((a, b) => (a.deadline > b.deadline) ? 1 : -1);
 }
}
class Objective {
 constructor(name, deadline, description) {
 this.name = name;
 this.deadline = new Date(deadline);
 this.description = description;
 this.checkpoints = new Array();
 this.status = false;
 this.id = uuidv4();
 }
 // Push back deadline
 moveDeadline(days=0, moveAll=false) {
 this.deadline = this.deadline.addDays(days)
 // Move all checkpoints after current date unless incomplete
 if (moveAll) {
 let today = new Date();
 for (let i = 0; i < this.checkpoints.length; i++) {
 if (this.checkpoints[i].deadline < today || !this.checkpoints[i].status) {
 this.checkpoints[i].deadline.addDays(days);
 }
 }
 }
 }
 // Remove objective from calendar object
 delete() {
 calendar.objectives.splice(calendar.objectives.indexOf(this), 1);
 }
 // Create checkpoint, add to list, sort order by deadline
 addCheckpoint(name, deadline, description) {
 this.checkpoints.push(new Checkpoint(name, deadline, description, this.id));
 this.sortCheckpoints();
 }
 getName() {
 return this.name;
 }
 setName(name) {
 this.name = name;
 }
 getDeadline() {
 return this.deadline.toDateString();
 }
 setDeadline(deadline) {
 this.deadline = new Date(deadline);
 }
 getDescription() {
 return this.description;
 }
 setDescription(description) {
 this.description = description;
 }
 getStatus() {
 return this.status;
 }
 // Checks whether all checkpoints are complete and determines status, invoked by Checkpoint.prototype.setStatus()
 setStatus() {
 let checkpoints = this.getCheckpoints();
 let allComplete = true;
 // Iterates through checkpoints
 for (let i = 0; i < checkpoints.length; i++) {
 // If at least one checkpoint is incomplete, objective status is false
 if (!checkpoints[i].status) {
 allComplete = false;
 break;
 }
 }
 if (allComplete) {
 this.status = true;
 }
 else {
 this.status = false;
 }
 }
 getCheckpoints() {
 return this.checkpoints;
 }
 getid() {
 return this.id;
 }
 // Sort checkpoints by deadline, most recent last
 sortCheckpoints() {
 this.checkpoints.sort((a, b) => (a.deadline > b.deadline) ? 1 : -1);
 }
}
class Checkpoint {
 constructor(name, deadline, description, id) {
 this.name = name;
 this.deadline = new Date(deadline);
 this.description = description;
 this.status = false;
 this.id = uuidv4();
 this.objectiveid = id;
 }
 // Push back deadline
 moveDeadline(days=0, moveAll=false) {
 this.deadline = this.deadline.addDays(days)
 // Move all checkpoints after this checkpoint deadline
 if (moveAll) {
 for (let i = 0; i < this.getObjective().getCheckpoints().length; i++) {
 if (this.getObjective().getCheckpoints()[i].deadline <= this.deadline && this.getObjective().getCheckpoints()[i] != this) {
 this.getObjective().getCheckpoints()[i].deadline.addDays(days);
 }
 }
 }
 }
 // Remove checkpoint from objective object
 delete() {
 let objective = this.getObjective();
 objective.checkpoints.splice(objective.checkpoints.indexOf(this), 1);
 }
 getName() {
 return this.name;
 }
 setName(name) {
 this.name = name;
 }
 getDeadline() {
 return this.deadline.toDateString();
 }
 setDeadline(deadline) {
 this.deadline = new Date(deadline);
 }
 getDescription() {
 return this.description;
 }
 setDescription(description) {
 this.description = description;
 }
 getStatus() {
 return this.status;
 }
 // Update checkpoint status
 setStatus(status) {
 if (status == true) {
 if (calendar.strict) {
 let previousCheckpoints = this.getObjective().getCheckpoints().slice(0, this.getObjective().getCheckpoints().indexOf(this));
 let strictCondition = true;
 // Checks if all preceding checkpoints are completed if "strict" progress is enabled
 for (let i = 0; i < previousCheckpoints.length; i++) {
 if (!previousCheckpoints[i].status) {
 strictCondition = false;
 break;
 }
 }
 if (strictCondition) {
 this.status = true;
 }
 else {
 console.log('must complete preceding checkpoints');
 }
 }
 else {
 this.status = true;
 }
 }
 // No conditions for reverting checkpoint to incomplete
 else if (status == false) {
 this.status = false;
 }
 // Check objective status
 this.getObjective().setStatus();
 }
 getid() {
 return this.id;
 }
 getObjectiveid() {
 return this.objectiveid;
 }
 // Return reference to parent objective object
 getObjective() {
 for (let i = 0; i < calendar.objectives.length; i++) {
 let objective = calendar.objectives[i]
 if (objective.getid() == this.objectiveid) {
 return(objective);
 }
 }
 }
}
```

• Sᴀᴍ Onᴇᴌᴀ: Welcome to Code Review! Would you please edit your post to provide sample usage code? I could utilize the API of the code but am curious what your recommendations would be...

Answer 1

Class Structure

Objectives and checkpoints [...] are part of the calendar object.

The given description would result in the following (simplified) UML-Diagram, where the Calendar is an aggregate of Objective and Checkpoint:

enter image description here

But the code would give us the following (simplified) UML-Diagram, where the Calendar knows Objective and Checkpoint and vise versa:

enter image description here

The bidirectional relationship allows the use of non-intuitive and inconsistent methods. The method of the delete is in Objective and Checkpoint but the add is in Calendar.

Adding and deleting should be in the aggregate, the Calendar, as it is the most intuitive.

Flag Arguments

Flag arguments are ugly. Passing a boolean into a function is a truly terrible practice. It immediately complicates the signature of the method, loudly proclaiming that this function does more than one thing. It does one thing if the flag is true and another if the flag is false!

— Clean Code, Page 41, Robert C. Martin

We can split the moveDeadline methods in Objective and Checkpoint into two methods. This would result into two methods that are simpler to read and use, because they get smaller and we lose the if-statement:

moveDeadlineBy(days=0) {
 this.deadline = this.deadline.addDays(days)
}
moveAllDeadlinesBy(days=0) {
 let today = new Date();
 for (let i = 0; i < this.checkpoints.length; i++) {
 if (this.checkpoints[i].deadline < today || !this.checkpoints[i].status) {
 this.checkpoints[i].deadline.addDays(days);
 }
 }
}

The Object itself as an Argument

addObjective: function (name, deadline, description) {
 this.objectives.push(new Objective(name, deadline, description));
 this.sortObjectives();
}

Instead of passing in every little argument that Objective's constructor uses, we can pass in the Objective's object itself. If the constructor of Objective is changed, those changes need not be carried over to this method. Because beforehand we would have had to change all additional or omitted arguments of the constructor with this method as well.

addObjective(objective) {
 this.objectives.push(objective)
 this.sortObjectives()
}

Law of Demeter

Each unit should only talk to its friends; don't talk to strangers.

— https://en.wikipedia.org/wiki/Law_of_Demeter

Chained calls like the following violate the Law of Demeter:

• calendar.objectives.splice(/*...*/)
• calendar.objectives.indexOf(/*...*/)
• calendar.objectives.length

Beceause calendar is the "frind" of the object that calls calendar but the caller calls methods of the "frinds" of calendar like. Instead we could implement methods on calendar that "hides" the "friends":

• calendar.deleteBy(/*...*/)
• calendar.getBy(/*...*/)
• calendar.size()

This principle based on the idea of Encapsulation in Object Oriented Programming, where an object should hide the internals to the outer world and makes them only accessible by interacting with the object itself instead with the internals.

Alt-Text

Class Structure, Again

I would suggest a class structure that could look like the following UML-Diagram

dsfs

This would look in code like he following

class Calendar {
 constructor() { 
 this.objectives = {}
 }
 add(objective) { 
 this.objectives[objective.id] = objective
 }
 deleteBy(id) { /* ... */ } 
 getBy(id) { 
 return this.objectives[id]
 }
 getAll() { /* ... */ }
 size() { /* ... */ }
 sort() { /* ... */ }
 setStrict(strict) { /* ... */ }
}
class Objective {
 constructor(name, deadline, description) { /* ...*/ }
 add(checkpoint) { /* ... */ }
 getCheckpointBy(id) { /* ... */ }
 moveDeadlineBy(days=0) { /* ... */ }
 moveAllDeadlinesBy(days=0) { /* ... */ }
 // getter, setter if needed
}
class Checkpoint {
 constructor(name, deadline, description, id) { /* ... */ }
 fulfilled() { /* ... */ }
 unfulfilled() { /* ... */ }
 // getter, setter if needed
}

const calendar = new Calendar()
calendar.add(new Objective(/* ... */))
const objectives = calendar.getAll()
const objective = objectives[0]
objective.add(new Checkpoint(/* ... */))
objective.moveDeadlineBy(5)
const checkpoint = objective.getBy(/* id */)
checkpoint.fulfilled()

• Morrison Bower: Thank you! What would the code for the getBy function look like?
• Roman: @MorrisonBower I added the implementation - fell free to ask additional questions :)
• Morrison Bower: When I run the fulfilled method on Checkpoint, is there a way to call a method on the parent objective object (that will check if all checkpoints have been fulfilled)? I know that Checkpoint isn't supposed to be able to access Objective, so how would I do this?
• Roman: @MorrisonBower, you could add a Method fullfillBy(id) to Objective which could call the fullfilled method of a Checkpoint and do additional things.

Answer 2

I notice that you have many member functions entitled getX or setX. It would be much more idiomatic in JavaScript to use getters and setters for this purpose.

e.g. instead of getObjective

get objective() {
 for (const objective of calendar.objectives) {
 if (objective.id === this.objectiveid) {
 return objective;
 }
 }
}

Edit: A few other changes I'd make to your code, as seen in the snippet above, are replacing C-style for loops with for...of loops, preferring const to let and let to var, and (important!) replacing == with ===.