0

I have nodejs accessing a database to retrieve a row of orders. The following function is defined in a file called orderDb.js

module.exports.getWaitingOrders = function getWaitingOrders(callback) {
    pool.query("SELECT * FROM live_order_table WHERE isWaiting = 1;", function(err, rows, fields) {
        if (err) {
            return new Error('Db error: ' + err);
        }
        if (rows.length <= 0) {
            return new Error("No results"); //Is this recommended?
            return [];// or this?
        }

        rows.forEach(function(row) {
            var order = {
                orderID: row.order_id
            }
        });
    }
}

When nothing is returned from the database, should I return an error or an empty array/object?

I'm calling the method like this:

orderDb.getWaitingOrders(function handleWaitingOrders(err, orders){});

I also have a question on how to handle database issues. What i'm doing now, is to log the error and retry later. Is there a better practice i should be following?

sfabriece
  • 121
  • 2
  • 10
  • That depends on what you want to return, and what you're expecting to be returned. You could throw an error, return false, null, undefined, empty array, unicorns or sunshine, it doesn't really matter as long it's what you're expecting in the other end of things. – adeneo Jul 20 '14 at 16:59
  • I agree with @adeneo. But my opinion is that you should only throw an error if that's the source of a problem. An error and an empty list are semantically different, and each should be used in their respective places. If you search for something and 0 items are found, I feel like that should be an empty list, because you're specifically describing that 0 items were found. But if there's a database problem, IO problem, exception in whatever way, you should propagate that. Just my opinion – Ian Jul 20 '14 at 17:02
  • I'm expecting orders on the other end. The dilemma i'm having is do I treat no orders as an error, undefined, empty orders. What makes sense from a good design standpoint? – sfabriece Jul 20 '14 at 17:03
  • @ian I think your thinking makes sense. – sfabriece Jul 20 '14 at 17:06
  • 1
    It honestly depends on what the "on the other end" means. The frontend? If you're simply getting orders and listing them, then it depends on what you want to visually do. Maybe you display a message saying "No waiting orders", otherwise list out the orders. At the same time, if you get an **error**, you display that instead (or in a different place, like a modal). I guess it just feels weird to me to mix errors with empty data, because they're two totally different things and **mean** different things. Obviously, you could mix them and it would work, but it might be harder to maintain – Ian Jul 20 '14 at 17:07
  • 1
    @Ian +1 simply it depends upon how you handle things on other end – Vivek Bajpai Jul 20 '14 at 17:11
  • Thanks for the input, I'll definitely keep errors and returned data as separate as possible. – sfabriece Jul 20 '14 at 17:18
  • 1
    @VivekBajpai Exactly, just thought I'd throw my opinion into this :) – Ian Jul 20 '14 at 17:20
  • Its not an error, its zero results. – Jake Sellers Jul 20 '14 at 20:54

1 Answers1

0

You should return an empty array or null so that you can identify an error with your database

module.exports.getWaitingOrders = function getWaitingOrders(callback) {
    pool.query("SELECT * FROM live_order_table WHERE isWaiting = 1;", function(err, rows, fields) {
        if (err) {
            return new Error('Db error: ' + err) ;
        }
        if (rows.length <= 0) {
            callback(err , []); // you should return an empty response here so that you can differentiate between error and no order

        }

        rows.forEach(function(row) {
            var order = {
                orderID: row.order_id
            }
        });
    }
}


orderDb.getWaitingOrders(function handleWaitingOrders(err, orders){

// do your error handling here 

});
Vivek Bajpai
  • 1,617
  • 1
  • 19
  • 35
  • Same thing @ian suggested in the comments above. Sorry I don't have enough rep to upvote your answer. – sfabriece Jul 20 '14 at 17:09
  • @sfabriece yeah he is totally correct its upto you how you handle the things at other end.But you should always have clear differentiation between the error in case of no orders are return with database – Vivek Bajpai Jul 20 '14 at 17:14