https://github.com/bloomberg/blazingmq/blob/main/src/applications/bmqstoragetool/m_bmqstoragetool_parameters.h
This class has a few places that might be improved:
- Minimize includes
These includes are not needed in header, since they are not used there:
// MQB
#include <mqbs_datafileiterator.h>
#include <mqbs_filestoreprotocol.h>
#include <mqbs_journalfileiterator.h>
#include <mqbs_mappedfiledescriptor.h>
// BMQ
#include <bmqu_stringutil.h>
Other includes (in BDE section) might also be unused in the header.
If they are not used, they should be removed.
It is possible that other headers rely on includes in this file, so they should be updated to include needed directly.
- There is a block of constants declared in the header. These constants are only used internally in
cpp file and should be removed from the class API:
/// Record types constants
static const char* k_ALL_TYPE;
static const char* k_MESSAGE_TYPE;
static const char* k_QUEUEOP_TYPE;
static const char* k_JOURNALOP_TYPE;
static const char* k_CSL_ALL_TYPE;
static const char* k_CSL_SNAPSHOT_TYPE;
static const char* k_CSL_UPDATE_TYPE;
static const char* k_CSL_COMMIT_TYPE;
static const char* k_CSL_ACK_TYPE;
/// Print modes constants
static const char* k_HUMAN_MODE;
static const char* k_JSON_PRETTY_MODE;
static const char* k_JSON_LINE_MODE;
The constants should go to unnamed namespace in its cpp file.
Also, the type should be changed to const bsl::string_view, for example (other existing constant in the codebase):
/// Control Sequence Introducer
const bsl::string_view k_CSI = "\x1b[";
- Classes
Parameters, CommandLineArguments declared in the source miss allocator traits. Should have traits similar to (other existing example in the codebase):
// TRAITS
BSLMF_NESTED_TRAIT_DECLARATION(CslSearchOffsetDecorator,
bslma::UsesBslmaAllocator)
- Class API accepts allocator on construction.
However, many methods also require allocator, for example:
bool validate(bsl::string* error, bslma::Allocator* allocator = 0);
void validateCslModeArgs(bsl::ostream& stream,
bslma::Allocator* allocator = 0);
void validateQueueNames(bslma::Allocator* allocator = 0) const;
A more clear way is to keep d_allocator_p field in each class that require it, init it in constructor and use it in the corresponding methods. Allocator arg can be removed from them after this.
https://github.com/bloomberg/blazingmq/blob/main/src/applications/bmqstoragetool/m_bmqstoragetool_parameters.h
This class has a few places that might be improved:
These includes are not needed in header, since they are not used there:
Other includes (in BDE section) might also be unused in the header.
If they are not used, they should be removed.
It is possible that other headers rely on includes in this file, so they should be updated to include needed directly.
cppfile and should be removed from the class API:The constants should go to unnamed namespace in its
cppfile.Also, the type should be changed to
const bsl::string_view, for example (other existing constant in the codebase):Parameters,CommandLineArgumentsdeclared in the source miss allocator traits. Should have traits similar to (other existing example in the codebase):However, many methods also require allocator, for example:
A more clear way is to keep
d_allocator_pfield in each class that require it, init it in constructor and use it in the corresponding methods. Allocator arg can be removed from them after this.