CODEWITHSUNDEEP
c++constructorconstants
Elazar Leibovich
Elazar Leibovich

Reputation: 33593

Initializing C++ const fields after the constructor

I want to create an immutable data structure which, say, can be initialized from a file.

class Image {
public:
   const int width,height;
   Image(const char *filename) {
     MetaData md((readDataFromFile(filename)));
     width = md.width();   // Error! width  is const
     height = md.height(); // Error! height is const
   }
};

What I could do to fix the problem is

class Image {
   MetaData md;
public:
   const int width,height;
   Image(const char *filename):
     md(readDataFromFile(filename)),
     width(md.width()),height(md.height()) {}
};

However

  1. It forces me to save MetaData as a field in my object. Which I don't always want.
  2. Sometimes the logic in the constructor is much more complex than a single read (say, error handling can take a few lines)

So the only solution I thought of is along the lines of

class A {
  int stub;
  int init(){/* constructor logic goes here */}
  A():stub(init)/*now initialize all the const fields you wish
  after the constructor ran */{}
};

Is there a better idea? (In Java, you're allowed initializing finals in the constructor).

Upvotes: 14

Views: 9999

Answers (10)

celavek
celavek

Reputation: 5705

You could cast away the constness in the constructor:

class Image {
public:
    const int width,height;
    Image(const char *filename) : width(0), height(0) {
        MetaData md(readDataFromFile(filename));

        int* widthModifier = const_cast<int*>(&width);
        int* heightModifier = const_cast<int*>(&height);
        cout << "Initial width " << width << "\n";
        cout << "Initial height " << height << "\n";
        *widthModifier = md.GetWidth();
        *heightModifier = md.GetHeight();
        cout << "After const to the cleaners " << width << "\n";
        cout << "After const to the cleaners " << height << "\n";
    }
};

That would achieve what you want to do but I must say I personally would stay away from that because it causes undefined behavior according to the standard (excerpt from cppreference)

const_cast makes it possible to form a reference or pointer to non-const type that is actually referring to a const object ... Modifying a const object through a non-const access path ... results in undefined behavior.

I would fear any public data members(at least in regarding your particular example). I would go with Georg's approach or make the data private and provide only the getter.

Upvotes: 2

HesNotTheStig
HesNotTheStig

Reputation: 568

This is one of my least favorite aspects of C++ when compared to Java. I'll use an example I was working on when I needed to solve this problem.

What follows is the equivalent of a readObject method. It deserializes a Video key from a provided file path. 

#include <fstream>
#include <sstream>
#include <boost/archive/binary_iarchive.hpp>
#include <boost/archive/binary_oarchive.hpp>

using namespace std;
using namespace boost::filesystem;
using namespace boost::archive;

class VideoKey
{
  private:
    const string source;
    const double fps;
    const double keyFPS;
    const int numFrames;
    const int width;
    const int height;
    const size_t numKeyFrames;
    //Add a private constructor that takes in all the fields
    VideoKey(const string& source,
      const double fps,
      const double keyFPS,
      const int numFrames,
      const int width,
      const int height,
      const size_t numKeyFrames)
    //Use an initializer list here
    : source(source), fps(fps), keyFPS(keyFPS), numFrames(numFrames), width(width), height(height), numKeyFrames(numKeyFrames)
    {
      //Nothing inside this constructor
    }
  public:
    //Then create a public static initializer method that takes in
    //the source from which all the fields are derived
    //It will extract all the fields and feed them to the private constructor
    //It will then return the constructed object
    //None of your fields are exposed and they are all const.
    const static VideoKey create(const path& signaturePath)
    {
      const path keyPath = getKeyPath(signaturePath);
      ifstream inputStream;
      inputStream.open(keyPath.c_str(), ios::binary | ios::in);
      if (!inputStream.is_open())
      {
        stringstream errorStream;
        errorStream << "Unable to open video key for reading: " << keyPath;
        throw exception(errorStream.str().c_str());
      }
      string source;
      double fps;
      double keyFPS;
      int numFrames;
      int width;
      int height;
      size_t numKeyFrames;
      {
        binary_iarchive inputArchive(inputStream);
        inputArchive & source;
        inputArchive & fps;
        inputArchive & keyFPS;
        inputArchive & numFrames;
        inputArchive & width;
        inputArchive & height;
        inputArchive & numKeyFrames;
      }
      inputStream.close();
      //Finally, call your private constructor and return
      return VideoKey(source, fps, keyFPS, numFrames, width, height, numKeyFrames);
    }

Upvotes: 0

indrajit
indrajit

Reputation: 1

class A
{
public:
    int weight,height;

public:
    A():weight(0),height(0)
    {
    }

    A(const int& weight1,const int& height1):weight(weight1),height(height1)
    {
        cout<<"Inside"<<"\n";
    }
};

static A obj_1;

class Test
{
    const int height,weight;

public:
    Test(A& obj = obj_1):height(obj.height),weight(obj.weight)
    {
    }

    int getWeight()
    {
        return weight;
    }

    int getHeight()
    {
        return height;
    }
};

int main()
{
    Test obj;

    cout<<obj.getWeight()<<"\n";

    cout<<obj.getHeight()<<"\n";

    A obj1(1,2);

    Test obj2(obj1);

    cout<<obj2.getWeight()<<"\n";

    cout<<obj2.getHeight()<<"\n";

    return 0;
}

As far my understanding i think this mechanism will work.

Upvotes: 0

Sadeq
Sadeq

Reputation: 8033

If it was C++0x, I would recommend this (delegating constructors):

class Image
{
  public:

    const int width, height;

    Image(const char* filename) : Image(readDataFromFile(filename)) { }
    Image(const MetaData& md) : width(md.width()), height(md.height()) { }
};

Upvotes: 5

Matthieu M.
Matthieu M.

Reputation: 299790

You can simply use the NamedConstructor idiom here:

class Image
{
public:
  static Image FromFile(char const* fileName)
  {
    MetaData md(filename);
    return Image(md.height(), md.width());
  }

private:
  Image(int h, int w): mHeight(h), mWidth(w) {}

  int const mHeight, mWidth;
};

One of the main advantage of Named Constructors is their obviousness: the name indicates you are building your object from a file. Of course it's slightly more verbose:

Image i = Image::FromFile("foo.png");

But that never troubled me.

Upvotes: 9

Frank Osterfeld
Frank Osterfeld

Reputation: 25155

I'd use a static method:

class Image {
public:
    static Image* createFromFile( const std::string& filename ) {
        //read height, width...
        return new Image( width, height ); 
    } 

    //ctor etc...
}

Upvotes: 0

Chubsdad
Chubsdad

Reputation: 25497

How about passing MetaData as an argument to the constructor. This gives a lot of benefits:

a) The constructor interface makes it clear about the dependency on MetaData. b) It facilitates testing of the Image class with different types of MetaData (subclasses)

