fac-15 / I-Choose-Who-

Alex, Charlie, Sak and Jason's project. Heroku link -->
https://i-choose-who.herokuapp.com/
0 stars 0 forks source link

There are some parts in your handler.js which could be refactored. #13

Open HStewart23 opened 5 years ago

HStewart23 commented 5 years ago

As I said, super big fan of the way you have split up your files so far this is prehaps something you could do as an extension of that ? In your handler.js file, this block of code (or very similar versions) repeats a few times)

    if (error) {
      console.log(error);
      response.writeHead(500, "Content-Type: text/html");
      response.end("<h1>Sorry, we've had a problem on our end</h1>");
    } else {
      response.writeHead(200, "Content-Type: text/html");
      response.end(file);
    }
  });  

^ This is in your handelHomeRoute

    if (error) {
      console.log(error);
      response.writeHead(404, "Content-Type: text/html");
      response.end("<h1>404 file not found</h1>");
    } else {
      response.writeHead(200, `Content-Type: ${extensionType[extension]}`);
      response.end(file);
    }
  }); 

^ This is in your handlePublic route

    if (error) {
      console.log(error);
      console.log("HANDLED JSON REQ BUT ERROR");
      response.writeHead(500, "Content-Type: text/html");
      response.end("<h1>Sorry, we've had a problem on our end</h1>");
    } else {
      console.log("handled JSON req");
      response.writeHead(200, "Content-Type: application/json");
      response.end(JSON.stringify(pokeObj));
    }
  }); 

^ This is in your handleJSON route

Is there a way to make it less repetitive?

matthall8 commented 5 years ago

There is a way to refactor all 3 of these functions into 1 function and also have a 404 error which captures all remaining events.

The 404 error may not always be the correct one to send in your handlePublic route function (is it a 404 if there is an error when trying to read a file that exists?)