Mungo Frage Versprechen in Funktion machen Code wurde hässlich

Question

Ich war meine Benutzer API erstellen, möchte ich überprüfen, ob der Benutzername verwendet wurde. Also schrieb ich eine statische Funktion

static findByName(name) { 
    const query = User.where({ 
    username: name, 
    }); 
    query.findOne((queryErr, user) => { 
    if (queryErr) { 
     console.log(queryErr); 
     return false; 
    } 
    return user; 
    }); 
} 

, wenn ich es in SIGNUP

signup(req, res) { 
    if (!req.body.username || !req.body.password || !req.body.email) { 
    return res.status(400).json({ success: false, message: 'Bad Request' }); 
    } 
if (!Users.findByName(req.body.username)) { 
    return res.status(409).json({ success: false, message: 'Username has been used' }); 
} 
    const hashedPassword = this.genHash(req.body.password); 
    const newUser = User({ 
    username: req.body.username, 
    }); 
} 

genannt

findByName undefiniert zurück. Schließlich verwende ich Versprechen.

signup(req, res) { 
    if (!req.body.username || !req.body.password || !req.body.email) { 
    return res.status(400).json({ success: false, message: 'Bad Request' }); 
    } 
    return Users.findByName(req.body.username).then((existingUser) => { 
    if (existingUser) { 
     return res.status(409).json({ success: false, message: 'Username has been used' }); 
    } 
    const hashedPassword = this.genHash(req.body.password); 
    const newUser = User({ 
     username: req.body.username, 
     password: hashedPassword, 
     email: req.body.email, 
    }); 
    return newUser.save().then((user) => { 
     res.json({ success: true, user }); 
    }).catch((err) => { 
     res.status(500).json({ success: false, message: 'Internal Server Error' }); 
    }); 
    }).catch((err) => { 
    res.status(500).json({ success: false, message: 'Internal Server Error' }); 
    }); 
} 

Das ist wirklich schrecklicher Code. Gibt es eine bessere Möglichkeit, den Code zu reinigen?

Answer 1

Gibt es eine bessere Möglichkeit, den Code zu reinigen

Ja. Ich werde /signup anzunehmen, wird als POST Route definiert auf der app Instanz

üblich Express Damit Sie gesagt haben, da Sie bereits Zusagen verwenden, können Sie weiterhin einen Schritt gehen und verwenden async/await, die in Knoten standardmäßig aktiviert ist .js v7.6 +.

Dadurch wird Ihr Code machen mehr synchron lesen:

async signup(req, res) { 
    if (!req.body.username || !req.body.password || !req.body.email) { 
    return res.status(400).json({ success: false, message: 'Bad Request' }); 
    } 

    try { 
    const existingUser = await Users.findByName(req.body.username) 
    if (existingUser) { 
     return res.status(409).json({ success: false, message: 'Username has been used' }) 
    } 

    const hashedPassword = this.genHash(req.body.password); 
    const newUser = await User({ 
     username: req.body.username, 
     password: hashedPassword, 
     email: req.body.email, 
    }).save() 

    res.json({ success: true, newUser }); 

    } catch (error) { 
    res.status(500).json({ success: false, message: 'Internal Server Error' }); 
    } 
} 

Sie bemerkt, die Verwendung von try/catch haben. Dies liegt daran, da wir .catch() nicht verwenden und wir noch jeden Fehler behandeln müssen, der auftritt. Um weiter aufzuräumen Code können wir einen Fehlerhandler Middleware schreiben, die Pflege von Fehlern für uns nehmen:

src/middleware/error-handlers.js

// Wraps the router handler, catches any errors, and forwards to the next middleware that handles errors 
exports.catchErrors = action => (req, res, next) => action(req, res).catch(next); 

// Notice the first parameter is `error`, which means it handles errors. 
exports.displayErrors = (error, req, res, next) => { 
    const err = error; 
    const status = err.status || 500; 
    delete err.status; 
    err.message = err.message || 'Something went wrong.'; 

    if (process.env.NODE_ENV === 'production') { 
    delete err.stack; 
    } else { 
    err.stack = err.stack || ''; 
    } 

    res.status(status).json({ 
    status, 
    error: { 
     message: err.message, 
    }, 
    }); 
}; 

