-1

Using Typescript with Angular JS, sometimes I would create a temporary variable in a loop to set a value. I am looping through a JSON response and determining which items I will want to show. Items have a few variables that determine whether or not they will show on the front end. Specifically, this is for a survey. I want to show only questions that are active if the survey is not completed, but if it is completed, then I want to only show questions that have an answer (these can be "inactive"[deleted] questions.)

Example JSON response:

{
  "surveyId": 2,
  "completed": false
  "questions" : [ {
    "id" : 1111,
    "question": "Blah blah blah blah ?",
    "answer": null
  }, 
  {
    "id" : 1112,
    "question" : "Yes no no no yes?",
    "answer": 1,
    "active": true
}

Dealing with the response data in my controller:

myService.getSurvey(vm.id)
  .success(function(data: any) {
    ...
    let questionableJSON: {id: number, question: string, answer: number}[] = [];
    for (let obj of data.questions)  {
      //comparing how i do this: set a "temp" variable to use:
      let answerNum: number = 1;
      //this is only checking for "falsey" not undefined or null -- that is unimportant for the sake of this question
      if (obj.answer) {
        answerNum = obj.answer;
      }
      //vs. way #2: just checking if the var exists and setting it
      if (angular.isUndefined(obj.active)) {
        obj.active = true;
      }
      //now push to the temp JSON structure:
      if (data.completed && obj.answer) {
        questionableJSON.push({
          id: obj.id,
          question: obj.question,
          answer: answerNum
        });
      }
      else if (!data.completed && obj.active) {
        questionableJSON.push({
          id: obj.id,
          question: obj.question,
          answer: answerNum
        });
      }
    }
  });
  //now set our controller question data to our newly obtained data populated in our temp JSON from the response:
  vm.questions = questionableJSON;

The question is: is it better to set a temp variable for obj.active the same way that I set a temp variable for answerNum? Or is it better to just go right ahead and set obj.active to a value instead of declaring another temporary variable for each iteration of the loop? Which of these practices is cleaner or more efficient? This goes with regular javascript too and this function is not much different from regular JS much at all. This is not specific to Angular in any way.

A second part question if that isn't enough, is it more efficient to just combine the if.. else if into one if statement with a || like:

if (data.completed && obj.answer || !data.completed && obj.active)

Thanks

Ian J Miller
  • 83
  • 1
  • 5
  • 1
    I bet you have already wasted more CPU cycles writing this question than you will ever save by assigning `obj.active` to a temp var rather than dereferencing it every time :). Readability first ! Maintaining code **is** expensive. – Bruno Grieder Jan 25 '17 at 17:27

1 Answers1

0

Your question asked, "is it better," which can be interpreted from a number of vantage points. I'm going to interpret your question as, "What is most performant?"

Referring to this question, it seems that let-scoped variables are scoped in the loop body level, which means variables declared inside the loop body will be up for garbage collection immediately after each loop body run. From this point of view, it really makes no difference as long as your data sets returning from your survey api aren't hundreds of thousands of results long. The performance hit of creating one boolean variable that exists only in the scope of the loop body surely isn't significant compared to the time it takes for the api call itself to resolve.

However, like Bruno mentioned in the comments, it may be more readable and maintainable down the line to simply be explicit about it. Although your question wasn't about readability, it was about "what's better," which IMHO includes code readability.

Community
  • 1
  • 1
N. Taylor
  • 61
  • 4