suggestion: Simpler API with constructor-style Open

[tv42]

Mirroring os.File, if instead of DB.Open you had

func Open(name string, mode os.FileMode) (*DB, error)

that could save some error checking, and even more interestingly, that would almost remove the current only source of errors for DB.Transaction and .RWTransaction.

Almost because one could still run db.Close(); d.Transaction(). But that oughta be rare enough... so to make this idea complete, what if .Transaction and .RWTransaction, when run on a closed db, always successfully returned transactions, where all the operations will just fail? So in essence, this would delay the error checking to the point where you need to check errors anyway. This seems well aligned with http://talks.golang.org/2013/bestpractices.slide#5 and would, in my opinion, simplify typical code.

Unless you foresee other sources of errors for Transaction and RWTransaction.

P.S. Bolt looks really promising. No more fighting with cgo for me!

[tv42]

Similarly, if Bucket was a datatype that's actually useful for callers, and refcounted the buckets, one could use that to avoid the error on every call of .Cursor; the only source for error is bucket not existing.

[benbjohnson]

I like the idea of adding bolt.Open(). That'd be easy enough to add. Making Bucket a useful type makes a lot of sense too. It'd be nice to do this without error checks:

b := txn.Bucket("foo")
value := b.Get([]byte("bar"))

I'm hesitant to defer ErrDatabaseNotOpen errors to transaction operations. For example, if I know that bucket "widgets" exists in my database then I'll won't do a nil check after retrieving from a transaction. I can't do that if the transaction will fail and return nil for a closed database.

Ultimately I think most people will use the Do() and With() functions which combines transaction open and close errors together.

Thanks for taking such a deep dive into the code and poking around. It's great to get feedback like this!

[tv42]

I wrote that part before discovering Do() and With(). They look good, hopefully the closures won't cost too much.

[benbjohnson]

@tv42 I'm a little worried about closure performance too but hopefully it won't be too bad. I'll add some closure vs non-closure benchmarks once I get to profiling.

[benbjohnson]

@tv42 I'm going to move the data accessor and mutator functions from Transaction and RWTransaction to the Bucket:

Cursor() *Cursor
Delete(key []byte) error
ForEach(fn func(k, v []byte) error) error
Get(key []byte) []byte
NextSequence() error
Put(key, value []byte) error

If accessors (e.g. Bucket.Get(), Bucket.Cursor()) are called after a Transaction is closed should they panic? I think so.

I'm still going to keep those functions on DB as ease-of-use functions.

I'll try to get to it this weekend but it may be early next week before I can do it.

[benbjohnson]

Added bolt.Open() in #53.