Title: Code Critique Competition 119

Author: Bob Schmidt

Date: 03 September 2019 17:09:32 +01:00 or Tue, 03 September 2019 17:09:32 +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’m writing code to process sets of address lists, and was hoping to avoid having to make copies of the data structures by using references. However, I can’t get it working – I’ve produced a stripped-down example that reads sample data from a file but it doesn’t work at all.

Sample data:

     Roger Peckham London UK
     John Beaconsfield Bucks UK
     Michael Chicago Illinois USA

Expected output:

addresses, sorted by country and then county

Actual output:

three blank lines!

Can you solve the problem – and then give some further advice?

Listing 1 contains the code.

#include <algorithm>
#include <iostream>
#include <iterator>
#include <map>
#include <string>
#include <vector>
struct address /* simplified */
{
  std::string city;
  std::string county;
  std::string country;
};
std::istream& operator>>(std::istream& is,
  address& t)
{
  is >> t.city >> t.county >> t.country;
  return is;
}
std::ostream& operator<<(std::ostream& os,
  address& t)
{
  os << t.city << ' '<< t.county << ' '
     << t.country;
  return os;
}
// sort addresses by country, county, and
// then city
bool operator<(address const &a,
               address const & b)
{
  if (a.country < b.country) return true;
  if (a.country == b.country &&
      a.county < b.county) return true;
  if (a.country == b.country &&
      a.county == b.county &&
      a.city < b.city) return true;
  return false;
}

// This is just for testing, real data
// is supplied differently
auto read(std::istream &is)
{
  std::map<std::string, address> ret;
  while (is)
  {
    std::string name;
    address addr;
    if (is >> name >> addr)
    {
      ret[name] = addr;
    }
  }
  return ret;
}

int main()
{
  std::map<std::string, address> map;
  map = read(std::cin);

  // Don't copy the addresses
  std::vector<std::tuple<address&>> addrs;
  for (auto entry : map)
  {
    addrs.push_back(entry.second);
  }

  // sort and print the addresses
  std::sort(addrs.begin(), addrs.end());
  for (auto& v : addrs)
  {
    std::cout << std::get<0>(v) << '\n';
  }
  std::cout << '\n';
}
			

Critiques

Nicolas Golaszewski

The Original Code (OC) wanted to spare copies by using references, which unfortunately cannot be used inside std::vector. Using a std::tuple with only one reference as the vector’s element was a nice trick. However, when it comes to sorting, this also brings in the use of swapping. Given a and b (two values of type T) and Ra and Rb (respectively the two references), the normal first step of swapping:

  T & Rc = Ra;

does not create a copy (of a) but just a synonym of Ra.

The second swapping step

  Ra = Rb;

ends by getting rid of the value of a in favor of the value of b.

So, why not use pointers to save copies? Old time raw pointers are now discouraged. Smart std::shared_ptr<T> would maybe be too expensive when a copy of the T value is cheap.

Fortunately there exists std::reference_wrapper<T> which just... encapsulates a raw pointer to T.

So, just rewording the OC addrs variable

  std::vector<std::tuple<std::reference_wrapper<address>>> addrs;

is enough to make things work again.

Other ideas on the OC came from the operator

  bool operator<(address const &a,
    address const & b){...}

which defines the comparison order by country first, county and city last.

This is exactly what the operator< for std::tuple does.

So, the idea is to recycle the tuple trick to hold the 3 members of the address (I know the address class is simplified but I have seen the comparison operator< manually redefined many times where a tuple would get the job done).

So, the idea is to have an empty address class just to define what kind of data will be used

  struct address  {
    // only good idea if sorting order stays
    // the same
    enum item { country = 0, county, city };
    using type = std::tuple<std::string,
                 std::string,
                 std::string>;
    using ref_wrap_type =
      std::reference_wrapper<type>;
    using ref_wrap_const_type =
      std::reference_wrapper<const type>;
  };

And, for example, the input operator>> becomes

  std::istream& operator>>(
    std::istream& is,address::type& t)
  {
    is >> std::get<address::city>(t)
       >> std::get<address::county>(t)
       >> std::get<address::country>(t);
    return is;
  }

The most interesting thing is in the comparison operator< – we do not need operator< for address as we will use the operator< of std::tuple.

The auto read(std::istream &is) testing function stays the same.

