javascript - Is it bad practice to use a promise without reject? - Stack Overflow

admin管理员组

文章数量:1332361

I have the following;

function isUserInDatabase(serverID, playerID) {
    return new Promise((resolve, reject) => {
            executeQuery("SELECT * FROM playerdata where serverID=" + serverID + " AND playerID=" + playerID).then((res) => {
                if (res[0] === undefined) {
                    resolve(false);
                } else {
                    resolve(true);
                }
            });
    }).catch ((e) => {
        console.error(e);
        console.log("Error retrieving data from database.");
    });
}

But I have no reject call. Is this bad convention?

Edit: I'm honestly not sure if this is better. I've done a bit more reading into promises, and perhaps this is a little better, but I'm not sure.

async function handlePlayer(serverID, playerID) { //TEST
    console.log(await isUserInDatabase(serverID, playerID));
}

function isUserInDatabase(serverID, playerID) {
    return executeQuery("SELECT * FROM playerdata where serverID=? AND playerID=?", [serverID, playerID]).then((res) => {
        if (res[0] === undefined) {
            return false;
        } 
        return true;
    })
    .catch ((err) => {
        console.log(err);
    });
}

async function executeQuery(query, opts) {
    let conn;
    try {
        conn = await pool.getConnection();
        return await conn.query(query, opts);
    } catch (err) {
        console.log(err);
    } finally {
        conn.end();
    }
}

I have the following;

function isUserInDatabase(serverID, playerID) {
    return new Promise((resolve, reject) => {
            executeQuery("SELECT * FROM playerdata where serverID=" + serverID + " AND playerID=" + playerID).then((res) => {
                if (res[0] === undefined) {
                    resolve(false);
                } else {
                    resolve(true);
                }
            });
    }).catch ((e) => {
        console.error(e);
        console.log("Error retrieving data from database.");
    });
}

But I have no reject call. Is this bad convention?

Edit: I'm honestly not sure if this is better. I've done a bit more reading into promises, and perhaps this is a little better, but I'm not sure.

async function handlePlayer(serverID, playerID) { //TEST
    console.log(await isUserInDatabase(serverID, playerID));
}

function isUserInDatabase(serverID, playerID) {
    return executeQuery("SELECT * FROM playerdata where serverID=? AND playerID=?", [serverID, playerID]).then((res) => {
        if (res[0] === undefined) {
            return false;
        } 
        return true;
    })
    .catch ((err) => {
        console.log(err);
    });
}

async function executeQuery(query, opts) {
    let conn;
    try {
        conn = await pool.getConnection();
        return await conn.query(query, opts);
    } catch (err) {
        console.log(err);
    } finally {
        conn.end();
    }
}

• javascript
• node.js

Share Improve this question Follow edited Dec 1, 2019 at 8:55 asked Nov 30, 2019 at 22:56 user12346820user12346820 7

• 1 nothing wrong with not rejecting – Bravo Commented Nov 30, 2019 at 22:59
• 1 From my point of view: no. Absolutely nothing out there to suggest that it's a bad practice to use a Promise without reject. – AJC24 Commented Nov 30, 2019 at 23:01
• 1 you should always use prepared queries – Lawrence Cherone Commented Nov 30, 2019 at 23:03
• 2 Note: you are using the Promise constructor antipattern. You should stop doing that. Note also that not using the pletely unnecessary construction avoids the question entirely. Also, and most importantly, NOTE that your query is vulnerable to SQL injection! – Jared Smith Commented Nov 30, 2019 at 23:07
• 2 I'll have a look into that, thank you @Jared Smith. Additionally, I was aware of SQL Injection, but I had issues with the query string to begin with, so I wanted it simplistic for testing. – user12346820 Commented Nov 30, 2019 at 23:24

 |  Show 2 more ments

1 Answer 1

Sorted by: Reset to default 6

Well, you can use promises without reject. However you must be sure that resolve is called in all your function flows to trigger the "then" handler (or the function must end with an error or exception to trigger the catch clause). Reject is often used to trigger error handling logic linked to variable values and results. If the only way your code would fail is due to an exception, then reject is not necessary (this is my opinion though)

As @Jared Smith pointed out, you should do things this way, where the promise actually can reject:

function isUserInDatabase(serverID, playerID) {
    const query = "SELECT * FROM playerdata where serverID=" + serverID + " AND playerID=" + playerID;
    return executeQuery(query).then(res => {
        const isUndefined = res[0] === undefined
        return !isUndefined
    }).catch (e => {
        console.error(e);
        console.log("Error retrieving data from database.");
    });
}

Though you will not be able to handle this promise errors (there's already a catch clause)

You could ignore said catch clause and handle errors after calling isUserInDatabase:

function isUserInDatabase(serverID, playerID) {
    const query = "SELECT * FROM playerdata where serverID=" + serverID + " AND playerID=" + playerID;
    return executeQuery(query).then(res => {
        const isUndefined = res[0] === undefined
        return !isUndefined;
    });
}

...

isUserInDatabase("some_server","some_player")
.then( ok => console.log("User is in database!") )
.catch( e => {                
    console.error(e);
    console.log("Error retrieving data from database.");
})

本文标签： javascriptIs it bad practice to use a promise without rejectStack Overflow