Set the correct server request path

Question

Can this if-else statement be simplified and/or cleaned up?

var path = require('path')
var express = require('express')
var app = express()
app.get('/*', function (req, res) {
 var page;
 if (req.params[0] === '') {
 // if requesting root dir, add `index.html`
 page = path.parse(path.join(__dirname, 'index.html'))
 } else {
 // otherwise add requested path
 page = path.parse(path.join(__dirname, req.params[0]))
 if (page.ext === '') {
 // if requested page has no extension, change page, adding `index.html`
 page = path.parse(path.join(__dirname, req.params[0], 'index.html'))
 }
 }
 if (page.ext === '.html') {
 /*... do html stuff ...*/
 }
 else if (page.ext === '.css') {
 /*... do css stuff ...*/
 }
 /*...*/
})

It works correctly but I feel it’s "bad coding". There are nested if statements and I’m repeating a lot of code. Also, below the first condition, there are other conditions testing the type of page (HTML, CSS, etc.) Since in the page.ext==='' condition, the value of page may change, it's not a mutually exclusive case of the other conditions.

It’s weird and feels wrong, but I can’t quite explain why.

I tried using the ternary expression (?:), but I still can’t avoid the if statement afterward.

var page = path.parse(path.join(__dirname, (req.params[0] === '') ? 'index.html' : req.params[0]))
if (page.ext === '') page = path.parse(path.join(__dirname, req.params[0], 'index.html'))

Is there a way to do it without repeating code? I’d like to avoid assigning page more than once.

---

+++ I’ve come up with a less verbose way of doing it, and it only requires assigning page once. Though it still requires calling path.parse() more than once.

var filepath = path.join(__dirname, req.params[0] || 'index.html')
var page = path.parse(path.join(filepath, (path.parse(filepath).ext === '') ? 'index.html' : ''))

---

+++ After discovering path.extname(<String>), I realized I don’t need to parse it beforehand. So this simplifies to:

var filepath = path.join(__dirname, req.params[0] || 'index.html')
var page = path.parse(path.join(filepath, (path.extname(filepath) === '') ? 'index.html' : ''))

Answer 1

Perhaps consider building an array of path segments through your logic and then setting page with it.

app.get('/*', function (req, res) {
 var page
 var base = req.params[0]
 var baseExtension = path.parse(base).ext
 // determine path segments
 var pathSegments = [__dirname]
 if (base !== '') {
 pathSegments.push(base)
 }
 if (baseExtension === '') {
 pathSegments.push('index.html') 
 }
 // set page
 // here we use spread operator
 page = path.parse(path.join(...pathSegments))
 // if not using ES6, you could alternatively use
 // page = path.parse(path.join.apply(pathSegments))
 // perhaps consider switch here instead of if-else if you are going
 // to have more than just html and css extension types
 if (page.ext === '.html') {
 /*... do html stuff ...*/
 }
 else if (page.ext === '.css') {
 /*... do css stuff ...*/
 }
 /*...*/
})

Be consistent on your usage or non-usage of semicolons to end lines of code. It seems like you have chosen to use the no semi-colon style, so why is there one after var page;?

• chharvey: I usually stick to the no-semicolon style, but that semicolon shows the unassigned var isn't a typo
• Mike Brant: @chharvey Gotcha. I usually see intentionally inserted semicolons at the start of the following line with this style (which I admit I am not particularly used to using.) but wasn't going to add a code review comment to "add semicolons to every line" as that is really a styling choice a developer or development team needs to make - so long as it is applied consistently.