Initial CPP-SGL Draft - #113
Conversation
There was a problem hiding this comment.
This file should be removed before delivering the fully prepared/implemented module
For now move this file and the edge-scetch.pdf file to docs/sgl
There was a problem hiding this comment.
In general the library avoids such umrella headers for the entire module's content.
In GL and HGL there are the main graph.hpp and hypergraph.hpp file which include all the necessary stuff to operate on a graph and if a user wants to use the elements that are not essential (algorithms, topology generators, file io, etc.), they should include them separately
|
|
||
| namespace traits { | ||
|
|
||
| inline constexpr std::size_t min_hash_word_size = 8uz; |
There was a problem hiding this comment.
Is that platform-independent?
Maybe this should be something like sizeof(std::size_t) ???
| #include <concepts> | ||
| #include <cstddef> | ||
|
|
||
| namespace sgl { |
There was a problem hiding this comment.
Can be collapsed to sgl::traits
| using gl::traits::is_instantiation_of_v; | ||
|
|
||
| template <typename T> | ||
| concept c_weight_type = c_arithmetic<T> and std::floating_point<T>; |
There was a problem hiding this comment.
std::floating_point is a more strict requirement than c_arithmetic (which is higly general)
so either you want to restrict the weight types to only floating point types or you want to allow it to be an arbitrary arithmetic type (floating point, integral or custom) - but choose one
then if you have only one requirement there is no need to add a dedicated concept (unless you want to make it explicit for readability) (same for c_uniform_type)
| return this->_inv_ids.at(idx); | ||
| } | ||
|
|
||
| [[nodiscard]] view_type view_at(const size_type idx) { |
There was a problem hiding this comment.
Try using the C++23 explicit object parameter to avoid having two versions of the same method only to satisfy the const/non-const semantics
[[nodiscard]] <conditional-type-or-auto> view_at(this auto& Self, const size_type idx) {
self._verify_idx(idx);
// ...
}
The same should be done for other const/non-const methods
| }; | ||
|
|
||
| template <traits::c_sketch_traits Traits, typename Key> | ||
| void update_es( |
There was a problem hiding this comment.
update_edge_scetch - do not abbreviate like that
| #include <stdexcept> | ||
| #include <vector> | ||
|
|
||
| namespace sgl { |
There was a problem hiding this comment.
- Could be collapsed to sgl::detail
- If this entire file is in the detail namespace, it should be placed in a detail directory (and probably renamed to update_edge_scetch or edge_scetch_util or something)
| return this->_registry.vertex_ids(); | ||
| } | ||
|
|
||
| [[nodiscard]] gl_attr_force_inline view_type sketch_of(const id_type id) { |
There was a problem hiding this comment.
As in registry.hpp - should use explicit object parameter
Same for the registry method
There was a problem hiding this comment.
There should be more tests (for the remaining functionality, not only the scetch type)
2 participants
No description provided.