Skip to content

Some cleanup for extended diagnostic branch#1

Open
karzhenkov wants to merge 5 commits intodiagnosticsfrom
diagnostics-cleanup
Open

Some cleanup for extended diagnostic branch#1
karzhenkov wants to merge 5 commits intodiagnosticsfrom
diagnostics-cleanup

Conversation

@karzhenkov
Copy link
Owner

No description provided.

@karzhenkov karzhenkov mentioned this pull request Jan 24, 2021
5 tasks
@nlohmann
Copy link

nlohmann commented Jan 24, 2021

I see what you are doing, and it seems to me the code could be simplified as follows:

  • Instead of passing a diagnostics_t object, we pass a basic_json::reference to the exceptions (just as you did).
  • But instead of using templates, that parameter could just default to nullptr; that is, a null JSON value.
  • This value will never occur in the parent relation, so the diagnostic string will always be empty.

This will have the same code as you have on the call site, but may have a simplified diagnostics_t class. What do you think?

@karzhenkov
Copy link
Owner Author

basic_json is a template, so a function taking basic_json::reference cannot be non-templated; there is no universal null JSON value. I think, the further simplification is quite possible, but in a different way. The diagnostics_t class can be eliminated; its primary functionality can be moved to diagnostics_message.

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