Jetzt müssen wir nur unsere Fehler-Handler use:

app.js

const { catchErrors, displayErrors } = require('./middleware/error-handlers') 

// Whenever you defined the function, needs to have the `async` keyword 
async function signup(req, res) { ... } 

// Wrap the function call 
app.post('/signup', catchErrors(signUp)) 

// Handle any errors 
app.use(displayErrors) 

die oben Middleware verwandelt unseren Code:

async signup(req, res) { 
    const error = new Error() 

    if (!req.body.username || !req.body.password || !req.body.email) { 
    error.status = 400 
    error.message = 'Bad Request' 
    throw error 
    } 

    const existingUser = await Users.findByName(req.body.username) 
    if (existingUser) { 
    error.status = 409 
    error.message = 'Username has been used' 
    throw error 
    } 

    const hashedPassword = this.genHash(req.body.password); 
    const newUser = await User({ 
    username: req.body.username, 
    password: hashedPassword, 
    email: req.body.email, 
    }).save() 

    res.json({ success: true, newUser }); 
} 

Sie können sehen, wie der Code viel einfacher zu lesen ist, ohne das Rauschen.

Achten Sie darauf, lesen:

• Writing middleware for use in Express apps
• Express error handling

Answer 2

Ich kann nicht testen, ob meine Lösung tatsächlich funktioniert (wahrscheinlich nicht) oder dass ich alle Funktionen des ursprünglichen Codes abdecke. Ich habe versucht, Funktionen zu erstellen, die bestimmte Dinge handhaben, und sie dann miteinander zu verketten, damit Sie einen klareren Fluss des Programms sehen können.

signup(req, res) { 
    if (!req.body.username || !req.body.password || !req.body.email) { 
    return res.status(400).json({ success: false, message: 'Bad Request' }); 
    } 
    const handleError = error => res.status(500).json({ success: false, message: 'Internal Server Error' }); 
    const handleExistingUser = existingUser => { 
    if (existingUser) { 
     return res.status(409).json({ success: false, message: 'Username has been used' }); 
     } else { 
     return Promise.resolve(); 
     } 
    } 
    const handleCreateUser =() => { 
    const hashedPassword = this.genHash(req.body.password); 
    const newUser = User({ 
     username: req.body.username, 
     password: hashedPassword, 
     email: req.body.email, 
    }); 
    return newUser.save().then((user) => { 
     res.json({ success: true, user }); 
    }); 
    }; 

    // the flow of the program is hopefully more clear here 
    return Users.findByName(req.body.username) 
    .then(handleExistingUser) 
    .then(handleCreateUser) 
    .catch(handleError); 
} 

Wenn Sie den Fehler der inneren und äußeren Versprechen der gleiche Griff, dann denke ich, es ist genug in der äußeren Schicht um den Fehler haben Handhabung. (In Ihrem letzten Beispiel.) Aber ich bin mir nicht 100% sicher.

Answer 3

Ihre Funktion gibt undefined, da es keine return Aussage hat. Die return user Anweisung ist der (nutzlose) Rückgabewert für die Callback-Funktion findOne, nicht für findByName.

Wenn Sie verspricht gehen, dann die Funktion wie folgt definieren:

static findByName(name) { 
    return User.where({ username: name }).findOne().exec(); 
} 

Und Ihre Versprechen Kette kann ein wenig vereinfacht werden, wie folgt aus:

signup(req, res) { 
    function report(message, status = 200) { 
     res.status(status).json({ success: status === 200, message }); 
    } 

    if (!req.body.username || !req.body.password || !req.body.email) { 
     return report('Bad Request', 400); 
    } 
    Users.findByName(req.body.username).then((existingUser) => { 
     return existingUser ? null // treat this condition in the next `then` 
       : User({ 
        username: req.body.username, 
        password: this.genHash(req.body.password), 
        email: req.body.email, 
       }).save().exec(); 
    }).then((user) => { 
     return existingUser ? report(user) 
          : report('Username has been used', 409); 
    }).catch((err) => { 
     report('Internal Server Error', 500); 
    }); 
}