The main() code becomes

  //...
  std::map<std::string, address::type> map =
    read(ifs);
	
  // Don't copy the addresses
  std::vector<address::ref_wrap_const_type>
    addrs;
	
  for (const auto & entry : map) {
    addrs.push_back(entry.second);
  }
  //...

Note that now the map value type is a tuple of 3 std::string and that the addrs vector holds reference to this const qualified type. (Put const whenever possible...)

I thought this would be the end of the story but, naturally, the std::sort algorithm will look for a comparison operator< of… std::reference_wrapper<address::type>… which naturally (and fortunately) does not exist.

Not being discouraged, the following adjustment with a lambda as comparator

  std::sort(addrs.begin(), addrs.end(),
         [](address::ref_wrap_const_type rwc_a,
            address::ref_wrap_const_type rwc_b)
    {return rwc_a.get() < rwc_b.get();}
  );

was simple enough to escape this pitfall.

Below is the complete code, tested on coliru just in case… g++ -std=c++14 -O2 -Wall -pedantic main.cpp && ./a.out

#include <algorithm>
#include <iostream>
#include <map>
#include <string>
#include <vector>
#include <tuple>
#include <fstream>
#include <functional>
#include <sstream>
struct address
{
  // only good idea if sorting order stays
  // the same
  enum item { country = 0, county, city };
  using type = std::tuple<
    std::string,std::string,std::string>;
  using ref_wrap_type          =
    std::reference_wrapper<type>;
  using ref_wrap_const_type    =
    std::reference_wrapper<const type>;
};
std::istream& operator>>(
  std::istream& is,address::type& t)
{
  is >> std::get<address::city>(t)
     >> std::get<address::county>(t)
     >> std::get<address::country>(t);
  return is;
}

std::ostream& operator<<(
  std::ostream& os,const address::type& t)
{
  os << std::get<address::city>(t) << ' '
     << std::get<address::county>(t) << ' '
     << std::get<address::country>(t);
  return os;
}

/*We do not need operator< for address as we
  will use the operator< of std::tuple*/

// This is just for testing, real data
// is supplied differently
auto read(std::istream &is)
{
  std::map<std::string, address::type> ret;
  while (is) {
    std::string name;
    address::type addr;
    if (is >> name >> addr) {
      ret[name] = addr;
    }
  }
  return ret;
}
int main()
{
  std::stringstream  ifs(
    "Roger Peckham London UK\n"
    "John Beaconsfield Bucks UK\n"
    "Bart Springfield Illinois USA\n"
    "Michael Chicago Illinois USA\n"
    "Fuzz Oldbury West_Midlands UK\n"
    "Lisa Springfield Illinois USA\n"
    "Thomas Champaign Illinois USA\n");
  std::map<std::string, address::type> map =
    read(ifs);

  // Don't copy the addresses
  std::vector<address::ref_wrap_const_type>
    addrs;
  for (const auto & entry : map)  {
    addrs.push_back(entry.second);
  }
  for (const auto& v : addrs)  {
    std::cout << v << '\n';
  }
  std::cout <<
    "\n---------------------------------\n\n";
  std::sort(addrs.begin(), addrs.end(),
        [](address::ref_wrap_const_type rwc_a,
           address::ref_wrap_const_type rwc_b)
        {return rwc_a.get() < rwc_b.get();});
  for (const auto& v : addrs) {
    std::cout << v << '\n';
  }
  std::cout << std::flush;
}

James Holland

The student states that the stripped-down example reads data from a file. In fact, it reads data from std::cin. This is a small matter but it means that the end-of-file character has to be entered from the keyboard by pressing the appropriate key. On my system this is Ctrl-D.

Having sorted the input, we can now investigate the processing of the data. The student states that three blank lines are produced. When I ran the program it issued “Peckham London UK” on three separate lines. I must confess I am not exactly sure why the program produces this output but it has something to do with declaring the vector addrs with a template parameter of type std::tuple<address &>. Removing the & results in the program working as expected. Now the tuple holds a copy of objects of type address. This is unfortunate as the student wanted to store references to address objects. The way the student has declared the tuple to store references seems perfectly natural but adding the & is incorrect. The correct way to store references is to declare the tuple without the & but to make a tuple from an address object by using std::ref() (declared in the functional header file). This requires an extra line in the for-loop as shown below.

  std::vector<std::tuple<address>> addrs;
  for (auto & entry : map)
  {
    auto tup =
      std::make_tuple(std::ref(entry.second));
    addrs.push_back(tup);
  }

