Title: Code Critique Competition 107

Author: Bob Schmidt

Date: 03 September 2017 16:12:58 +01:00 or Sun, 03 September 2017 16:12:58 +01:00

Summary: Set and collated by Roger Orr. A book prize is awarded for the best entry.

Body: 

Please note that participation in this competition is open to all members, whether novice or expert. Readers are also encouraged to comment on published entries, and to supply their own possible code samples for the competition (in any common programming language) to scc@accu.org.

Note: If you would rather not have your critique visible online, please inform me. (Email addresses are not publicly visible.)

Last issue’s code

I am learning some C++ by writing a simple date class. The code compiles without warnings but I’ve made a mistake somewhere as the test program doesn’t always produce what I expect.

    > testdate
    Enter start date (YYYY-MM-DD): 2017-06-01
    Enter adjustment (YYYY-MM-DD): 0000-01-30
    Adjusted Date: 2017-07-31
    >testdate
    Enter start date (YYYY-MM-DD): 2017-02-01
    Enter adjustment (YYYY-MM-DD): 0001-01-09
    Adjusted Date: 2018-03-10
    >testdate
    Enter start date (YYYY-MM-DD): 2017-03-04
    Enter adjustment (YYYY-MM-DD): 0001-00-30
    Adjusted Date: 2017-04-03

That last one ought to be 2018-04-04, but I can’t see what I’m doing wrong.

Please can you help the programmer find his bug – and suggest some possible improvements to the program!

• Listing 1 contains date.h
• Listing 2 contains date.cpp
• Listing 3 contains testdate.cpp

// A start of a basic date class in C++.
#pragma once

class Date
{
  int year;
  int month;
  int day;

public:
  void readDate();
  void printDate();
  void addDate(Date lhs, Date rhs);
  bool leapYear();
};
			

#include "date.h"
#include <iostream>
using namespace std;

// Read using YYYY-MM-DD format
void Date::readDate()
{
  cin >> year;
  cin.get();
  cin >> month;
  cin.get();
  cin >> day;
}
// Print using YYYY-MM-DD format
void Date::printDate()
{
  cout << "Date: " << year << '-' <<
    month/10 << month%10 << '-' <<
    day/10 << day %10;
}
void Date::addDate(Date lhs, Date rhs)
{
  year = lhs.year + rhs.year;
  month = lhs.month + rhs.month;
  day = lhs.day + rhs.day;
  // first pass at the day -- no months
  // are over 31 days
  if (day > 31)
  {
    day -= 31;
    month = month + 1;
    if (month > 12)
    {
      year += 1;
      month -= 12;
    }
  }
  // normalise the month
  if (month > 12)
  {
    year += 1;
    month -= 12;
  }
  // now check for the shorter months
  int days_in_month = 31;
  switch (month)
  {
    default: return; // done 31 earlier
    case 2: // Feb
      days_in_month = 28 + leapYear()?1:0; 
      break;
    case 4:  // Apr
    case 6:  // Jun
    case 9:  // Sep
    case 11: // Nov
      days_in_month = 30;
  }
  if (day > days_in_month)
  {
    day -= days_in_month;
    month += 1;
    if (month > 12)
    {
      month -= 12;
      year += 1;
    }
  }
}
bool Date::leapYear()
{
  // Every four years, or every four centuries
  if (year % 100 == 0) return year % 400 == 0;
  else return year % 4 == 0;
} 
			

#include "date.h"
#include <iostream>
using std::cout;

int main()
{
  Date d1, d2, d3;
  cout << "Enter start date (YYYY-MM-DD): ";
  d1.readDate();
  cout << "Enter adjustment (YYYY-MM-DD): ";
  d2.readDate();
  // Add the two dates
  d3.addDate(d1, d2);
  cout << "Adjusted ";
  d3.printDate();
}
			

Critique

Jason Spencer

