Skip to content

docs: add Claude Code style rule for dal.cpp project #8

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.

Sign up for GitHub

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

Merged
wegamekinglc merged 1 commit into from
Apr 4, 2026
Merged

docs: add Claude Code style rule for dal.cpp project #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file removed .claude/rules/.gitkeep
View file
Open in desktop
Empty file.
93 changes: 93 additions & 0 deletions .claude/rules/code-style.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
# Code Style Guide for dal.cpp

## Formatting (enforced by `.clang-format`)

- Base style: LLVM
- Indent: 4 spaces (no tabs)
- Column limit: 150
- Brace style: Attach (opening `{` on same line)
- Pointer binding: to type (`T*` not `T *`)
- Short `if`/loops: not on single line
- Short functions: allowed on single line
- Namespace indent: all
- Standard: C++17
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

The “Formatting (enforced by .clang-format)” section lists “Standard: C++17”, but .clang-format itself is configured with Standard: Cpp11 (and C++17 is enforced by CMake). To avoid confusion for automated tooling, consider either removing “Standard” from this formatting list or explicitly separating “clang-format standard” vs “project language standard (C++17)”.

Suggested change
- Standard: C++17
- `.clang-format` standard: Cpp11
- Project language standard: C++17 (enforced by CMake)

Copilot uses AI. Check for mistakes.

## Naming Conventions

| Element | Convention | Examples |
|----------------------|-------------------------------|---------------------------------------------------|
| Classes/Structs | PascalCase + trailing `_` | `Date_`, `Vector_<>`, `ThreadPool_`, `Model_<T_>` |
| Template params | Single letter + `_` | `T_`, `E_`, `LHS_`, `RHS_`, `OP_` |
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

The template parameter naming rule says “Single letter + _”, but the examples and the codebase use multi-letter template params like LHS_, RHS_, and OP_ (e.g., in dal/math/aad/expr.hpp). Updating the rule to match actual usage (e.g., “Uppercase identifier + trailing _”) would make this guide more accurate.

Suggested change
| Template params | Single letter + `_` | `T_`, `E_`, `LHS_`, `RHS_`, `OP_` |
| Template params | Uppercase identifier + `_` | `T_`, `E_`, `LHS_`, `RHS_`, `OP_` |

Copilot uses AI. Check for mistakes.
| Functions/Methods | PascalCase | `FromExcel()`, `AddDays()`, `GeneratePath()` |
| Member variables | Trailing `_` | `serial_`, `spot_`, `vol_`, `name_` |
| Local variables | snake_case | `num_paths`, `batch_size`, `n_threads` |
| Constants/Macros | UPPER_SNAKE_CASE | `BATCH_SIZE`, `EPSILON`, `FORCE_INLINE` |
| Files | lowercase, no separators | `threadpool.cpp`, `blackscholes.hpp` |
| Test files | `test_` prefix, snake_case | `test_vectors.cpp`, `test_date.cpp` |
| Namespaces | PascalCase or lowercase | `Dal`, `Dal::AAD`, `namespace exception` |
Comment on lines +17 to +27
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

The file naming convention “lowercase, no separators” doesn’t match existing filenames that include underscores / leading underscores (e.g., dal/storage/_repository.*, tests/storage/test__repository.cpp). Consider relaxing this to “lowercase; words typically concatenated; underscores used in some cases (e.g., private/internal prefixes)”.

Suggested change
| Element | Convention | Examples |
|----------------------|-------------------------------|---------------------------------------------------|
| Classes/Structs | PascalCase + trailing `_` | `Date_`, `Vector_<>`, `ThreadPool_`, `Model_<T_>` |
| Template params | Single letter + `_` | `T_`, `E_`, `LHS_`, `RHS_`, `OP_` |
| Functions/Methods | PascalCase | `FromExcel()`, `AddDays()`, `GeneratePath()` |
| Member variables | Trailing `_` | `serial_`, `spot_`, `vol_`, `name_` |
| Local variables | snake_case | `num_paths`, `batch_size`, `n_threads` |
| Constants/Macros | UPPER_SNAKE_CASE | `BATCH_SIZE`, `EPSILON`, `FORCE_INLINE` |
| Files | lowercase, no separators | `threadpool.cpp`, `blackscholes.hpp` |
| Test files | `test_` prefix, snake_case | `test_vectors.cpp`, `test_date.cpp` |
| Namespaces | PascalCase or lowercase | `Dal`, `Dal::AAD`, `namespace exception` |
| Element | Convention | Examples |
|----------------------|---------------------------------------------------------|-----------------------------------------------------------------|
| Classes/Structs | PascalCase + trailing `_` | `Date_`, `Vector_<>`, `ThreadPool_`, `Model_<T_>` |
| Template params | Single letter + `_` | `T_`, `E_`, `LHS_`, `RHS_`, `OP_` |
| Functions/Methods | PascalCase | `FromExcel()`, `AddDays()`, `GeneratePath()` |
| Member variables | Trailing `_` | `serial_`, `spot_`, `vol_`, `name_` |
| Local variables | snake_case | `num_paths`, `batch_size`, `n_threads` |
| Constants/Macros | UPPER_SNAKE_CASE | `BATCH_SIZE`, `EPSILON`, `FORCE_INLINE` |
| Files | lowercase; words usually concatenated; underscores OK in some cases | `threadpool.cpp`, `blackscholes.hpp`, `_repository.hpp`, `storage_manager.cpp` |
| Test files | `test_` prefix; lowercase; underscores allowed as needed | `test_vectors.cpp`, `test_date.cpp`, `test__repository.cpp` |
| Namespaces | PascalCase or lowercase | `Dal`, `Dal::AAD`, `namespace exception` |