I have also made the entry loop variable a reference. This may help speed things up as entry will no longer be a copy of entry.second. The whole tuple is copied, however, when it is pushed onto the back of the vector.

I notice that operator<() has been defined explicitly. There is a slightly neater way of defining operator<() by making use of std::tie() as shown below.

  bool operator<(address const & a,
    address const & b)
  {
    return std::tie(a.country, a.county, a.city)
      < std::tie(b.country, b.county, b.city);
  }

In this version of operator<(), the two addresses are first decomposed into the component parts and then reassembled into tuple objects containing three objects: the country, the county and finally the city. When comparing two tuple objects, initially the first parameters of the tuples are compared. If these parameters are equal, the next two parameters are compared, and so on. This is exactly the behaviour that is required for comparing countries, counties, and cities.

As the code uses std::tuple, the header file, #include <tuple>, should be added explicitly. In the student’s program, I suspect the tuple header file is being found in #include <map>. The second parameter of operator<<() could be made const as could the identifier entry of the first range-based for-loop of main(). There is no need to abbreviate variable names quite so much, I suggest. Perhaps the variable addrs could be renamed addresses_of_candidates or something similar that is appropriate to the application.

The desire to write fast and efficient code is understandable but it is important to, firstly, ensure the code works correctly. Only then is it worth considering modifications that are designed to make the code run faster. Compilers are getting very clever at optimising code for speed and so one should always measure the performance of any code modification to see if it has really made an improvement. I know of cases where an enthusiastic engineer has amended code with the desire of making it faster but has actually made it run slower. It has been said that you should not help the compiler. If you keep things simple the compiler will know what you are trying to do and will probably make a good job at optimising the code. This does not mean that you should not provide the compiler with as much information as possible, however. As an example, making function parameters const ref where appropriate is almost always a good idea.

As the code provided is a stripped-down version of the original application, it is hard to know why particular features have been employed. I am thinking particularly of the use of std::tuple. I can think of no use for a tuple that contains just one value but this may be a result of providing example code that exhibits the problem. Perhaps the code could be designed not use std::tuple thus simplifying things further.

Marcel Marré and Jan Eisenhauer

The main issue of this competition’s code is the use of references. The original author writes in a comment that he doesn’t want to copy the addresses as he builds the vector to be sorted. However, for (auto entry : map) does exactly that: it makes a copy. Even worse, it makes a temporary copy that is thrown away later, pushing us into the realm of undefined behaviour, which can explain the three empty lines (in my g++ I got three identical lines rather than empty lines; this kind of different behaviour across implementations can indicate undefined behaviour).

However, even after changing the loop to for (auto & entry : map), all is not well. One entry is duplicated, another gone. The problem is again an incorrect use of references. The original author stores the addresses to be sorted in a std::vector<std::tuple<address&>>. The page on std::sort on cppreference, https://en.cppreference.com/w/cpp/algorithm/sort, notes that the iterators must be ValueSwappable. This, however, does not hold for references. Rather than swapping references in the vector, we change data in the map that the vector entries refer to. In other words, with the std::tuple<address&>, construction has different semantics from assignment.

With std::reference_wrapper, construction and assignment have consistent semantics, rebinding the reference in both cases. Hence, changing the vector to std::vector<std::reference_wrapper <address>> corrects the code’s behaviour.

Following the rule of being as const as possible, we can also declare the output operator as std::ostream& operator<<(std::ostream & os, address const & t) with the body remaining unchanged. This also goes for the output loop: for (auto const & v : addrs).

While this fixes the behaviour of the code, the operator< can also be written a lot more elegantly in modern C++:

  return std::tie(a.country, a.county, a.city)
    < std::tie(b.country, b.county, b.city);

is the whole required body of the operator, which is immensely more readable, more easily expanded for further fields and thus also a lot less error-prone.

Even though the read function is only for testing, it can also be abbreviated, because formatted input on a bad stream has no effect:

  auto read(std::istream &is)
  {
    std::map<std::string, address> ret;
    std::string name;
    address addr;
    while (is >> name >> addr)
    {
      ret[name] = addr;
    }
    return ret;
  }

This also reuses the capacity of the strings in each iteration of the while-loop rather than constructing a new local string each time.

Finally, the code uses nothing from the <iterator> header, so it is cleaner to remove the include.

Hans Vredeveld

When I build (with g++ 7.4.0 on Linux) and execute the program with the same data as the OP, I don’t get three blank lines, but three times the line ‘Peckham London UK’. It looks like we run into some undefined behaviour.

