tags:

views:

58

answers:

3

I've got class which is using plain-only-data struct with const variables and I'm not sure, if I'm allocating these structures in a proper way. It looks more or less like:

#include <cstdlib>
#include <iostream>
using std::cout;
using std::endl;

struct some_const_struct {
     const int arg1;
     const int arg2;
};

class which_is_using_above_struct {
 private:

  some_const_struct* m_member;

  const some_const_struct* const m_const_member;

 public:

  const some_const_struct& get_member() const { return *m_member; }

  const some_const_struct& get_const_member() const { return *m_const_member; }

  void set_member(const int a, const int b) {
   if(m_member != NULL) {
    delete m_member;
    m_member = NULL;
   }
   m_member = new some_const_struct((some_const_struct){a, b});
  }

  explicit which_is_using_above_struct(const int a, const int b)
   : m_const_member(new some_const_struct((const some_const_struct){a, b})) {
   m_member = NULL;
  }

  ~which_is_using_above_struct() {
   if(m_member != NULL) {
    delete m_member;
   }
   if(m_const_member != NULL) {
    delete m_const_member;
   }
  }

};

int main() {
 which_is_using_above_struct c(1, 2);
 c.set_member(3, 4);
 cout << "m_member.arg1 = " << c.get_member().arg1 << endl;
 cout << "m_member.arg2 = " << c.get_member().arg2 << endl;
 cout << "m_const_member.arg1 = " << c.get_const_member().arg1 << endl;
 cout << "m_const_member.arg2 = " << c.get_const_member().arg2 << endl;
 return 0;
}

I'm just not quite sure if the statement:

m_member = new some_const_struct((some_const_struct){a, b});

doesn't produce unnessesary use of some_const_struct's copy constructor, ergo allocating that struct twice. What do you think? And is it reasonable to make that struct's members const? (they're not supposed to change in their lifetime at all)

A: 

Why not write a constructor for the struct :

struct some_const_struct 
{
    const int arg1;
    const int arg2;
    some_const_struct(int a, int b) : arg1(a), arg2(b){}
};

Then you can use :

m_member = new some_const_struct(a, b);

avoiding a needless copy and verbose code.

rep_movsd
Wow that was fast :D I need to have some_const_struct plain-data-only, becouse (afaik) I'm not able to put structs with constructors in shared memory (there're being used some unions etc.). I'm just curious if that solution you find acceptable or I should redesign it someway.
Willy
@Willy: It seems to me that trying to put a struct with const member functions in an union leads to a compiler error in the first place. If you give it a constructor, then it isn't a POD, and you shouldn't be putting it in an union either. - Just make the fields non-const. You are over-zealous with const-correctness, particularly as you otherwise want to program in a C style. The union, even if you manage to get it to compile, will not respect the constness of a sub-field.
UncleBens
@UncleBens: it seems so you're right. Thanks :)
Willy
+1  A: 

Making the struct members const means that you should assign them in your structure constructor (yes a struct can have a constructor).

struct some_const_struct {
     const int arg1;
     const int arg2;

     some_const_struct(int n1, int n2) : arg1(n1), arg2(n2) {}
};

I don't understand why you want m_member not to be const, since you are not modifying it.

You may actually be better using non-const members in your struct since you are declaring the whole struct const whenever you need it.

Vincent Robert
+1  A: 
m_const_member(new some_const_struct((const some_const_struct){a, b}))

Such syntax is not allowed by standard C++ and is only supported by your compiler as an extension.

I think the only standard-compiliant way with the current standard1 would be to use copy-construction and a helper function to initialize the struct in a portable way.2

inline some_const_struct make_some_const_struct(int a, int b)
{
    some_const_struct x = {a, b};
    return x;
}

//usage:
m_const_member(new some_const_struct(make_some_const_struct(a, b))) 

I think you should just make your life easier and not use const members, particularly since set_member allows you to modify the fields anyway (at a great cost both coding- and performance-wise).

You could also give constructors3 to the classes, but then they won't be Plain Old Data structs any more, if that is important to you. (Actually I'm not sure whether a struct with const members qualifies as POD in the first place: e.g memsetting and memcpying such a struct would violate the constness of the members, but should be legal things to do with real POD structs.)


1 AFAIK, C++0x will revise the rules concerning {} in initialization, making it easier to achieve what you want.

2 Under favorable conditions, the compiler should be able to optimize out the needless copy, transforming it into the exact equivalent of what you are doing.

3 Comeau's compiler appears to be very helpful. It even tells you what (it thinks) you should do: warning: class "some_const_struct" defines no constructor to initialize the following: ...

UncleBens
Now I realized that I've unnecessarily written that set_member modifier, since I'm not using it anywhere in actual code. So if it works thanks to my compiler's extension I really should find other way. Thanks for answer! :)re 2) I'm not very familiar to valgrind, but actually he tells me that each member is allocating only once (2 memcpy's during runtime, I think its one per member). And yes, I'm using -std=c++0x
Willy