Skip to content

Commit

Permalink
Adopt some aspects of Google's C++ style guide. (#97)
Browse files Browse the repository at this point in the history
- All class member variables are now prefixed with 'my_' to avoid confusion
  with argument parameters, particularly in the constructor and init list.
- 'struct' is now reserved for simple data carriers. If it does any
  calculations, it should be a 'class' instead. 
- Prefer composition over some unnecessary inheritance for code re-use.

A side effect of the member variable rule is that our constructors can now use
more descriptive argument names (that were previously truncated to avoid
confusion in the initialization list). So I went and gave every class some
better names, along with some of the functions as well.
  • Loading branch information
LTLA authored May 17, 2024
1 parent c4edffb commit dc42289
Show file tree
Hide file tree
Showing 37 changed files with 3,174 additions and 2,855 deletions.
56 changes: 30 additions & 26 deletions include/tatami/base/Extractor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ namespace tatami {
* @brief Extract an element of the target dimension in dense form without an oracle.
*/
template<typename Value_, typename Index_>
struct MyopicDenseExtractor {
class MyopicDenseExtractor {
public:
/**
* `buffer` may not necessarily be filled upon extraction if a pointer can be returned to the underlying data store.
* This can be checked by comparing the returned pointer to `buffer`; if they are the same, `buffer` has been filled.
Expand Down Expand Up @@ -68,7 +69,8 @@ struct MyopicDenseExtractor {
* @brief Extract an element of the target dimension in dense form with an oracle.
*/
template<typename Value_, typename Index_>
struct OracularDenseExtractor {
class OracularDenseExtractor {
public:
/**
* `buffer` may not necessarily be filled upon extraction if a pointer can be returned to the underlying data store.
* This can be checked by comparing the returned pointer to `buffer`; if they are the same, `buffer` has been filled.
Expand Down Expand Up @@ -135,27 +137,28 @@ struct OracularDenseExtractor {
* @brief Extract an element of the target dimension in sparse form without an oracle.
*/
template<typename Value_, typename Index_>
struct MyopicSparseExtractor {
class MyopicSparseExtractor {
public:
/**
* `vbuffer` may not necessarily be filled upon extraction if a pointer can be returned to the underlying data store.
* This be checked by comparing the returned `SparseRange::value` pointer to `vbuffer`;
* if they are the same, `vbuffer` has been filled with `SparseRange::number` values.
* The same applies for `ibuffer` and the returned `SparseRange::index` pointer.
* `value_buffer` may not necessarily be filled upon extraction if a pointer can be returned to the underlying data store.
* This be checked by comparing the returned `SparseRange::value` pointer to `value_buffer`;
* if they are the same, `value_buffer` has been filled with `SparseRange::number` values.
* The same applies for `index_buffer` and the returned `SparseRange::index` pointer.
*
* If `Options::sparse_extract_value` was set to `false` during construction of this instance,
* `vbuffer` is ignored and `SparseRange::value` is set to `NULL` in the output.
* `value_buffer` is ignored and `SparseRange::value` is set to `NULL` in the output.
* Similarly, if `Options::sparse_extract_index` was set to `false` during construction of this instance,
* `ibuffer` is ignored and `SparseRange::index` is set to `NULL` in the output.
* `index_buffer` is ignored and `SparseRange::index` is set to `NULL` in the output.
*
* @param i Index of the target dimension element, i.e., the row or column index.
* @param[out] vbuffer Pointer to an array with enough space for at least `N` values,
* @param[out] value_buffer Pointer to an array with enough space for at least `N` values,
* where `N` is defined as described for `MyopicDenseExtractor::fetch()`.
* @param[out] ibuffer Pointer to an array with enough space for at least `N` indices,
* @param[out] index_buffer Pointer to an array with enough space for at least `N` indices,
* where `N` is defined as described for `MyopicDenseExtractor::fetch()`.
*
* @return A `SparseRange` object describing the contents of the `i`-th dimension element.
*/
virtual SparseRange<Value_, Index_> fetch(Index_ i, Value_* vbuffer, Index_* ibuffer) = 0;
virtual SparseRange<Value_, Index_> fetch(Index_ i, Value_* value_buffer, Index_* index_buffer) = 0;

/**
* @cond
Expand Down Expand Up @@ -187,28 +190,29 @@ struct MyopicSparseExtractor {
* @brief Extract an element of the target dimension in sparse form with an oracle.
*/
template<typename Value_, typename Index_>
struct OracularSparseExtractor {
class OracularSparseExtractor {
public:
/**
* `vbuffer` may not necessarily be filled upon extraction if a pointer can be returned to the underlying data store.
* This be checked by comparing the returned `SparseRange::value` pointer to `vbuffer`;
* if they are the same, `vbuffer` has been filled with `SparseRange::number` values.
* The same applies for `ibuffer` and the returned `SparseRange::index` pointer.
* `value_buffer` may not necessarily be filled upon extraction if a pointer can be returned to the underlying data store.
* This be checked by comparing the returned `SparseRange::value` pointer to `value_buffer`;
* if they are the same, `value_buffer` has been filled with `SparseRange::number` values.
* The same applies for `index_buffer` and the returned `SparseRange::index` pointer.
*
* If `Options::sparse_extract_value` was set to `false` during construction of this instance,
* `vbuffer` is ignored and `SparseRange::value` is set to `NULL` in the output.
* `value_buffer` is ignored and `SparseRange::value` is set to `NULL` in the output.
* Similarly, if `Options::sparse_extract_index` was set to `false` during construction of this instance,
* `ibuffer` is ignored and `SparseRange::index` is set to `NULL` in the output.
* `index_buffer` is ignored and `SparseRange::index` is set to `NULL` in the output.
*
* @param[out] vbuffer Pointer to an array with enough space for at least `N` values,
* @param[out] value_buffer Pointer to an array with enough space for at least `N` values,
* where `N` is defined as described for `MyopicDenseExtractor::fetch()`.
* @param[out] ibuffer Pointer to an array with enough space for at least `N` indices,
* @param[out] index_buffer Pointer to an array with enough space for at least `N` indices,
* where `N` is defined as described for `MyopicDenseExtractor::fetch()`.
*
* @return A `SparseRange` object describing the contents of the next element of the target dimension,
* as predicted by the `Oracle` used to construct this instance.
*/
SparseRange<Value_, Index_> fetch(Value_* vbuffer, Index_* ibuffer) {
return fetch(0, vbuffer, ibuffer);
SparseRange<Value_, Index_> fetch(Value_* value_buffer, Index_* index_buffer) {
return fetch(0, value_buffer, index_buffer);
}

/**
Expand All @@ -223,15 +227,15 @@ struct OracularSparseExtractor {
* given that the value of `i` is never actually used.
*
* @param i Ignored, only provided for consistency with `MyopicSparseExtractor::fetch()`.
* @param[out] vbuffer Pointer to an array with enough space for at least `N` values,
* @param[out] value_buffer Pointer to an array with enough space for at least `N` values,
* where `N` is defined as described for `MyopicDenseExtractor::fetch()`.
* @param[out] ibuffer Pointer to an array with enough space for at least `N` indices,
* @param[out] index_buffer Pointer to an array with enough space for at least `N` indices,
* where `N` is defined as described for `MyopicDenseExtractor::fetch()`.
*
* @return A `SparseRange` object describing the contents of the next dimension element,
* as predicted by the `Oracle` used to construct this instance.
*/
virtual SparseRange<Value_, Index_> fetch(Index_ i, Value_* vbuffer, Index_* ibuffer) = 0;
virtual SparseRange<Value_, Index_> fetch(Index_ i, Value_* value_buffer, Index_* index_buffer) = 0;

/**
* @cond
Expand Down
3 changes: 2 additions & 1 deletion include/tatami/base/Oracle.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ namespace tatami {
* Check out `ConsecutiveOracle` and `FixedVectorOracle` for some examples of concrete subclasses.
*/
template<typename Index_>
struct Oracle {
class Oracle {
public:
/**
* @cond
*/
Expand Down
24 changes: 13 additions & 11 deletions include/tatami/base/SparseRange.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,39 +25,41 @@ namespace tatami {
* If zeroes are explicitly initialized in the underlying structure, they will be reported here.
* However, one can safely assume that all indices _not_ reported in `index` have values of zero.
*
* @tparam Value Data value type, should be numeric.
* @tparam Index Row/column index type, should be integer.
* @tparam Value_ Data value type, should be numeric.
* @tparam Index_ Row/column index type, should be integer.
*/
template <typename Value, typename Index>
template <typename Value_, typename Index_>
struct SparseRange {
/**
* @param n Number of structural non-zero values.
* @param v Pointer to the values. This should have at least `n` addressible elements.
* @param i Pointer to the indices. This should have at least `n` addressible elements.
* @param number Number of structural non-zero values.
* @param value Pointer to the values.
* This should have at least `number` addressible elements.
* @param index Pointer to the indices.
* This should have at least `number` addressible elements.
*/
SparseRange(Index n, const Value* v=NULL, const Index* i=NULL) : number(n), value(v), index(i) {}
SparseRange(Index_ number, const Value_* value = NULL, const Index_* index = NULL) : number(number), value(value), index(index) {}

/**
* Default constructor.
*/
SparseRange() {}
SparseRange() = default;

/**
* Number of structural non-zero elements.
*/
Index number = 0;
Index_ number = 0;

/**
* Pointer to an array containing the values of the structural non-zeros.
* If non-`NULL`, this has at least `number` addressible entries.
*/
const Value* value = NULL;
const Value_* value = NULL;

/**
* Pointer to an array containing the indices of the structural non-zeros.
* If non-`NULL`, this has at least `number` addressible entries.
*/
const Index* index = NULL;
const Index_* index = NULL;
};

}
Expand Down
Loading

0 comments on commit dc42289

Please sign in to comment.