Handling rejected HTTP responses with status 401

Question

I'm kinda new to javascript and have question about it. Now my practical project is growing a bit bigger. I got the following if else statement. Is it possible to refactor this ? Should i use an use case or are there other options or will this decrease the loading speed of the application.

(function () {
 'use strict';
 angular
 .module('app')
 .factory('HttpResponseInterceptor', HttpResponseInterceptor);
 HttpResponseInterceptor.$inject = ['$q','$location'];
 function HttpResponseInterceptor($q,$location) {
 return {
 response: function(response){
 if (response.status === 401) {
 }
 return response || $q.when(response);
 },
 responseError: function(rejection) {
 if(rejection.status ===401 && rejection.config.url === "user") {
 console.log("Check")
 }
 else if (rejection.status ===401 && rejection.config.url == "l18n/fr_FR.json")
 {
 console.log("Check2")
 }
 else if (rejection.status ===401 && rejection.config.url == "l18n/nl_NL.json")
 {
 console.log("Check2")
 }
 else if (rejection.status ===401 && rejection.config.url == "l18n/de_DE.json")
 {
 console.log("Check2")
 }
 else if (rejection.status ===401 && rejection.config.url == "l18n/es_ES.json")
 {
 console.log("Check2")
 }
 else if (rejection.status ===401 && rejection.config.url == "l18n/en_US.json")
 {
 console.log("Check2")
 }
 else
 {
 $location.path('/login');
 }
 /* if (rejection.status === 401 && rejection.config.url !== "user") {
 $location.path('/login');
 }*/
 return $q.reject(rejection);
 }
 };
 }
})();

• BCdotWEB: As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. For examples of good titles, check out Best of Code Review 2014 - Best Question Title Category You may also want to read How to get the best value out of Code Review - Asking Questions.

Answer 1

I'd actually go a bit further than janos. He is perfectly right in that by repeatedly checking rejection.status, you're repeating yourself in code. Repeatedly checking the variable against multiple values makes this a perfect candidate for a switch statement:

responseError: function(rejection) {
 if (rejection.status === 401) {
 switch (rejection.config.url) {
 case "user":
 console.log("Check");
 break;
 case "l18n/fr_FR.json":
 case "l18n/nl_NL.json":
 case "l18n/de_DE.json":
 case "l18n/es_ES.json":
 case "l18n/en_US.json":
 console.log("Check2");
 break;
 default:
 $location.path('/login');
 break;
 }
 }
 return $q.reject(rejection); 
}

Answer 2

Don't repeat yourself. The repeated checks on rejection.status ===401 are clearly pointless. Rewrite in a way to check this condition only once, for example by nesting the other conditions:

if (rejection.status === 401) {
 if (rejection.config.url === "user") {
 console.log("Check")
 }
 else if (rejection.config.url == "l18n/fr_FR.json")
 {
 console.log("Check2")
 }
 else if (rejection.config.url == "l18n/nl_NL.json")
 {
 console.log("Check2")
 }
 //
 // ... more checks
 //
 else
 {
 $location.path('/login');
 }
} else
{
 $location.path('/login');
}

• Jonah: You're better off with a guard clause at the top to handle the non-401 case. That way the large if...else can be handled without nested indentation.