So, I would probably suggest similar to follows:

struct MD{
   int f(){return 0;}
};

struct A{
   A(MD &r) : m(r.f()){}
   int const m;
};

int main(){}

Upvotes: 0

Notinlist
Notinlist

Reputation: 16640

You should add inline getters for the width and height instead of public const member variables. The compiler will make this solution as fast as the original try.

class Image {
public:
   Image(const char *filename){ // No change here
     MetaData md((readDataFromFile(filename)));
     width = md.width();
     height = md.height();
   }
   int GetWidth() const { return width; }
   int GetHeight() const { return height; }
private:
   int width,height;
};

P.S.: I used to write private things at the end because they are less important for the user of the class.

Upvotes: 4

GManNickG
GManNickG

Reputation: 503815

First, you should understand the constructor body is just for running code to complete initializing your object as a whole; the members must be completely initialized before the body is entered.

Ergo, all members are initialized in an (implicit unless made explicit) initialization list. Clearly, const variables must be initialized in the list because once you enter the body, they are already suppose to be initialized; you'd simply be trying to assign them.

Generally, you don't have const members. If you want those members to be immutable, just don't give any public access to them that could change them. (Also, having const members make your class non-assignable; typically unnecessarily.) Going this route easily fixes your problem, as you'd just assign them values in the body of the constructor like you wish.

A method to do what you want while maintaining const could be:

class ImageBase
{
public:
    const int width, height;

protected:
    ImageBase(const MetaData& md) :
    width(md.width()),
    height(md.height())
    {}

    // not meant to be public to users of Image
    ~ImageBase(void) {} 
};

class Image : public ImageBase
{
public:
    Image(const char* filename) : // v temporary!
    ImageBase(MetaData(readDataFromFile(filename)))
    {}
};

I don't think this route is worth it.

Upvotes: 3

Georg Fritzsche
Georg Fritzsche

Reputation: 98984

You could move width and height into one type and move the initialization code into an initialization helper function:

// header:
struct Size { 
    int width, height;
    Size(int w, int h) : width(w), height(h) {}
};

class Image {
    const Size size; // public data members are usually discouraged
public:
    Image(const char *filename);
};

// implementation:
namespace {
    Size init_helper(const char* filename) {
        MetaData md((readDataFromFile(filename)));
        return Size(md.width(), md.height());
    }
}

Image::Image(const char* filename) : size(init_helper(filename)) {}

Upvotes: 12

Related Questions