Before we delve into the main problem, let’s get some small issues out of the way.

The include for tuple is missing.

There is a spurious include of iterator.

Like the function read, the operator>> to read in an address is fine for the testing done here, but it is too simple to handle real data. For example, consider the Dutch city Den Haag in the province Zuid Holland.

The operator<< to output an address, does not modify the address and should take it by const&.

To improve readability, it would be nice to have a class or typedef name_t and write std::map<name_t, address>, instead of writing std::map<std::string, address>.

In main, using map both for the type and for the variable is confusing. I suggest renaming the variable to addr_map.

The first two statements in main can be combined into a single statement that initializes the map on construction with the result of read.

The for loop to print the addresses at the end of main should take an address as const&: for (auto const& v : addrs).

With the small things out of the way, we can start to look into the main issues with the code. In the following, it is instructive to also look at the content of the address map. Therefore, let’s add the following code snippet to the end of main:

  for (auto const& entry : addr_map)
  {
    std::cout << entry.first << " -> "
      << entry.second << '\n';
  }
  std::cout << '\n';

This will show the input data sorted on name.

The OP wanted to avoid copying addresses. Unfortunately (s)he forgot that when you don’t explicitly specify the loop variable in a range-based for-loop as reference, it will take the elements of the container by copy. The result is that the vector addrs is filled with references to a local variable that is out of scope by the time we sort the vector. In my test situation, it still contained the data of the last address added.

Making the loop-variable entry a reference, the elements in the vector now reference the elements in the map. Building and executing the program, I now get the output

  Beaconsfield Bucks UK
  Chicago Illinois USA
  Chicago Illinois USA

  John -> Beaconsfield Bucks UK
  Michael -> Chicago Illinois USA
  Roger -> Chicago Illinois USA

In other words, by sorting the addresses, we made Roger move in with Michael. A serious breach of data integrity! (Replacing std::sort with std::stable_sort, I was able to let Roger move to Chicago and let John and Michael both move to Peckham. Other implementations of std::sort and std::stable_sort may have John, Michael and Roger move around between Beaconsfield, Chicago and Peckham in other ways.)

To use std::tuple<address&> as an element type for the vector addrs is a clever trick to store references in the vector. Too clever, in my opinion. Also note that we don’t want to change anything in the map when sorting the vector, but that changing the addrs’s type to std::vector<std::tuple<address const&>> will not compile. To understand why, we have to delve a little bit into what is happening in std::sort. Whatever actual sorting algorithm is used, std::sort will have to move around the elements in the vector. It can only do this by first moving one or more elements into temporary variables, then move other elements into the free spaces in the vector and finally move the elements from the temporary variables into the (new) free spaces in the vector. For example, say that element 0 and 1 have to be swapped. This will result in something similar to:

  auto temp = std::move(addrs[0]); // (1)
  addrs[0] = std::move(addrs[1]);  // (2)
  addrs[1] = std::move(temp);      // (3)

In (1), we move-construct a temporary variable of type std::tuple<address&>. This will initialize the embedded reference with std::forward<address&>( std::get<0>(addrs[0])), which, because of reference collapsing, means that the reference is copied. Now both temp and addrs[0] reference the same address object. Next, in (2), we move-assign from addrs[1] to addrs[0]. This assigns std::forward<address&>(get<0>(addrs[1])) to get<0>(addrs[0]). Again, reference collapsing applies, and we have an assignment of one lvalue-reference to another lvalue-reference. This is nothing more than copying the content of the address object referenced by addrs[1] to the address object referenced by addrs[0]. Finally, in (3), like in (2), we copy the content of the address object referenced by temp, which was changed via addrs[0] in (2), to the address object referenced by addrs[1]. As a result, we changed the content of the objects referenced by addrs[0] and addrs[1] to both have the value that the object referenced by addrs[1] had before. Sorting the vector thus modifies the map.

To get the desired behaviour of sorting the vector of addresses, we have to use pointers instead of references in the vector. This will also remove the need to use tuple, and we can also make everything related to the map (and the map itself) const. In the sort statement, we have to add a predicate to sort on the values pointed to instead of the pointers themselves. main now has become

  int main()
  {
    std::map<name_t, address> const addr_map
      = read(std::cin);

    // Don't copy the addresses
    std::vector<address const*> addrs;
    for (auto const& entry : addr_map)
    {
      addrs.push_back(&entry.second);
    }

    // sort and print the addresses
    std::sort(addrs.begin(), addrs.end(),
      [](auto a, auto b) { return *a < *b; });
    for (auto const& v : addrs)
    {
      std::cout << *v << '\n';
    }
    std::cout << '\n';
  }

