Skip to content
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

What can we expect when we call get with a non-exist key? #3

Closed
huan opened this issue Aug 14, 2019 · 3 comments
Closed

What can we expect when we call get with a non-exist key? #3

huan opened this issue Aug 14, 2019 · 3 comments

Comments

@huan
Copy link
Contributor

huan commented Aug 14, 2019

I found the snapDb.get('non-exist-key') will return a string:

Unable to get key or key not found!

Which is not expected from my code.

I believe it could be better if returns an undefined.

I write a unit test for it, and it can not pass with the head of the master branch code.

        it("Get non-exist key", async (done: MochaDone) => {
            try {
                const val = await db_str.get('non-exist-key')
                console.log('VAL', val)
                expect(val).to.equal(undefined, "get a non-exist key should return undefined");
                done();
            } catch (e) {
                done(e);
            }
        });

Could you please explain that what's the expected behavior of the get method, and what should I do if I want to know whether my key exists in the snapDb?

Thanks!

@only-cliches
Copy link
Owner

The newest version, 1.1.2 will return undefined instead of an error on blank keys.

@huan
Copy link
Contributor Author

huan commented Aug 15, 2019

Thanks for the fix!

However, I found the current version of the code returns null instead of the undefined, it's wired because I saw your code to return undefined. So I send a PR to add a unit test for that, you can reproduce the value by running the unit test.

And also, I believe we should change the return type from Promise<string> to Promise<undefined | string> in index.ts:

https://github.com/ClickSimply/snap-db/blob/4358242347fde0240ba95977e1aadf6e94cf2151/src/index.ts#L280

only-cliches pushed a commit that referenced this issue Aug 15, 2019
Add a unit test for get() with non-exist-key (#3)
@only-cliches
Copy link
Owner

1.1.2 is now live on NPM, this issue has been resolved. Feel free to comment if you find anything further!

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

No branches or pull requests

2 participants