3
\$\begingroup\$

I am exploring functional programming design with C++. The code below uses a struct passed to ranges::fold_left for processing. The final result is returned in the struct.

**Updated to remove a toy version of code since full code was requested and supplied

Is this an appropriate functional programming design? Specifically, using LineTotal appropriate to maintain state information through the interactions of fold_left used in extract_values?


#include <algorithm>
#include <iomanip>
#include <iostream>
#include <ranges>

#include "../AdventCpp.h"

namespace rng = std::ranges;
namespace vws = std::views;

//-------------------------------------------------------
execFunc execute = [ ](std::string_view&& aoc_data) noexcept -> uint64_t {
   using vws::slide, vws::split, rng::fold_left, //
      vws::take, vws::zip, vws::filter;
   using std::string_view, std::string;

   auto summation = [ ](uint64_t sum, auto&& aoc_dual) noexcept -> uint64_t {

      auto top_line = string(string_view(aoc_dual.front()));
      auto bottom_line = string(string_view(aoc_dual.next().front()));

      // change dots to spaces so ispunct doesn't see dots
      rng::replace(top_line, '.', ' ');
      rng::replace(bottom_line, '.', ' ');

      // generate container with pairs of chars from the two lines of data
      auto zip_lines = zip(top_line, bottom_line);

      // change dots to spaces so ispunct doesn't see dots
      auto two_line_proc = [ ](uint64_t sum, auto&& lines) {

         // some structs to pass data through fold_left
         struct LineState {
            uint64_t digits_sum { };
            bool have_symbol { };
         };
         struct LineTotal {
            uint64_t total { };
            LineState top_data;
            LineState bot_data;
         };

         auto out_sd = [&](char const tag, LineTotal s_d) {
            auto [total, top, bot] = s_d;
            std::cout << tag << '\t' << total << '\t' //
                      << top.digits_sum << '\t' << top.have_symbol << '\t'
                      << bot.digits_sum << '\t' << bot.have_symbol << '\n';
         };

         // is the value valid with symbols tagging it?
         auto is_value = [ ](uint64_t value, bool symbol) noexcept -> uint64_t {
            return (value != 0 and symbol) ? value : 0;
         };

         // calculate running value of digits from a line
         auto calc_value = [ ](uint64_t value, char ch) noexcept -> uint64_t {
            if (isdigit(ch)) {
               value = value * 10 + (ch - '0');
            }
            return value;
         };
         auto extract_values =
            [&is_value, &calc_value](LineTotal sum_data, auto&& two_char) {
               auto [top_char, bot_char] = two_char;
               auto& [total, top, bottom] = sum_data;

               if (top_char == ' ' and ispunct(bot_char)) {
                  total += bottom.digits_sum;
                  total += top.digits_sum;

                  top = {0, true};
                  bottom = {0, true};
               } else if (bot_char == ' ' and ispunct(top_char)) {
                  top = {top.digits_sum, true};
                  total += bottom.digits_sum;

                  bottom = {0, bottom.have_symbol};
               } else if (top_char == ' ' and bot_char == ' ') {
                  total += is_value(top.digits_sum, bottom.have_symbol);
                  total += is_value(bottom.digits_sum,
                                    top.have_symbol or bottom.have_symbol);

                  top = { };
                  bottom = { };
               } else {
                  top = {calc_value(top.digits_sum, top_char),
                         top.have_symbol or ispunct(top_char)};

                  bottom = {calc_value(bottom.digits_sum, bot_char),
                            bottom.have_symbol or ispunct(bot_char)};
               }

               return sum_data;
            };

         // process two lines of aoc_data
         return sum + fold_left(lines, LineTotal { }, extract_values).total;
      };

      return two_line_proc(sum, zip_lines);
   };

   return fold_left(aoc_data | split('\n') | slide(2), 0ul, summation);
};

int main() {

   advent_cpp(test_data1, aoc_data, execute, 4361); // 4361 / 528799
   return 0;
}
\$\endgroup\$
1
  • \$\begingroup\$ The current question title, which states your concerns about the code, is too general to be useful here. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles. \$\endgroup\$ Commented Jan 3 at 16:44

1 Answer 1

4
\$\begingroup\$

It's not pure functional programming

There are different ways to think about functional programming. Is it just a tool in your programming toolbox? Is it something you should strive towards? Or should you write something that is embracing the function programming paradigm 100%? You are using a functional programming style a lot, but there are statements that are definitely not FP, like:

rng::replace(top_line, '.', ' ');

This modifies a std::string in-place, which means this statement has side-effects. If you want to make this functional, I'd use something like std::ranges::transform_view():

auto top_line_no_dots =
    rng::transform_view(top_line, [](auto c){return c == '.' ? ' ' : c;});

Similarly, you define lambdas that capture by reference, so they technically are functions with side-effects, again that's also not very FP. Even just adding something to an existing variable can be considered to be non-functional. In functional programming languages you then have to jump through hoops to do that anyway. I am sure you can rewrite your code to be completely functional; it's all doable in C++, although it lacks the concise notation of functional programming languages in this area.

You single out std::ranges::fold_left() in your question, but that's just a tool in your toolbox. It helps write functional code, but the mere fact of using it doesn't make your code functional per se. Also, internally it's implemented using the good old imperative for-loop, make of that what you will. You did write it in a way that doesn't have side effects, so in that regard its use here is indeed pure functional.

Pass std::string_views by value

You let execute() take a std::string_view by r-value reference, but that is unnecessary; just take it by value instead.

Unnecessary use of lambdas

Sure, functional programming languages like their lambda functions. But a lambda function assigned to a variable with a name is pretty much just a regular function. The only time that isn't the case is when you have captures. So consider declaring execute() as a regular function. You could also define some of the other lamdas as regular functions.

Then there is this pattern:

auto two_line_proc = [ ](uint64_t sum, auto&& lines) {
    // Do some stuff
    …
    return sum + fold_left(lines, LineTotal { }, extract_values).total;
};

return two_line_proc(sum, zip_lines);

You could instead immediately execute the lambda:

return [ ](uint64_t sum, auto&& lines) {
    // Do some stuff
    …
    return sum + fold_left(lines, LineTotal { }, extract_values).total;
}(sum, zip_lines);

But why use a lambda at all? You can just write:

// Do some stuff
…
return sum + fold_left(lines, LineTotal { }, extract_values).total;

Naming things

I have a hard time understanding what the code is supposed to do. I'm guessing it's one of the Advent of Code problems. But which day? What is the problem? Neither the comments nor the names of functions and variables give me any hints.

Try to give better names to functions and variables such that their meaning becomes more obvious to someone who isn't already familiar with the exact problem you are trying to solve. Use comments to fill in where you can't with just the function and variable names.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you. One reason I posted the toy version was that the full code wasn't ready for review. I probably wouldn't have caught all your suggestions, though. Yes, this is Advent of Code, Day 3, Part 1. I'm using the AOC problems to hone my FP capabilities. Since you didn't object to the use of "LineTotal" with "fold_left" I assume it is appropriate to use a struct that is repetitively passed as I am doing. I eliminated "string_view" entirely by moving the "dot" replacement to the outer "fold_left". I had a similar version but changed to the replace to use a library function. \$\endgroup\$
    – rm1948
    Commented Jan 1 at 21:51
  • \$\begingroup\$ There is no single LineTotal object that is being passed around; instead, copies are made every time. That's very functional. Of course, the compiler will do its best to avoid all those unnecessary copies so the resulting assembly code might indeed just have one object that is updated in-place. \$\endgroup\$
    – G. Sliepen
    Commented Jan 1 at 23:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.