CODEWITHSUNDEEP
javascriptreactjs
Annah Isenberg
Annah Isenberg

Reputation: 157

Is there a simpler way to write this React function?

I feel like I'm repeating myself a lot in this React function. I need to check if most fields in my state are empty, but there's a few I do not want to check. So I am not sure how to do that.

Here it is:

onSearchClick = () => {
  const {insightName, insightDescription, createdBy, category, role, insightSource, assignTo, endDate, startDate} = this.state;
  if(
    insightName === "" &&
    insightDescription === "" &&
    createdBy === "" &&
    category === "" &&
    role === "" &&
    insightSource === "" &&
    assignTo === "" &&
    (endDate === "" || endDate === null) &&
    (startDate === "" || startDate === null) 
  )
  {
    window.scrollTo(500, 0);
    this.setState({
      isFormValid: false,
    })
  } else if (
    insightName === "" &&
    insightDescription === "" &&
    createdBy === "" &&
    category === "" &&
    role === "" &&
    insightSource === "" &&
    assignTo === "" &&
    (endDate === "" || endDate === null)
  ) {
    window.scrollTo(500, 0);
    this.setState({
      showEndDateMsg: true
    })
  } else if (
    insightName === "" &&
    insightDescription === "" &&
    createdBy === "" &&
    category === "" &&
    role === "" &&
    insightSource === "" &&
    assignTo === "" &&
    (startDate === "" || startDate === null)
  ) {
    window.scrollTo(500, 0);
    this.setState({
      showStartDateMsg: true
    })
  } else {

    this.setState({
      showTable: true
    })
}
}

I want to follow the DRY principles but am not sure what to do! Any advice would be much appreciated. Thanks.

Upvotes: 1

Views: 82

Answers (3)

jiminikiz
jiminikiz

Reputation: 2899

invalidForm = (reason) => {
  window.scrollTo(500, 0);

  switch(reason) {
    case 'noStartDate': 
      return this.setState({ showStartDateMsg: true });
    case 'noEndDate': 
      return this.setState({ showEndDateMsg: true });
    default:
      return this.setState({ isFormValid: false });
  }
}

onSearchClick = () => {
  const {
    insightName,
    insightDescription,
    createdBy,
    category,
    role,
    insightSource,
    assignTo,
    endDate,
    startDate
  } = this.state;

  const hasEmptyFields = [
    insightName,
    insightDescription,
    createdBy,
    category,
    role,
    insightSource,
    assignTo,
    endDate,
    startDate
  ].filter((field) => field === '');

  if(hasEmptyFields) {
    return this.invalidForm();
  }

  if(!startDate) {
    return this.invalidForm('noStartDate');
  }

  if(!endDate) {
    return this.invalidForm('noEndDate');
  }

  return this.setState({ showTable: true });
};

Upvotes: 0

Taki
Taki

Reputation: 17664

You can check if the concated value of your variables equal an empty string instead of checking them one by one

const emptyValues = insightName + insightDescription + createdBy + category + role + insightSource + assignTo === "";

And put the scrollTo in the if block when the values are empty 

onSearchClick = () => {
  const {
    insightName,
    insightDescription,
    createdBy,
    category,
    role,
    insightSource,
    assignTo,
    endDate,
    startDate
  } = this.state;

  const emptyValues = insightName + insightDescription + createdBy + category + role + insightSource + assignTo === "";

  const noEndDate = endDate === "" || endDate === null;  

  const noStartDate = startDate === "" || startDate === null;

  if (emptyValues) {
    window.scrollTo(500, 0);
    if (noEndDate && noStartDate) {
      this.setState({
        isFormValid: false
      });
    } else if (noEndDate) {
      this.setState({
        showEndDateMsg: true
      });
    } else if (noStartDate) {
      this.setState({
        showStartDateMsg: true
      });
    }
  } else {
    this.setState({
      showTable: true
    });
  }
};

Upvotes: 2

krillgar
krillgar

Reputation: 12815

Set up a variable to contain everything you're continually checking:

const isMostlyEmpty = insightName === "" &&
                      insightDescription === "" &&
                      createdBy === "" &&
                      category === "" &&
                      role === "" &&
                      insightSource === "" &&
                      assignTo === "";

Then you can reuse that in each of your statements:

if(isMostlyEmpty &&
   (endDate === "" || endDate === null) &&
   (startDate === "" || startDate === null)) {
        window.scrollTo(500, 0);
        this.setState({
          isFormValid: false,
        });
} else if (isMostlyEmpty &&
           (endDate === '' || endDate === null)) {
    window.scrollTo(500, 0);
    this.setState({
      showEndDateMsg: true
    });
} else if (isMostlyEmpty &&
           (startDate === '' || endDate === null)) {
    window.scrollTo(500, 0);
    this.setState({
      showStartDateMsg: true
    });
} else {
    this.setState({
        showTable: true
    });
}

Upvotes: 0

Related Questions