Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

connect() is asynchronous, API (esp. isConnected()) encourages synchronous interface #18

Open
fatuhoku opened this issue Mar 15, 2013 · 1 comment

Comments

@fatuhoku
Copy link

Node-DBI users are encouraged to write code like the following (in Coffeescript):

db = new DBWrapper 'sqlite3', { ... }
db.connect()
console.error 'Failed to connect to database' if not db.isConnected()

The two solutions to this issue are:

  • use async, and document this in README.md:
db = new DBWrapper 'sqlite3', { ... }
async.series [
   -> db.connect()
   -> console.error 'Failed to connect to database' if not db.isConnected()
]
  • provide a callback in the connect(): i.e. change the interface to connect(callback)
@olivierphi
Copy link
Owner

@fatuhoku
Hello !

Thank you for this report. Currently there is a "connection" Event emitted by the DBWrapper instance when the DB connection is resolved, with a "err" param.
But you're right, it may be more "Nodish" to be able to provide a callback on the "connect()" method. I'm going to try to add this feature...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants