Hey there! Great job getting your first Express routes up and running—let's tackle your two questions with clear explanations and actionable improvements.

1. Does your current code follow industry standards? #

Your code works, but there are several areas where it can be adjusted to align with common Node.js/Express best practices, plus a couple of hidden bugs to fix:

Key Issues & Fixes: #

• Unscoped variables: You're assigning rate and time without let/const, which makes them global variables. This is risky because they can be overwritten by other parts of your app.
• Async logic flaws: getBTC uses a callback, but you're calling it synchronously before your routes. When the routes render, the API response might not have arrived yet—so rate and time will be undefined. Also, the this in your route callbacks doesn't point to your index object, so even if the data was ready, this.rate would fail.
• Outdated syntax: Using var is not recommended in modern Node.js; use const for variables that don't change, and let for those that do.
• Mixed concerns: You're combining route definitions, API calls, and rendering logic in one object, which makes the code harder to maintain as your app grows.

Improved Version of Your Code: #

const express = require("express")
const router = express.Router()
const request = require("request-promise-native") // Use promise-based request for async/await

// Async function to fetch BTC data (no global variables!)
async function getBTC() {
  try {
    const body = await request.get("http://api.coindesk.com/v1/bpi/currentprice.json")
    const data = JSON.parse(body)
    return {
      rate: data.bpi.USD.rate,
      time: data.time.updated
    }
  } catch (err) {
    console.error("Failed to fetch BTC data:", err)
    return { rate: "Unavailable", time: "Unavailable" } // Fallback for errors
  }
}

// Route handlers with proper async handling
router.get('/', async (req, res, next) => {
  const btcData = await getBTC()
  res.render('common/index', { title: 'Index Title', ...btcData })
})

router.get('/test', async (req, res, next) => {
  const btcData = await getBTC()
  res.render('common/test', { title: 'Test Title', rate: btcData.rate })
})

module.exports = router

2. Can you use external controllers (like controller/users/index.js)? #

Absolutely—this is actually a best practice for Express apps! Separating route definitions from business logic (controllers) makes your code more modular, testable, and easier to scale.

Recommended Directory Structure: #

your-project/
├── routes/
│   ├── index.js          # Main routes
│   └── api/
│       └── users.js      # API user routes
├── controllers/
│   ├── homeController.js # Handles homepage/test routes
│   └── api/
│       └── userController.js # Handles user-related API logic
├── services/             # Optional: For shared logic like API calls
│   └── cryptoService.js  # BTC fetching logic here
├── views/
│   └── common/
│       ├── index.pug
│       └── test.pug
└── app.js

Example Implementation: #

1. services/cryptoService.js (shared API logic)

const request = require("request-promise-native")

async function getBTCPrice() {
  try {
    const body = await request.get("http://api.coindesk.com/v1/bpi/currentprice.json")
    const data = JSON.parse(body)
    return {
      rate: data.bpi.USD.rate,
      time: data.time.updated
    }
  } catch (err) {
    console.error("BTC fetch error:", err)
    return { rate: "Unavailable", time: "Unavailable" }
  }
}

module.exports = { getBTCPrice }

2. controllers/homeController.js (route logic)

const { getBTCPrice } = require("../services/cryptoService")

async function getIndexPage(req, res) {
  const btcData = await getBTCPrice()
  res.render('common/index', { title: 'Index Title', ...btcData })
}

async function getTestPage(req, res) {
  const btcData = await getBTCPrice()
  res.render('common/test', { title: 'Test Title', rate: btcData.rate })
}

module.exports = { getIndexPage, getTestPage }

3. routes/index.js (clean route definitions)

const express = require("express")
const router = express.Router()
const homeController = require("../controllers/homeController")

router.get('/', homeController.getIndexPage)
router.get('/test', homeController.getTestPage)

module.exports = router

This structure keeps each file focused on one job—routes just map URLs to controllers, controllers handle request/response logic, and services handle reusable tasks like API calls.

内容的提问来源于stack exchange，提问作者John Loves JS