Skip to content

Make error messages more descriptive.#40

Open
nix1 wants to merge 1 commit intomedikoo:masterfrom
nix1:master
Open

Make error messages more descriptive.#40
nix1 wants to merge 1 commit intomedikoo:masterfrom
nix1:master

Conversation

@nix1
Copy link
Collaborator

@nix1 nix1 commented Jul 23, 2015

No description provided.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sKey in dbjs is stringified key, logically it's key that should be displayed, but key can be anything (an object, date, function etc). so indeed maybe it's better to inform here with sKey.

Other note. The good convention I saw in some other libraries is to always use double quotes to wrap names in messages. Also to make it more bulletproof it might be wise to use JSON.stringify, so e.g.: "Cannot delete nested object " + JSON.stringify(sKey)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't check libraries, but in node and in Chrome they are in single quotes, eg:

undefined.test
TypeError: Cannot read property 'test' of undefined

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, still if there's a quote in sKey (that's valid), it may display in unclean way, as e.g. Cannot delete nested object "3fw" sadfadsfa" JSON.stringify will assure that it's one line (even if in key are new line characters, that's also allowed), and that quotes are quoted

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So which ones do you prefer to have?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely should use JSON.stringify here in all cases

@medikoo medikoo assigned nix1 and unassigned medikoo Jul 23, 2015
@medikoo
Copy link
Owner

medikoo commented Jul 23, 2015

Also Travis failed due to too long lines

@nix1
Copy link
Collaborator Author

nix1 commented Aug 25, 2015

Fixed the code style issues.

@nix1 nix1 assigned medikoo and unassigned nix1 Aug 25, 2015
@medikoo medikoo assigned nix1 and unassigned medikoo Aug 25, 2015
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

Successfully merging this pull request may close these issues.

2 participants