Note that we can also use the algorithms std::transform to fill the vector from the map and std::for_each to print the addresses. This is left as an exercise for the reader.

Balog Pál

This code looks really interesting. However, before engaging, let me insert the rant I have been pushing back for the last few entries and as I really can’t take it any more. Iostreams. ☹ Can we say goodbye to them? It is only noise and distraction. If we still have related issues, what is the chance we have not pointed them out 36 times over in the 117 previous entries? Especially the input part that only fills our vector or map – all std:: collections have very fine {} initialization, the input(s) should just be created that way. For outputs, I’d also prefer using some simple framework like catch, those who just read printed code can hopefully understand it without a peek.

Having said that, I ignore the iostream parts of this sample. Ok, except for the missing const in the <<, but that is it. With that eliminated we have just half the code. Not even half, just 4 effective lines. How many problems can we fit in this tiny space?

Certainly my improved version would not compile having the map const as it should be… and the series of changes needed just to compile would likely remove the problems too. So let’s go somewhat more conservatively and leave the map mutable. While at it, mark the leading 2 lines for doing ‘create-then-assign’ instead of just initialize, and naming the map map… that is legal but not nice in my book.

A collection of references packed into a tuple is interesting to start with. Is that even valid? Let’s put that question aside and for the time being assume it is. We iterate over the map incorrectly. Range-based for without & is suspect and the norm is to have const& there. Otherwise it creates a copy of each item, which works at gimped speed when we just use the value and leads to catastrophic failure if we happen to store the address. Which is our case? Looks like the second: we bind the address to the copy not the original as intended. As a side note, emplace_back is a more fitting call here. Even considering that in modern C++, we get the same code for both because of mandatory copy elimination.

Then we sort the vector. We didn’t pass a compare function, so we’ll need an op< here. Ah, we have it too, just lost between the << >> noise. It looks bad whichever way you look at it, even with a quick glance. The current canonical form is simple: return tie(lhs.k1, lhs.k2... ) < tie(rhs.k1, rhs.k2... ). Yeah, no kidding. Maybe magical but will surely do what is needed, just arrange the same sequence of keys and no lhs/rhs typos once. If done by hand, then you take key one, return immediately if they are < or >, continue with the next key if neither. We miss the step right on the second line with a bad combination. With some luck, in a year’s time we can just ask for a defaulted <=> and roll with it.

Messing up the compare function is no small deal either, as we face UB if it breaks the rules of irreflexivity, transitivity and transitivity of equivalence. I’m too lazy to follow up with the actual data; it might be benign by pure luck but don’t count on it. Some STL implementations have nice instrumentation and might point out a problem for you, but again that is not a thing to rely on: just make sure your function is correct. If in doubt, ask an independent review. Some cases appear unintuitive, i.e. I have had to explain that “comparing doubles with tolerance is NOT legal in a sort function!” a few dozen times to the same set of people.

So, we have fixed the vector and the compare function, will we sort well now? Well, sort-of. ☺ Now it will sort something; the question is, how happy we are with that? As we use references in the vector, sort will swap around the values sitting in the map, slicing them off their original keys. So, we have the map itself ordered: John, Michael, Roger. And the addresses will be sorted too, as Bucks, London, Illinois, associated to the people based on that order. The last two guys may not be particularly enthusiastic about that.

Certainly, it would not be evident from the code that it outputs only the addresses, not bothering with the map yet.

With all problems addressed, it’s time to get back to the initial issue of whether collecting and sorting references, that are after all immutable beings (yeah, a reference is not even an object), is a fair deal or not. I honestly cannot tell for sure. I followed the criteria in the standard and got swamped at ‘equivalent’, which is inconveniently not defined. We have a nice answer on SO: https://stackoverflow.com/questions/37142510/can-i-sort-vector-of-tuple-of-references where the user Barry is adamant that MoveAssignable this way is not conforming on this last point (formally everything seems valid, as the type_traits functions report true). He may be right. But if ‘equivalent’ means equality or substitutability, then with references it kinda works in its weird way – as we never observe the ref itself just the object behind it. As long as they all look at a distinct object.

Interesting that Barry pointed out sort as bad, but if it is, similar requirement break happens on the vector too.

