Question

In the ArduinoUnit unit testing library I have provided a mechanism for giving a TestSuite a name. A user of the library can write the following:

TestSuite suite("my test suite");
// ...
suite.run(); // Suite name is used here

This is the expected usage - the name of the TestSuite is a string literal. However to prevent hard-to-find bugs I feel obliged to cater for different usages, for example:

char* name = (char*) malloc(14);
strcpy(name, "my test suite");
TestSuite suite(name);
free(name);
// ...
suite.run(); // Suite name is used here

As such I have implemented TestSuite like this:

class TestSuite {
public:
  TestSuite(const char* name) {
    name_ = (char*) malloc(strlen(name) + 1);
    strcpy(name_, name);
  }

  ~TestSuite() {
    free(name_);
  }

private:
  char* name_;
};

Putting aside the issue of failing to deal with memory allocation failures in the constructor I'd prefer to simply allocate the pointer to a member variable like this:

class TestSuite {
public:
  TestSuite(const char* name) : name_(name) {
  }

private:
  const char* name_;
};

Is there any way I can change the interface to force it to be used 'correctly' so that I can do away with the dynamic memory allocation?

Was it helpful?

Solution

What if you provide two overloaded constructors?

TestSuite(const char* name) ...
TestSuite(char* name) ...

If called with a const char*, then the constructor could make a copy of the pointer, assuming that the string will not go away. If called with a char*, the constructor could make a copy of the whole string.

Note that it is still possible to subvert this mechanism by passing a const char* to the constructor when the name is in fact dynamically allocated. However, this may be sufficient for your purposes.

I should note that I have never actually seen this technique used in an API, it was just a thought that occurred to me as I was reading your question.

OTHER TIPS

Documentation. For example,

/**
* Test suite constructor.
* @param name test suite name cstring, shared
*/
TestSuite(char const *name) {
// ...

A shared pointer implies that the pointed object must be alive during the lifetime of this object.

Well, you can use a std::string that will take care of all memory allocation

class TestSuite {
public:
  TestSuite(const std::string &name):name_(name) {
  }

  ~TestSuite() {
  }

private:
  std::string name_;
};

Edit : If it is the call to malloc() that you want to avoid you could do this :

class TestSuite {
public:
  TestSuite(const char *name){
    memcpy(name_, name, min(16, strlen(name));
  }

private:
  char name_[16];
};

This will waste some memory however, which can be an issue on embedded platforms.

Have a char name[XYZ] member of your TestSuite (with an XYZ large enough to comfortably display the name) and use strncpy to copy the string (with a maximum length of XYZ-1).

Why are you using char* and malloc when you have the nice C++ string class which can takes a string literal or a char* in its constructor ?

Are you able to use std::string? You could have it as std::string name_ and have the STL take care of the memory allocation for you..

class TestSuite {
    public:
      TestSuite(const char* name) : name_(name) {}

      ~TestSuite() {}

    private:
      std::string name_;
};

Don't forget to include <string>.

Reference

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top