-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add rocksdb.swift #5 #40
Conversation
do { | ||
db = try RocksDB(path: path, prefix: prefix) | ||
} catch { | ||
throw NSError(domain: Database.errorDomain, code: 2, userInfo: [NSLocalizedDescriptionKey: "Failed to init RocksDB: \(error)"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't use NSError.
use something like
public enum Error: Swift.Error {
case initFailed,
case notOpen,
}
and throw Error. initFailed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but in this case, there is no need to do anything, just do catch it and let the error propagate will be fine
public class Database { | ||
public let path: URL | ||
public let prefix: String? | ||
private var db: RocksDB? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to make db
optional.
do { | ||
return try type.init(data: db.get(key: key)) | ||
} catch { | ||
throw error | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to catch and throw
keyType _: Key.Type, | ||
valueType _: Value.Type, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they are not used?
Database/Tests/LinuxMain.swift
Outdated
@@ -0,0 +1,7 @@ | |||
import XCTest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to have this, swift test
should just work
@@ -0,0 +1,9 @@ | |||
import XCTest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to have this
public let path: URL | ||
public let prefix: String? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to have those
No description provided.