Functional Refactoring in C++

Consider the following C++ code snippet:

  1. bool UNIVERSE::Initialize(const std::string & assetpath,
  2. const std::string & shippath,
  3. const std::string & behaviorpath,
  4. MODELMANAGER & modelmanager,
  5. std::ostream & info_output, std::ostream & error_output)
  6. {
  7. if (LoadAssets(assetpath,
  8. modelmanager,
  9. error_output))
  10. {
  11. info_output << "Loaded universe assets successfully" << std::endl;
  12. }
  13. else
  14. {
  15. error_output << "Error loading universe assets" << std::endl;
  16. return false;
  17. }
  19. if (behaviors.Load(behaviorpath,
  20. error_output))
  21. {
  22. info_output << "Loaded AI behaviors successfully" << std::endl;
  23. }
  24. else
  25. {
  26. error_output << "Error loading AI behaviors" << std::endl;
  27. return false;
  28. }
  30. ... etc ...
  32. return true;
  33. }

We have a function that performs some initialization and returns true on success or false if it fails. Internally the function calls various loading functions. There's some obvious code duplication here; for each load function we call we do almost exactly the same thing to handle success or failure. In the future we might want to change what we do on success or failure, and it'd be nice to only have to change the code in one place instead of after every load call. How can we remove this duplication?

We could make OnLoadSuccess and OnLoadFailure functions and use them like so:

  1. if (behaviors.Load(behaviorpath,
  2. error_output))
  3. {
  4. OnLoadSuccess(info_output, "AI behaviors");
  5. }
  6. else
  7. {
  8. OnLoadFailure(error_output, "AI behaviors");
  9. return false;
  10. }

This is better, but there's still some duplication and it's not as generic as it could be; for example, if we wanted to display a "retry" message box to the user and allow them to retry a particular load step, we couldn't do that without changing all of the places where we called OnLoad*. We can go further.

We could make some variadic LOADFUNCTION macro, but that could get pretty ugly.

We also could refactor all of our loading functions to conform to some standard interface so we could call them in a loop. That's not a bad way to go, but it turns out that we can get similar genericity without having to do that.

In functional programming, it's very common to pass around functions, and it's also easy to do partial function application (currying), which means taking a function object with multiple arguments and supplying some of those arguments, yielding a new function (called a closure) that takes only the arguments you didn't supply. We can do something similar with the TR1 functional extensions. The std::tr1::function class lets us use function objects, and the std::tr1::bind function lets us create something like a closure. We can bind all of the necessary arguments to our load functions and then pass them to a GenericLoad function as a zero-argument function object that returns a boolean. Here's what this looks like:

  1. bool GenericLoad(std::ostream & info_output,
  2. std::ostream & error_output,
  3. const std::string & name,
  4. std::tr1::function <bool ()> f)
  5. {
  6. if (f())
  7. {
  8. info_output << "Loaded " << name << std::endl;
  9. return true;
  10. }
  11. else
  12. {
  13. error_output << "Error loading " << name << std::endl;
  14. return false;
  15. }
  16. }
  18. bool UNIVERSE::Initialize(const std::string & assetpath,
  19. const std::string & shippath,
  20. const std::string & behaviorpath,
  21. MODELMANAGER & modelmanager,
  22. std::ostream & info_output, std::ostream & error_output)
  23. {
  24. if (!GenericLoad(info_output, error_output, "universe assets",
  25. std::tr1::bind(&UNIVERSE::LoadAssets, this,
  26. assetpath,
  27. modelmanager,
  28. std::tr1::reference_wrapper<std::ostream>(error_output))))
  29. return false;
  31. if (!GenericLoad(info_output, error_output, "AI behaviors",
  32. std::tr1::bind(&behaviortree::TREES::Load, &behaviors,
  33. behaviorpath,
  34. std::tr1::reference_wrapper<std::ostream>(error_output))))
  35. return false;
  37. ... etc ...
  39. return true;
  40. }

Is this a win? We've removed a bunch of redundant code and made our loading much more generic. The syntax, though, is absolutely horrible. In languages with first-class functions and a lot of syntactic sugar (like Haskell), the syntax is often neater for the case where we use the generic loader. In C++, an argument could be made that the generic loader is actually harder to maintain in some cases due solely to how confusing and muddled the syntax is.

Comments are closed.

Dev Journal and Project Hosting