fbow icon indicating copy to clipboard operation
fbow copied to clipboard

improved const-correctness

Open jbfuehrer opened this issue 6 years ago • 2 comments

I tried to improve the (logical) const-correctness of the library as methods such as Vocabulary::transform don't modify any internal data from the library user's perspective(*). Since it's not const-qualified however, it enforces its non-constness on user methods which then snowballs further from there and has potential to makes code somewhat unclean.

I therefore templated the internal Block structure on a type that's either char or const char (could further be enforced via type traits) and then conditionally const-qualified all accesses to the internal block structure depending on whether or not the type itself was templated on a const type on construction (which is enforced in const methods, see below).

This results in this being valid:

void MyClass::foo()
{
  Block<char> block=getBlock(0);
  block.setLeaf(true);
  block.nonConstMethod();
  block.constMethod();
}

void MyClass::foo() const
{
  Block<const char> block=getBlock(0);
  bool is_leaf = block.isLeaf();
  block.constMethod();
}

while any of these don't compile

void MyClass::foo() const
{
  Block<char> block = getBlock(0);  // error: getBlock returns Block<const char>
  Block<const char> block=getBlock(0);  // ok
  block.setLeaf(true);  // error: can't modify through const object
  block.nonConstMethod();  // same

  auto* binfo = block.getBlockNodeInfo(0);
  binfo->nonConstMethod();  // error: constness is also forwarded to BlockNodeInfo
}

C++17 can even automatically deduce Block's template type from the ctor argument but since I'm not sure what the requirements for the library are, I didn't rely on this. Can you please tell me which platforms and/or language standard you are offering to support so I can make sure it compiles for these as well?

(*) The one exception to this rule is the cpu_info but since this is an implementation detail that imo should be transparent to the user and not affect (logical) constness of the API interface, I qualified it as mutable.

jbfuehrer avatar Mar 18 '19 17:03 jbfuehrer

Hi, I need the library to be available as many platforms and compilers as possible. So far I ahve tested (gcc with c++11, clang, msvc)

rmsalinas avatar Mar 25 '19 05:03 rmsalinas

Well, this is all standard cpp so it's rather a question of what the lowest standard version is you'd like to support. If you'd still like to support C++11 I can rewrite the C++14 parts.

jbfuehrer avatar Apr 16 '19 18:04 jbfuehrer