Refactoring using Predicates

The code below was a first stab in thinking what needed to be done. We did follow single responsibility patterns.

However if we look at the code patterns we see repeated code patterns. This violates the DRY principle, if we apply it to repeated patterns.

Repeated Coding Patterns?

Refactoring teaches us to see coding patterns early. See any repeated patterns here?

  saveNewRelatedPeople() {
   let newOnes = this.relatedPeople.filter((rp1) => rp1.isNew);
   newOnes.forEach((rp2) => {
     this.putRelatedPerson(rp2);
   });
   return newOnes;
  }

  saveExistingRelatedPeople() {
   let notNew = this.relatedPeople.filter((item) => !item.isNew);
   notNew.forEach((rp) => {
     this.rs.putRelatedPerson(rp);
   });
  }
  setIsNewToFalse() {
   if (this.saveIsValid) {
     let newOnes = this.saveNewRelatedPeople();
     this.saveExistingRelatedPeople();
     newOnes.forEach((rp) => (rp.isNew = false));
   }
  }

If we train ourselves to look at repeated code patterns then we are dialing into a more generic and better way of programming.

The Predicate

A predicate is a function that returns true or false.

// this code
 function saveRelatedPeople(predicate) {
   let items = this.relatedPeople.filter((item) => predicate(item));
   items.forEach((rp) => {
    this.rs.putRelatedPerson(rp);
   });
   return items;
 }

// Replaces all of this code.
//saveNewRelatedPeople() {
// let newOnes = this.relatedPeople.filter((rp1) => rp1.isNew);
// newOnes.forEach((rp2) => {
//   this.putRelatedPerson(rp2);
// });
// return newOnes;
//}

//saveExistingRelatedPeople() {
// let notNew = this.relatedPeople.filter((item) => !item.isNew);
// notNew.forEach((rp) => {
//  this.rs.putRelatedPerson(rp);
// });
//}

Using the Predicate

Resulting in using the new function like this:

 function setIsNewToFalse() {
   if (this.saveIsValid) {
   let newOnes = 
    saveRelatedPeople((item) => item.isNew);
    saveRelatedPeople((item) => !item.isNew);
    newOnes.forEach((rp) => (rp.isNew = false));
  }
 }

If we look for single responsibility violations in this code, indeed we can refactor this code one more time. Can you spot where? How about this...

Single Responsibility?

 function setIsNewToFalse() {
   if (this.saveIsValid) {   
   saveRelatedPeople((item) => item.isNew);
   saveRelatedPeople((item) => !item.isNew);   
   // The isNew state change has been moved 
  }
 }

Which brings us to the realization that we are probably able to combine these predicates into one routine.

Almost There

function setIsNewToFalse() {
  if (this.saveIsValid) {
    // mark the isnew flag to false when saved.
    saveRelatedPeople();
  }
 }
// this code
 function saveRelatedPeople() {
  let items = 
   this.relatedPeople.filter((item) => 
   item.isNew || !item.isNew);
   items.forEach((rp) => {
      this.rs.putRelatedPerson(rp);
      rp.isNew = false;
  });
  return items;
 }

Final Result
All of the code at the top, compressed downward to just this code here. We then renamed the function to reflect its behavior better.

// saveAll is a better name
function saveAll() {
   if (this.saveIsValid) {
      let items = 
         this.relatedPeople.filter(
         (item) => item.isNew || !item.isNew
      );
      items.forEach((rp) => {
         this.rs.putRelatedPerson(rp);
         rp.isNew = false;
      });
      return items;
   }
}

Although the predicate solution didn't work out, it helped us to see patterns to collapse. This is also why we say that functions should be around 10 lines of code max. A bit more maintainable and a bit faster!

JWP2020 Refactoring Using Predicates