Re: [lmi] Control flow in page_with_tabular_report::render()

[Vadim Zeitlin]

On Tue, 13 Feb 2018 19:31:39 +0000 Greg Chicares <address@hidden> wrote:

GC> There's one thing about it that I find especially compelling: its
GC> compactness. Apart from the lines that contain only a single brace,
GC> all the work is done in four lines of code. I can focus deeply on
GC> those four lines and pretty quickly come to a deep understanding of
GC> what they do and how they do it. I don't have to page up a hundred
GC> lines to consult a base class's implementation, look in another file
GC> to find a template, etc.; instead, I can get it all at a glance.

 Yes, the absence of inversion of control in this approach is an advantage.
I still find it difficult to believe that it's a big one, i.e. that a class
like my "paginator" with 2 or 3 pure virtual methods with obvious meaning
is difficult to understand.

GC> My code actually printed an array. This isn't quite comparable in
GC> that sense. Would it be too much trouble if I asked you to flesh
GC> it out, to the same degree as the little program I had posted,
GC> so that we can compare our design philosophies in microcosm?

 Sure, here is my version:
---------------------------------- >8 --------------------------------------
#include <iostream>
#include <string>
#include <vector>

class paginated_table_output
{
  public:
    void generate(int page_height, int total_rows)
    {
        auto const row_height = get_row_height();

        for(int row = 0; row < total_rows; )
            {
            int y = get_fixed_page_part_height();
            for(;;)
                {
                on_data_row(y, row);
                y += row_height;

                if(++row == total_rows)
                    break;

                if(row % rows_per_group == 0)
                    {
                    on_blank_row(y);
                    y += row_height;

                    auto rows_in_next_group = rows_per_group;
                    if(total_rows - row < rows_per_group)
                        rows_in_next_group = total_rows - row;

                    if(y > page_height - rows_in_next_group*row_height)
                        {
                        on_start_new_page();
                        break;
                        }
                    }
                }
            }
    }

  private:
    static int const rows_per_group = 5;

    virtual int get_row_height() = 0;
    virtual int get_fixed_page_part_height() = 0;
    virtual void on_data_row(int y, int n) = 0;
    virtual void on_blank_row(int y) = 0;
    virtual void on_start_new_page() = 0;
};

int main()
{
    class test_output : public paginated_table_output
    {
      public:
        test_output()
        {
            generate(200, data_.size());
        }

      private:
        int get_row_height() override
        {
            return 10;
        }

        int get_fixed_page_part_height() override
        {
            std::cout << "multi-line\nheader\n\n";
            return 3*get_row_height();
        }

        void on_data_row(int y, int n) override
        {
            std::cout << "At pos " << y << ":\t" << data_[n] << "\n";
        }

        void on_blank_row(int) override
        {
            std::cout << std::endl;
        }

        void on_start_new_page() override
        {
            std::cout << "----------------------------\n";
        }

        std::vector<std::string> data_
            
{"A","B","C","D","E","F","G","H","I","J","K","L","M","N","O","P","Q","R","S","T"};
    } out;
}
---------------------------------- >8 --------------------------------------

 It is, of course, bigger, because it contains both the pagination code and
the test code using it, and also outputs the header, but I think it's very
simple to understand and will be also simpler to modify.

GC> I'd like to see whether we're striving for much the same thing
GC> in much the same way, but describing our viewpoints in such
GC> different terms that it's hard to see where we're in agreement.

 No, unfortunately I don't think we're string for the same thing at all.
I'm [more than] ready to pay a small abstraction penalty for clarity,
"obvious" correctness and better maintainability.

GC> > GC> No fancy design patterns needed. No iterators. No lambdas.
GC> > 
GC> >  This is not completely fair. The "template method" pattern is not fancy 
at
GC> > all and many people use it without ever even realizing they do it. And
GC> > lambdas was an alternative to using it, so it's either one or the other
GC> > (and, of course, what could be more like ANSI C than using callbacks...).
GC> 
GC> Custom iterator classes, lambdas, and function pointers really do
GC> seem fancy to me.

 The lambdas syntax is ugly ("[](){}" is a valid lambda...), but any C++11
programmer should really be able to write them without any problems (unlike
custom iterators). But, again, we don't even need them here if overriding
virtual functions, as the example above does, is fine.

GC> I've tried to read the GoF book several times, hoping to realize that
GC> I've used at least some of its patterns without realizing it--like
GC> Molière's character who's excited to learn that he's been speaking
GC> prose all his life.

 It has been exactly this for me, when I read it all these years ago.
I'm really surprised you didn't recognize at least some patterns in it too.

GC> But that realization never comes, and I've grown to suspect that most
GC> of this stuff isn't pragmatically useful for me. Well, Singleton is,

 This is the worst pattern of all mentioned there...

GC> And I have had the "I can speak prose" insight with other patterns
GC> not on the GoF list. For instance, it's clear to me that if we'd
GC> never heard of CRTP, we'd all have to invent it. More strikingly,
GC> when I first heard of MVC, I realized first that I'd been doing it
GC> for years, but poorly; and second, that MVC was better than anything
GC> I would have invented myself.

 CRTP is nice, but C++-specific, unlike the patterns in the GoF book. MVC
is more than a pattern, I think, or, at least, a very complex one. I don't
remember all GoF patterns by far, but among the less trivial ones, Visitor
must be the one that is most useful on average. Again, I'm very surprised
you've never had to use it.

GC> Maybe I'd understand this better if you can point out something I've
GC> written in lmi that exemplifies the Template pattern

 It might be cheating, because it's not really something you have a choice
about, but you use the Template pattern in every application using
wxWidgets when you override wxApp::OnInit() in your code, including lmi.

GC> (or others in
GC> the GoF catalog). I'm not asking you to spend a lot of time poring
GC> over hundreds of source files--I'm hoping you might have noticed a
GC> GoF pattern or two somewhere already and can just point me to it.

 I can't think of anything right now... ViewEx could be seen as a Façade of
wxView, but Façade is a trivial pattern to begin with and this is a
particularly simple example of its application, so it's hardly interesting.


GC> What I am saying is that if we want to express the sort of control flow
GC> for which K&R designed for(), then we should use for(). And I think the
GC> task at hand is a matter of control flow, and one for which for() is
GC> best suited.

 Sorry, I still disagree. I see no reason to view this as a control flow
problem, we've seen how easy it is to make mistakes when treating it like
this and so it seems much better to encapsulate the iteration in an easily
testable class and then let the code using be really simple and obvious by
implementing just the bits not related to pagination.

 Regards,
VZ