I pass this mystery over to Roger who will hopefully go into detail on both how to interpret the situation and why it is not clear from the standard text – we might get a core issue here, or at least an editorial addition of a note to shorten the search for the next time

Commentary

This code is an example of someone knowing just enough to be dangerous. The most ‘interesting’ line is the declaration of addrs:

  std::vector<std::tuple<address&>> addrs;

Why did they use a tuple with a single element? As Nicolas correctly stated, it’s because you can’t create a vector of references… but by wrapping the reference in a tuple you can. This critique partly answers the question of whether you should.

As several people commented, std::reference_wrapper is designed for this purpose: it wraps a reference in a copyable and assignable object. (Note that while James’s solution correctly creates a reference_wrapper object by using std::ref, but as it is then immediately used to construct a copy the reference nature is not retained.)

References in C++ are quite tricky – they are, to a certain extent, ‘invisible’ as you can’t really access a reference, it acts as what can best be described as an alias for an existing object.

So given a and b of type int& the expression a = b simply does an assignment of the referenced objects; and if the type of both changes to std::tuple<int&>, the assignment still assigns the referenced objects.

While references, and tuples of references, comply with the syntax of MoveAssignable (and std::is_move_assignable_v<> will be true), they do nt comply with the expected semantics.

Generally, const references are relatively safe (as long as the lifetime of the referenced object outlasts the reference) but non-const references can be troublesome, especially as member data whether directly or, as in this case, indirectly via a template argument.

Finally, Pál’s diatribe against iostream is partly answered, at least for output, by the merge of std::format into the working paper for C++20 at the recent WG21 meeting in Cologne. See http://wg21.link/P0645 for the formal wording. (We’ve also had several informal mentions of this in recent articles in Overload/CVu.)

The winner of CC 118

All the entrants successfully resolved the presenting problem. Several also noted that, for code that tried hard to avoid copies by using a ‘trick’ with a tuple, the range-for loop was copying each element. Marcel and Jan also noted that hoisting the strings name and address out of the loop in the read function would be a small performance win.

I like Hans’ step-by-step explanation of what actually breaks when sorting the collection of references; I hope this explanation would make it clearer to the programmer not only what they had done wrong but why the results were what they were!

The implementation of operator< received some comments – you have to read it quite carefully to check whether it is correct. It does seem that the message about using std::tie to implement these methods is getting heard as almost all the solutions removed the hand-crafted code.

There were good answers from all entrant, but in my opinion, Nicolas wins, by a short head!

Code Critique 119

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

I’m relatively new to C++ and I’m trying to build up a simple hierarchy of different entities, where their name and id and a few more characteristics are held in an Entity base class; and specific subclasses, such as Widget in the code below, hold the more specialised data – in this case simulated by the member name data. I’ve been having problems with the program I’m writing crashing and so I’ve simplified it down to a short example which usually produces output like this:

     Test
     none
     Another widget
     <core dump>

Please can you suggest what might be causing the core dump? I’ve had to use -fpermissive with gcc (but not msvc) – but surely that’s not causing the problem?

Listing 2 contains Entity.h, Listing 3 is Widget.h and Listing 4 is example.cpp. As always, there are a number of other things you might want to draw to the author’s attention as well as the presenting problem.

#pragma once
class Entity
{
private: 
  char* pName{ NULL }; 
  int idNumber{ 0 };

public:
  void Setup(char const* name)
  {
    pName = strdup(name);
  }

  virtual ~Entity()
  {
    delete pName;
  }

  // Delete the current object using the dtor,
  // and re-create as a blank object
  void Reset()
  {
    this->~Entity();
    new(this) Entity();
  }

  char *Name() 
  {
    return pName ? pName : "none";
  }
};
			

#pragma once
class Widget : public Entity
{
  int* data;
public:
  Widget() { data = new int[10]; }
  ~Widget() { delete data; }
};
			

#include <iostream>
#include <memory>
#include <string.h>

#include "Entity.h"
#include "Widget.h"

int main()
{
  Widget w;
  w.Setup("Test");
  std::cout << w.Name() << '\n';
  w.Reset();
  std::cout << w.Name() << '\n';
  w.Setup("Another widget");
  std::cout << w.Name() << '\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.

Roger Orr Roger has been programming for over 20 years, most recently in C++ and Java for various investment banks in Canary Wharf and the City. He joined ACCU in 1999 and the BSI C++ panel in 2002.

Notes: 

More fields may be available via dynamicdata ..