2
\$\begingroup\$

Some might not think it's worth asking but I consider good function names quite important. I am not sure what should be the overridable function name in what I think is template design pattern. I am printing a report which has 3 types. There are lots of if-else which is confusing. I replace the long function with the polymorphic class hierarchy with base class below. I have than derived a class from each report type which overrides the PrintCustom() function but this doesn't strike me as a good name.

class CPrintReport
{
protected:
    CPrintReport(void);
public:
    virtual ~CPrintReport(void);

    // template function?
    void PrintReport();

protected:
    void PrintCommon();
    virtual void PrintCustom() = 0;
    virtual void PrintChart();


protected:
    CStringArray pstrData;

    CGraphCtrl * m_GraphCtrl;

};

PrintReport is the main function which is as following:

void CPrintReport::PrintReport()
{
    // prints thte common data
    PrintCommon();

    // prints specialized (verification,identification or regular type)
    PrintCustom();

    PrintChart();

}
  1. Does this implement the template design pattern where PrintReport() is the template function?

  2. As you can see from report, the first section prints common information, and the last section prints a chart, but the middle section varies based on what type of report it is. If it was procedural programming I would be calling this PrintVerificationData()and PrintIdentifcationData() depending on which type of report is being printed. How do we name such methods in the template design pattern so it makes better sense?

  3. It turns out PrintChart is going to be different as well for each type. However, it is the same for 2 types (verification and identification) and different for regular print. Does this mean PrintChartVerifyIdentify() (being lazy but making a point) should reside in the base class so it can be shared, or should I use a visitor pattern or something?

\$\endgroup\$
  • 1
    \$\begingroup\$ I think you'd find that most of us find that good names are extremely important. =) \$\endgroup\$ – RubberDuck Jun 24 '14 at 15:19
  • \$\begingroup\$ I would guess naming conventions is off topic. I would ask on programmers.stackexchange.com \$\endgroup\$ – Martin York Jun 25 '14 at 0:12
3
\$\begingroup\$

I think you're right to be concerned with the names you're using.

I'd start with the name of the base class. CPrintReport strikes me as a rather poor name. First of all, it sounds like a verb phrase (Print Report). Most classes should be nouns. Second, you typically want the name to reflect the fact that it's an abstract base class. Finally, tacking a C onto the beginning of a class name is, frankly, a pretty lousy idea.

I'd probably start with a report namespace, and name this base:

namespace report {    
    class Base {
        // ...
    };    
}

The other reports would derive from it, so you might have something like:

namespace report { 
    class Verification : public Base { 
        // ...
    };

    class Identification : public Base { 
        // ..
    };    
}

Then when you want to print one, your code would be something like:

report::Verification verification_report;
verification_report.print(printer);

As far as the names of the individual functions go, I think the problem is fairly simple: right now, your names basically reflect the mechanics of generating the reports. I'd prefer to see names that reflect the sections of the report they generate instead.

For example, your PrintCommon, PrintCustom and PrintChart might become: PrintIntroduction, PrintDetails, and PrintSummary respectively. Of course, I don't know for sure how your reports are structured, so there might be other names that make more sense, but the basic idea still applies--to the extent possible, base the names on their effect in the final product rather than on the details of how you're implementing the code.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ I like the idea but is it really okay to have multiple namespaces in single project? Also there is no clear grouping of data but if I give it meaningful names (I agree) it will be a separate function for each line for example PrintThreshold() PrintResult PrintSearchRange, so I guess I should list all functions cumulatively and then override the ones to not print in derived class as applicable? \$\endgroup\$ – zadane Jun 25 '14 at 20:49
  • \$\begingroup\$ @zadane: Yes, it's fine to have multiple namespaces in one project. Hard to guess more about how to organize the functions without knowing more, but at least at first glance, your comment sounds fairly reasonable. \$\endgroup\$ – Jerry Coffin Jun 25 '14 at 22:10
3
\$\begingroup\$

Besides the good points made by Jerry Coffin:

§1. Does this implement the template design pattern where PrintReport() is the template function?

Yes (but please refer to it as "template method" - "template function" makes most c++ developers think of template <...> function();).