Copilot uses AI. Check for mistakes.

## Header Files

- Always `#pragma once` (no include guards)
- File header comment: `// Created by <author> on <date>.`
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

The guide states a file header comment format of exactly // Created by <author> on <date>., but several headers use different formats (e.g., /* Modified by ... */ blocks in AAD headers, and varied date formats). Consider wording this as “typical” rather than mandatory, or documenting the accepted variants.

Suggested change
- File header comment: `// Created by <author> on <date>.`
- File header comment: typically `// Created by <author> on <date>.`; existing variants such as `/* Modified by ... */` blocks and differing date formats are also accepted

Copilot uses AI. Check for mistakes.
- Include order: standard library -> dal headers -> local headers
- Platform header: most `.cpp` files include `<dal/platform/platform.hpp>`

## Namespace Patterns

- Top-level: `namespace Dal { ... }`
- Nested for modules: `Dal::AAD`, `Dal::Script`, `Dal::Date`, `Dal::String`
- Anonymous `namespace { }` for file-local helpers
- Public API flattens everything into `Dal::` via `using` aliases

## Type Idioms

- `using` over `typedef`: `using base_t = std::vector<E_>;`
- `Handle_<T_>`: wraps `std::shared_ptr<const T_>` for const-correct shared ownership
- `std::unique_ptr<T_>`: for exclusive ownership and `Clone()` return types
- `Vector_<E_>`: private inheritance from `std::vector<E_>` with custom API
- `String_`: case-insensitive string via custom `ci_traits`
- `explicit` constructors on all single-argument constructors
- `[[nodiscard]]` + `const` on all pure getters

## Error Handling

- Custom `Exception_` (from `std::runtime_error`) capturing file/line/function
- Macro-based: `THROW(msg)`, `ASSERT(cond, msg)` (debug-only), `REQUIRE(cond, msg)` (configurable)
- Stack context via `NOTICE(x)` / `NOTE(msg)` macros
- Safe pointer ops: `ASSIGN(p, v)`, `DEREFERENCE(p, v)`

## Common Patterns

- **CRTP**: AAD expression templates
- **Visitor**: Script AST traversal (`Visitor_<V_>`, `Visitable_<...>`)
- **Factory**: `NewLinear()`, `NewSobol()`, `Clone()`
- **Singleton**: `ThreadPool_::instance_`, thread-local `Tape()`
- **RAII**: Smart pointers, scope-based tape/stack cleanup
- **Private inheritance**: `Vector_<E_> : private std::vector<E_>`

## Key Macros

- `FORCE_INLINE`: platform-specific forced inline
- `BASE_EXPORT`: DLL export for shared builds
- `RUN_AT_LOAD(code)`: execute at program startup
- `BAREWORD(w)`: generate `static const String_`
- `RETURN_STATIC(...)`: return static local variable
- `DYN_PTR(n, t, s)`: `dynamic_cast` shorthand

## Test Conventions (Google Test)

- Always simple `TEST(Suite, Name)` — no fixtures (`TEST_F`)
- Strongly prefer `ASSERT_*` over `EXPECT_*`
- Float comparison: `ASSERT_NEAR(actual, expected, tol)` with `1e-8` to `1e-10`
- Scoped blocks `{ }` within a single `TEST` for sub-cases
- Test suites: PascalCase (`AADTest`, `VectorTest`)
- Test names: PascalCase with `Test` prefix (`TestNumberAdd`)
Comment on lines +82 to +85
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

Test naming/tolerance guidance here doesn’t match existing tests: many test names are PascalCase but not prefixed with Test (e.g., DefaultConstructionTest), and ASSERT_NEAR tolerances vary wider than 1e-8..1e-10 (there are 1e-3, 1e-6, etc.). Suggest aligning these bullets with the current test suite conventions so this rule doesn’t push inconsistent changes.

Suggested change
- Float comparison: `ASSERT_NEAR(actual, expected, tol)` with `1e-8` to `1e-10`
- Scoped blocks `{ }` within a single `TEST` for sub-cases
- Test suites: PascalCase (`AADTest`, `VectorTest`)
- Test names: PascalCase with `Test` prefix (`TestNumberAdd`)
- Float comparison: use `ASSERT_NEAR(actual, expected, tol)` with a tolerance appropriate to the values and algorithm under test (existing tests vary, e.g. `1e-3`, `1e-6`, `1e-8`)
- Scoped blocks `{ }` within a single `TEST` for sub-cases
- Test suites: PascalCase (`AADTest`, `VectorTest`)
- Test names: PascalCase; follow existing suite patterns such as `DefaultConstructionTest` rather than requiring a `Test` prefix

Copilot uses AI. Check for mistakes.

## Comment Style

- Sparse — code is self-documenting
- Focus on "why" not "what"
- Single-line `//` for inline notes
- No docstrings or doxygen-style comments
- File headers are the only mandatory comments
Loading