kuzudb / kuzu

Embeddable property graph database management system built for query speed and scalability. Implements Cypher.
https://kuzudb.com/
MIT License
1.28k stars 90 forks source link

Namespace organisation #4222

Open benjaminwinger opened 1 week ago

benjaminwinger commented 1 week ago

Continuing an issue from here: https://github.com/kuzudb/kuzu/pull/4166#pullrequestreview-2276856794. In short, I think it would be helpful to standardize how we refer to namespaces, and maybe also look at doing some re-organization of the namespaces. It's obviously not a huge deal, except that, like code formatting, it's a pain if we keep having to re-write things to clean things up, pollutes the git history and makes things harder to understand generally.

One thing I think we could do is to move kuzu::common into the kuzu namespace. This will remove the need for the frequent use of using namespace kuzu::common. On the other hand, I sort of like the practice of using the root namespace for the primary public-facing APIs, which would be more like what's currently in kuzu::main, and since there's so much in kuzu::common, keeping that separate from the stuff in kuzu::main and so that kuzu::main->kuzu can be the obvious entry point into the API docs might be a good option. E.g. in the rust API everything is publicly exported into the root kuzu namespace`, but rust also has public/private designations for classes, variables etc.

I also suggest the following guidelines for referencing namespaces:

  1. If a name is used once or twice, just include the namespace inline when referring to the name.
  2. If a name is used many times, use using <namespace>::<Name>.
  3. Avoid using namespace ... (clang-tidy check, though it's probably too much work to go through them all and expand them out, so maybe leave the clang-tidy check for now).

This is something which is harder to enforce with QA tools (except number 3), so maybe we should start a coding style guide (maybe it's time to remove the old one on the wiki?) and add it to the developer section in the docs.

Those guidelines provide a soft barrier making it harder to write highly coupled code; if it seems like you're dealing with using and referring to namespaces too much, maybe the code is not structured very well and should be re-written. I think that we should avoid tightly coupled code across namespaces (with the idea that related and more tightly coupled code would be in the same namespace). In practice, I imagine there will be many exceptions to this.

ray6080 commented 1 week ago

We need to clean the main and common in the first place, perhaps quite a few classes/utilities should be moved into main if they're exposed as APIs.

so maybe we should start a coding style guide (maybe it's time to remove the old one on the wiki?) and add it to the developer section in the docs.

yeah. that old one in the wiki was perhaps out-of-dated already. we should draft a new one.