§2. How do we name such methods in the template design pattern so it makes better sense?

good naming is hard :)

I usually choose this by writing a line of client code, then rephrasing it to read like prose.

output_dependencies out; // std::ostream&, some file stream, whatever
                         // you need to inject into a report generator
                         // for generating output

reports::verification_report report;
report.generate(out);

This implies an interface similar to this:

namespace reports {

    class report { // base class for reports
    public:
         void generate()
         {
             print_header(); // chose names, based on position in generate
             print_body();   // this is "it"
             print_footer();
         }

         // constructor here ...

    protected:
         void print_header(); // me gusta snake_case :)
         virtual void print_body() = 0;
         void print_footer();
    };
}

Now, generate reads like prose. You don't really care when you look at it that print_body is a template method (at least, you shouldn't). You can follow the steps of implementing generate just by one look at the code. Just think of generate as client code of your private APIs, and then the print_ functions are APIs that need to "tell you what they do", in their names.

Basically, I focus on what the function looks like in client code, instead of what kind of place it has in your class hierarchy (or most other technical/implementation details).

Note: I didn't know what name to choose for the generate function (the "PrintReport" in your code) until I wrote an example of client code (and I thought "of course, you generate a report!" - it comes naturally off the tongue :) ).

Maybe for you, "generate" will not sound natural at all.

Edit (addressing question update):

It turns out PrintChart is going to be different as well for each type however it is same for 2 types (verification and identification) and different for regular print. Does this mean PrintChartVerifyIdentify() (being lazy but making a point) should reside in the base class so it can be shared or should use visitor pattern or something?

I would write it like this:

namespace reports {

    // base class for all chart types
    class chart { // todo: move in it's own header :D
    public:
        virtual void print() = 0; // can be implemented non-virtually with
                                  // the template method as well
    };

    // report containing a chart
    class chart_report: public report {
    public:
         chart_report(std::unique_ptr<chart> c)
         : chart_{ std::move(c) } {}
    protected:
         void print_body() override {
             // if needed, do stuff here
             chart_ptr->print();
             // ... or here
         }
    private:
         std::unique_ptr<chart> chart_;
    };

}

Now, you can write a chart that combines verification and identification information, or a chart that is "different for regular print" (as you said).

You can also add a different specialization for reports (say "multiple_charts_report" that uses charts differently, or in different positions in the report). This could implement verification and identification reports as two separate units (maybe different charts in different places in the generated report?). (sorry for abusing the word "different").

I chose this architecture, because now chart development can continue without touching the report hierarchy (it is orthogonal to it).

Also, if initializing a report becomes more than 1 LOC long, consider writing a factory function for creating report objects.

Client code (assuming you implement a class verify_id_chart: public reports::chart):

reports::chart_report report{ new verify_id_chart{} };
report.generate(out);
|improve this answer|||||
\$\endgroup\$
2
\$\begingroup\$

Although not related to naming, the print functions should all be const. Such functions do not involve modifying any data members.

void PrintReport() const;

void PrintCommon() const;

virtual void PrintCustom() const = 0;

virtual void PrintChart() const;

In addition, consider using a different naming convention for all of your functions, since they use the same naming as your types. Functions are commonly named in either camelCase or snake_case.

|improve this answer|||||
\$\endgroup\$
1
\$\begingroup\$

How about calling your method PrintSpecializedReportData and call your subclasses CPrintVerificationReport and CPrintIdentificationReport?

Also, rename PrintCommon to PrintCommonData.

And remove the comments that are now really unnecessary. Notice how, instead of commenting, you should have renamed your methods according to what they do.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ PrintCustom or if your prefer PrintSpecializedReportData is virtual method and the subclass implementation will be called automatically. Still not sold on the name. Maybe it could be PrintReportTypeData hmm.. \$\endgroup\$ – zadane Jun 24 '14 at 15:24
  • \$\begingroup\$ Well, it's what you wrote on the comment. In a first draft of my answer, I suggested PrintReportData. It makes your method read like so: print common data, then print report data, and finally, print chart. \$\endgroup\$ – Laurent LA RIZZA Jun 24 '14 at 19:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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