I have to admit, I can’t reproduce the error. I get 2018-04-03 for the last test. Perhaps if there was more information on OS, compiler and STL used I could get 2017-04-03. I suspect this is Windows (the > command prompt) and Visual Studio (the #pragma once directive is non-standard and used there more often), but don’t have such a dev environment to hand right now. Is it possible your build manager is using an old object file with different date calculation logic? Or that you’ve compiled one file but pasted a different file? (It happens to all of us.)

I get 2018-04-03 – the reason it is not 2018-04-04 is because your carry logic in addDate adds 30 to 4, gets 34 and then subtracts 31 – the remainder is 3, days_in_month is set to 30 in the switch statement (month was earlier incremented to 4), but that’s more than the day, so nothing else happens. There are coincidentally 31 days in March, so 2018-04-03 is correct. If you’d added 0000-00-30 to a February date, you’d get the wrong answer. If you were adding across a leap year, you’d also get the wrong answer.

As to why you might get 2017 – I can’t see it – there’s no decrement in the year and no pointer usage. Perhaps there is some garbled input from the console when reading the adjustment date, and because there is no input validation (of range, int parsing or delimiters) the date may be read as year=0, month=1, day=0, ie the adjustment shifted to the right with the 30 left in the buffer (although that would return 2017-04-04 as the adjusted date). This shouldn’t happen though – if there is garbage on the input, the first int parse will set the fail flag on the stream and the later int parsings will return with the variables unchanged (unchanged and therefore uninitialised!).

To test what is being read, put a debugger breakpoint on readDate and step through it while monitoring the values, or just print the year, month and day values at the end of the function.

Taking the functions in order, I’d comment thusly:

• Date::readDate() should check whether the delimiters are correct. Perhaps some bounds checking on the read day, month and year. If you use the same class to also express a duration then this becomes a problem (you might want to add 20 months, but that would be an invalid date). '-' is not only a delimiter but a valid prefix to a signed int and could be consumed by the int read, so check bounds. "-1--1--1" is a valid date. Your code would also accept "a-a-a", "abc", "a". There should be a test to see if the stream is valid at the end of the function – to detect EOF, and use cin::fail() to check if the integer has been correctly parsed.
• Date::printDate() should have a const suffix as it doesn’t change the object [1]. Rather than print the 1s and 10s separately, presumably to print "01" and not "1" for the first month, consider setfill('0') and setw(2) stream manipulators from <iomanip> instead.
• Date::addDate(Date lhs, Date rhs) is almost impenetrable. Rather than implementing the number of days in a month as logic in a switch statement, consider instead a look-up table and also try to avoid magic numbers. Try to make your code as self-documenting as possible. Also, you don’t need to pass the objects by mutable (ie non-const) copy. You could try by const reference: Date::addDate(const Date & lhs, const Date & rhs), but then the compiler has to insert possibly superfluous memory reads because lhs and rhs may be the same object (see "pointer aliasing"), and may even be the current object d1.addDate(d1,d1), which will probably corrupt the result. Date::addDate(const Date lhs, const Date rhs), on the other hand, will copy the Date objects on to the current stack frame, and may elide (omit) the copy if it sees it’s unnecessary (see ‘copy elision’). It also means that when implementing addDate, the compiler will complain if you accidentally write to lhs or rhs.

  Do we need two arguments to the method? This function is actually doing two things – addition and assignment. Perhaps Date::addDate(const Date other)?

  And your addDate logic assumes that the adjustment date is positive – it might not be.

• Date::leapYear() should have a const suffix [1], but I’d also propose a second version that is a static method, so that the programmer can test whether a specific year is a leap year without having to create a dummy Date object. The non-static method should call the static method so you don’t repeat yourself (see ‘DRY principle’) and keep the leap year logic in one place. Consider renaming to isLeapYear() to make it obvious it is a test.
• In terms of general class design, you really should initialize the year, month and day member ints in a default constructor, Date::Date(): year(0), month(0), day(0) {}, as C++ doesn’t always default initialise built-in data types. In main(), try printing d3 before the call to addDate to see what I mean.

So now the main problem with this code – a conceptual error. What does it really mean to add two dates? Is the year you are adding 365 or 366 days long? And in another situation, are the 7 months you want to add each 28, 29, 30 or 31 days long? And in what order do you add the year, month and day? The order affects the result. I’d suggest the simplest way to remove the ambiguity is not to add dates at all, but add a duration to a date.

If you want to program more defensively then you could create wrappers for days, months and years before they are passed to Date methods:

class Days {
  int days;
public:
  explicit Days( int days ): days(days) {}
  operator int() const { return days; }
};

Then you could create Date::add(const Days); and call d1.add(Days(1000)) if you want to add 1000 days to d1. Do not allow the addition of Date to Date. If you also create Months and Years wrappers then you can have a Date::Date(const Years, const Months, const Days) constructor which removes the confusion of US vs ISO ordering:

  Date d1 ( Years(2017), Months(3), Days(4) )

The Days constructor is marked as explicit so that the compiler doesn’t attempt to automatically convert between types.

I suppose you could also create an overloaded Date::add(const Years) and do d1.add(Days(30)); d1.add(Years(1)); This moves the responsibility of the ordering of the addition on to the Date class user, but do still be very careful that you do the carry correctly. In fact, perhaps reconsider whether storing three ints is the best way to represent the date internally – you could store the number of days since a fixed epoch (see ‘Unix time’, and the 2038 problem).

And rather than using program logic to determine how many days there are in a month, why not try a lookup table stored in a static protected member variable like:

  static const constexpr struct {
    char name [MONTH_NAME_MAXLEN+1];
    uint8_t days_nonleap_year;
    uint8_t days_leap_year;
    } MONTH_DETAILS [] = {
      { "Jan", 31, 31 },
      { "Feb", 28, 29 },
      { "Mar", 31, 31 },
  ...
      { "Nov", 30, 30 },
      { "Dec", 31, 31 },
      { "", 0, 0 }
  };

I’ve included a human readable month name for convenience. The struct could also be broken out into separate arrays (MONTH_NAMES[],DAYS_PER_MONTH_NON_LEAP_YEAR[],DAYS_PER_MONTH_LEAP_YEAR[]) to simplify internationalisation.

Having this look up table will also allow you to check input data (in readDate or equivalent) bounds more easily and uniformly across your code. You could also create a function static:

  Date::numberOfDaysInMonth(int month)

so your class could be used as part of a calendar printer, for example.

To remove various magic values consider using:

  enum MONTHS { JAN, FEB, MAR, APR, MAY, JUN,
    JUL, AUG, SEP, OCT, NOV, DEC, LAST };
  static const constexpr int MONTHS_IN_YEAR = 12;

Beware of off-by-one errors when converting the enum to an int, though (JAN above is 0, not 1, but would allow you to do MONTH_DETAILS[FEB].days_leap_year).

Another issue your code has is class design. The Date class should have one responsibility – to store and manipulate dates, not to read or print them. What if you want to read in a US date format, YYYY-DD-MM (an almost related story can be found here [2]), or from a string or file?

readDate and printDate should be standalone functions outside of Date, and then you can do readISODate ( std::cin, d1 ) and readUSDate ( std::cin, d1 ).

Try to avoid repeating information in method names – instead of d1.readDate(), why not d1.read()? Or declare a function outside the class called std::istream& operator>> ( std::istream & input_stream, Date & read_date ), so you can then do cin >> d1.

In general have a look at Boost:Date [3], java.util.Date, and std::chrono for design inspiration. And of course they’re already known to be working, so use them, when you’re not doing this for educational purposes.

In terms of class design, have a look at the books Clean Code [4], Code Complete [5] and The Pragmatic Programmer [6]. All excellent books.

References

[1] https://isocpp.org/wiki/faq/const-correctness#const-member-fns

[2] See root cause here: https://mars.jpl.nasa.gov/msp98/news/mco991110.html

[3] http://www.boost.org/doc/libs/1_61_0/doc/html/date_time.html

[4] Clean Code by Robert C. Martin, Prentice Hall, ISBN 0132350882

[5] Code Complete by Steve McConnell, Microsoft Press, ISBN 0735619670

[6] The Pragmatic Programmer by Andrew Hunt and David Thomas, Addison Wesley, ISBN 020161622X

James Holland

As with many student code examples, it is not entirely clear what the sample code is meant to do. At first glance, it appears that an attempt is being made to add two dates. This doesn’t really make sense. I assume what is meant to happen is that the two values of years entered are to be added, the two values of months are to be added and the two values of days are to be added. If the total number of days is greater than the current month, I assume the days and months are to be adjusted accordingly. If this results in the number of months exceeding 12, the months and years are to be adjusted. All this has to be performed while taking into account leap years. Furthermore, can any value be entered for the adjustment year, month and day?

The sample program output provided by the student adds to the confusion. The first two runs are quite easy to work out in one’s head and agree with my run of the student’s program. The third run is baffling. I do not understand how the program can produce a year of 2017. When I run the program I get 2018 as expected. According to my calculations, 30 days from 4th March is 3rd April, regardless of the year. This value is in agreement with the student’s printout and my running of the program. Why the student insists the date should be the 4th of April, I cannot fathom.

Despite these peculiarities, let’s have a look at the software to see if there is anything definitely wrong. When adding 38 days to 25th July 2017, for example, the student’s program gives 2017-08-32 which is clearly not a valid date. The problem is partly the order in which the student is normalising the days and months, and partly because normalising is not performed often enough and so not completely normalising the date. My solution consists of three while-loops that ensure complete date normalisation. I will not give further details as they are not specific to C++ and, as the student says, it is the writing of simple C++ classes that is of immediate interest. I have provided a few embellishments to the student’s Date class, described below, that may be of some value.

It is often desirable for programmer-defined objects to behave in a similar way to built-in types. For example, it would be convenient to print the value of a Date object by using << as the student does for text strings. This can be achieved by defining the free function operator<<() that takes two parameters; a reference to the output stream on which the value of Date is to be written, and a reference to the Date object. The function prints the value of year, month and day in a similar way to the student’s printDate() member function. One difference is that I have chosen to format the value of month and day using manipulators. In this way the intent of the code is, I think, more clearly stated. Because operator<<() needs access to the private members of the Date class, Date has to grant operator<<() permission by declaring operator<<() as a friend.

Another way to make Date behave more like built-in types is to allow two Date objects to be added using a simple + operator. This is achieved by defining a class member operator+(). This function takes a reference to the ‘right-hand side’ Date object as a single parameter and returns a Date object that is the sum of the current object plus the one referenced by the parameter. From the attached code, it can be seen that operator+() returns a const value. This is to prevent statements such as d1 + d2 = d3 from compiling, as is the case for built-in types.

Finally, I provide a copy constructor and a constructor taking a text string as a parameter. The copy constructor is fairly standard and allows one Date object to be constructed from another Date object. The constructor taking a string may be considered somewhat unconventional but ensures that the object is initialised and provides a convenient way of specifying the text to prompt the user. Declaring a constructor prevents the compiler from automatically generating a default constructor. This can be considered a good thing in this case as it prevents the user from constructing an uninitialised Date object.

Having defined the additional operators and constructors, Date objects can be created and manipulated in a more natural way as shown in main() of the supplied code.

  #include <iostream>
  #include <iomanip>
  using namespace std;
  class Date
  {
    int year;
    int month;
    int day;
    bool leap_year(int year) const
    {
      if (year % 100 == 0)
        return year % 400 == 0;
      return year % 4 == 0;
    }
    int days_in_month(int year, int month) const
    {
      switch (month)
      {
        case 2:
        return leap_year(year) ? 29 : 28;
        
        case 4: case 6: case 9: case 11:
        return 30;
        
        default:
        return 31;
      }
    }
    Date (int new_year, int new_month,
      int new_day)
      : year(new_year), month(new_month), 
        day(new_day){}
    public:
    const Date operator+(const Date & rhs)
    {
      int year = this->year + rhs.year;
      int month = this->month + rhs.month;
      int day = this->day + rhs.day;
      while (month > 12)
      {
        month -= 12;
        ++year;
      }
      while (day > days_in_month(year, month))
      {
        day -= days_in_month(year, month);
        ++month;
      }
      while (month > 12)
      {
        month -= 12;
        ++year;
      }
      return {year, month, day};
    }
    Date(string request)
    {
      cout << request;
      cin >> year;
      cin.get();
      cin >> month;
      cin.get();
      cin >> day;
    }
    friend ostream & operator<<(ostream &,
      const Date &);
  };
  ostream & operator<<(ostream & os,
    const Date & d)
  {
    os << d.year << '-'
       << setw(2) << setfill('0') << d.month
       << '-'
       << setw(2) << setfill('0') << d.day;
    return os;
  }
  int main()
  {
    Date d1("Enter start date (YYYY-MM-DD): ");
    Date d2("Enter adjustment (YYYY-MM-DD): ");
    Date d3 = d1 + d2;
    cout << "Adjusted " << d3 << endl;
  }

Robert Lytton

It would be helpful to start the code review with first impressions.

1. This looks like something that could be found in a library;
2. The ‘WET’ principle is being used;
3. The algorithm used by addDate() is difficult to perceive;
4. class Date is easy to use incorrectly and has a twinge of astonishment.

Let me unpack these impressions and hopefully find the bug in the process.

1. As this is a learning exercise, recreating a solution to a problem is an interesting thing to do. This does not necessarily mean the writer should not make use of lower level libraries during the exercise. More of this later. It should be noted that the cost of not using external libraries includes things such as maintenance & bug fixing; fewer users and tests exercising the code; more tacit knowledge required by people getting on board; Effort to get thing right – the interface.
2. The ‘WET’ (‘Write Every Time’) principle can be seen in the code that truncates months > 12 and increments the year. This fragment is repeated three times during addDate().

   ‘WET’ code is bad for several reasons including:

   1. an opportunity to raise the level of abstraction has probably been missed;
   2. there is more code to compile and to maintain;
   3. if the copies diverge during maintenance, it may be a bug;
   4. there is more code to read and thus hide the essential detail.

   Instead of WET, the ‘Don’t Repeat Yourself’ (DRY) principle should be followed. For addDate(), moving the repeated code into a function may be the correct choice.

3. One of the reasons the addDate() algorithm is difficult to perceive is the ‘WET’ code referred to in #2. Refactoring the functionality into void normaliseMonth(int& month, int& year); (taking the name from the comment and thus making the comment redundant) makes is easier to follow the intention of the code. Further refactoring could include adding the functions, both adding clarity to the code but keeping to the original intention:
   • void normaliseDay(int& day, int& month);
   • int daysInMonth(int month);

   During such refactorings, the bug may become apparent, or just vanish with the replacement code. Vanishing bugs are not good – they may reappear. Also, we want to understand how the defect happened so we can change our practises to avoid similar defects.

   In this case, the bug was in the first block where if (day > 31) reduces the day by 31 and increments the month. This is incorrect – some months require normalising with values less than 31. Later code normalises correctly, using days_in_month, but this can’t fix the earlier mistake. Could this be a case of #2c above – the WET normalise-day code being fixed in the 2nd but not the 1st occurrence?

   The aim of refactoring should be to turn ‘no obvious bugs’ into ‘obviously no bugs’ (paraphrasing Tony Hoare). In our case it should be possible to reason with the refactored code and reorder, reduce and simplify – an exercise for the reader.

4. Class design is more than a correct implementation, or even an implementation that is ‘obviously correct’. Scott Meyers exhorts “Make Interfaces Easy to Use Correctly and Hard to Use Incorrectly” – let us explore this principle. The class interface consists of the public elements: four methods plus six default methods added by the compiler. The default copy-constructor, move-constructor, copy-initialiser, move-initialiser and destructor all handle the private member data as we would wish.

   However:

   1. The default constructor – does not initialise the member data. To prevent undefined behaviour, the members need to be initialised.

      This may be done with default values in the declaration:

               int year=0;
               int month=0;
               int day=0;

      or by reusing our readDate() method:

               void Date::Date() {readDate();}

      or it may be better to specify a constructor that force the user to pick a valid date instead:

               void Date::Date(int y, int m, int d):
                 year{y}, month{m}, day{d} {}

      N.B. the use of int for each parameter in this example makes the constructor easy to misuse (wrong order) – see later.

   2. readDate() – the implementation is coupled to std::cin and is not robust. This coupling is hidden from users, is not necessary and reduces usability. An alternative is to have a stream passed in explicitly by the caller:

               void Date::readDate(std::istream& in);

      Or use a non-member function (which calls a Date setter):

               std::istream& operator>>(std::istream& is,
                 Date& d) {
                 // read in the values and validate them!
                 //...

      As the comment suggests, the current version lacks error or sanity checking of input values. Should day be set to 1000, the class will not work as expected!

      A bonus of using an istream is that we can using automated unit tests rather than typing!

   3. printData() – the implementation is coupled to std::cout and is not const. The choices are similar to those of readDate() but the preferred option is to add a print() method:

             std::ostream& Date::print(std::ostream &out)
               const;

      and a non-member function to call it:

             std::ostream& operator<<( std::ostream& os,
               const Date& d ) {
               return d.print(os);
             }

      It should also be noted that printing month and day don’t need to use / & %.

   4. addData() – does not follow the ‘principle of least astonishment’. This astonishment could be lessened by turning the method into a constructor. An alternative would be to do what other classes do and overload +:

              Date& Date::operator+=(const Date &rhs) {
                year += rhs.year;
                //...
              }

      and add a non-member function too:

              Date operator+(Date lhs, const Date& rhs) {
                lhs += rhs;
                return lhs;
              }

   5. leapYear() – is not an essential part of the interface. This should be removed from the interface by making it private. If it is require later, it may be added at the cost of a recompilation – thus following the ‘open close’ principle.

We could spend longer digging in deeper with issues such as:

• Constraining values within valid limits using unsigned, or enums;
• Using strong types for day, month, year rather than an int or weak typedef;
• Checking values entering the class are valid – thus maintaining invariants;
• Using lower-level abstractions to hold and manipulate state on behalf of the class e.g. time_t & time.h or Boost::DateTime;
• White-box unit testing of normalisation paths & combinations of paths;
• The true cost of a class.

But they are not first impressions.

Commentary

First off, an apology: I can’t explain the final result I posted in the original critique. While I thought I’d copy-pasted it into the critique from a command prompt, I obviously didn’t do this successfully because (a) I cannot reproduce this result and (b) the result is in fact correct and does not demonstrate the bug in the program. I apologise for the confusion this error caused!

I haven’t a lot of commentary to add as I think the entries between them cover pretty well all the ground; both the problems with the implementation and the various higher-level issues with the design of the class.

While it was mentioned that you don’t need to split the month and day into tens + units to ensure they print correctly, with standard iostreams it is quite painful as you need to:

• save the current state of the fill character and set it to '0'
• set the width to 2 (for each field)
• restore the fill character (avoids future surprises with the ostream...).

For example:

  // Print using YYYY-MM-DD format
  void Date::printDate()
  {
    auto orig(cout.fill('0'));
    cout << "Date: " << year << '-' <<
      std::setw(2) << month << '-' <<
      std::setw(2) << day;
    cout.fill(orig);
  }

It is arguable whether this is better than the original.

The Winner of CC 106

All three entrants did a good job of explaining the problem with the class and suggesting improvements. Pointing out ways in which the design differs from the usual C++ idioms for value types would be particularly useful given the context of someone trying to get more familiar with C++.

On balance, I think Jason’s critique was overall the best one, so I have awarded him this month’s prize.

Code Critique 106

(Submissions to scc@accu.org by Oct 1st)

I want to collect the meals needed for attendees for a one-day event so I’m reading lines of text with the name and a list of the meals needed, and then writing the totals. However, the totals are wrong – but I can’t see why:

    > meals
    Roger breakfast lunch
    John lunch dinner
    Peter dinner

    Total: 3 breakfast: 3 lunch: 2 dinner: 2

There should only be 1 breakfast, not 3!

Please can you help the programmer find the bug – and suggest some possible improvements to the program!

• Listing 4 contains meal.h
• Listing 5 contains meals.cpp

#pragma once
#include <iosfwd>
#include <sstream>
#include <string>
enum class meal : int
{
   breakfast, lunch, dinner,
};
// Used for name <=> value conversion
struct
{
  meal value;
  std::string name;
} names[] =
{
  { meal::breakfast, "breakfast" },
  { meal::lunch, "lunch" },
  { meal::dinner, "dinner" },
};
std::istream &operator>>(std::istream &is, 
  meal &m)
{
  std::string name;
  if (is >> name)
  {
    for (auto p : names)
    {
      if (p.name == name)
        m = p.value;
    }
  }
  return is;
}
std::ostream &operator<<(std::ostream &os, 
  meal const m)
{
  for (auto p : names)
  {
    if (p.value == m)
      os << p.name;
  }
  return os;
}
// Type-safe operations
constexpr meal operator+(meal a, meal b)
{
  return meal(int(a) + int(b));
}
meal operator+=(meal &a, meal b)
{
  a = a + b;
  return a;
}
constexpr meal operator|(meal a, meal b)
{
  return meal(int(a) | int(b));
}
constexpr meal operator&(meal a, meal b)
{
  return meal(int(a) & int(b));
}
// Check distinctness
static_assert((meal::breakfast | meal::lunch |
 meal::dinner) == (meal::breakfast +
 meal::lunch + meal::dinner), "not distinct");
			

#include "meal.h"

#include <iostream>
#include <list>

struct attendee
{
   std::string name;
   meal meals; // set of meals
};

using attendees = std::list<attendee>;

attendees get_attendees(std::istream &is)
{
  attendees result;
  std::string line;
  while (std::getline(is, line))
  {
    std::istringstream iss(line);
    std::string name;
    iss >> name;
    meal meal, meals{};
    while (iss >> meal)
      meals += meal; // add in each meal
    if (is.fail())
      throw std::runtime_error("Input error");
    result.push_back({name, meals});
  }
  return result;
}
size_t count(attendees a, meal m)
{
  size_t result{};
  for (auto &item : a)
  {
    // Check 'm' present in meals
    if ((item.meals & m) == m)
      ++result;
  }
  return result;
}
int main()
try
{
  auto attendees{ get_attendees( std::cin) };
  std::cout << "Total: " << attendees.size();
  for (auto m : { meal::breakfast,
                  meal::lunch, meal::dinner })
  {
    std::cout << ' ' << m <<  ": " <<
      count(attendees, m);
  }
  std::cout << '\n';
}
catch (std::exception &ex)
{
  std::cout << ex.what() << '\n';
}
			

You can also get the current problem from the accu-general mail list (next entry is posted around the last issue’s deadline) or from the ACCU website (http://accu.org/index.php/journal). This particularly helps overseas members who typically get the magazine much later than members in the UK and Europe.

Notes: 

More fields may be available via dynamicdata ..