Definitions:

Honest Functions

all reads/writes are shown in the signature
fully controlled by the caller
local reasoning
testable

Dishonest Functions

reads/writes occur OUTSIDE the signature
out of the caller’s control

Dishonest functions introduce non-local reasoning and are not testable. They are ALSO infectious - any previously-honest function that calls a dishonest function is now-dishonest.

Composing Honesty and Dishonesty

If we imagine our program as a call-tree. Obviously, honest functions can only be at the leaves of the program.

Best Practices

Build the system out of honest functions and inject dishonesty at the topmost level.

See the following cpp code, that takes a string as input and prints every permutation:

int main(){
  string s; cin >> s;
  sort(s);
  do {
    println("{}", s);
  } while (
    next_permutation(s.begin(), s.end())
  );
}

We have honesty and dishonesty intermingled. The sort and the while-body are honest. The cin and println are dishonest. What we can do is refactor it:

// Our honest function
auto all_permutations(string s){
  sort(s);
  vector<string> all;
  do {
    all.push_back(s);
    println("{}", s);
  } while (
    next_permutation(s.begin(), s.end())
  );
  return all;
}

// our dishonest function -int main(){
  string s; cin >>s;
  println("{}", all_permutations(s));
}

We’d like this because we can fundamentally do console-io or file-io - the “input” to the all_permutations is not salient to the actual program. Alternatively, if we do not want to generate all the values and pass them back, we could use dependency injection and pass in a function that we’d like e.g. in a for-each pattern

The function signature should communicate and show empathy for callers.

In python either use kwargs, or generally, across languages, use a struct

set_timer(
  nullptr,
  "id",
  5,
  true,
  false,
  true
);

set_timer(
  SetTimerParams{
    .context=nullptr,
    .id="id",
    .interval=5,
    .delay=true,
    .loop=false,
    .retriggerable=true}
  }
);

Note this is very different from passing in an entire struct out of laziness.

Another example in communication is:

auto a()

// a must be called before b!!
auto b()

b now has a hidden input and is dependent on a

and it can instead be refactored to:

class ProofA{
  ProofA() = default;
  friend auto a() -> ProofA;
};

auto a() -> ProofA;
auto b(const ProofA&)

Every line in a function body should exist at the same level of abstraction.

Consider the following, which does 3 things:

convert name to lowercase
binary search for the asset
check for asset type if it is found

bool AssetManager::is_asset_of_type(
  string asset_name, AssetType type) const{
    // convert name to lower
    for (char& c: asset){
      if (c >= 'A' && c <= 'Z')
        c+=32;
    }

    // binary search for asset
    auto *a = assets.cbegin();
    for (auto* end = assets.cend(); a != end;){
      // hand-written raw binary search code...
    }

    // check if not found
    if (not a or asset_name < a->name)
      Application::exit();

    // check for given asset type
    return a->type == type;
  }

We can see that clearly 1) we are reinventing the wheel 2) we’re “stepping in” to the individual components then stepping out. To fix this, we can analyze the code and what they are doing:

1) find asset
1) convert name to lower
1.1) convert each char to lower
2) binary search for asset
2.1) find lower bound
2.2) check if match
2) check for asset type if found

breaking things down like this

bool AssetManager::is_asset_of_type(
  string asset_name, AssetType type) const{
    auto *a = this->find_asset(move(asset_name));
    if (not a) return {};
    return a->type == type;
}

auto * AssetManager::find_asset(
  string asset_name) const{
    tolower_inplace(asset_name);
    return this->search(asset_name);
  }

void tolower_inplace(string& s){
  ranges::transform(s, s.begin(),
    [] (unsigned char c){
      return tolower(c);
    }
  );
}

string tolower(string s){
  tolower_inplace(s);
  return s; // implicit move
}

// NOTE: `search` can just be a library call, if it can be formulated that way!

This is all good, but stepping further, there are a LOT of assumptions about this. For example, if we add an asset, we have to use the tolower_inplace and the binary search to determine where to insert it. This creates TIES between the functions and makes them brittle. We’re effectively maintaining an ad-hoc top-level datastructure. We can encapsulate that!

If we just store our assets in something like a CaseInsensitiveMap<> (that the top-level just does not need to know about), we can write it as:

bool AssetManager::is_asset_of_type(
  string_view asset_name, AssetType type) const{
    auto *a = this->assets_by_name.find(asset_name);
    return a && a->type == type;
}

Ian Quah