CODEWITHSUNDEEP
c++
Questioneer
Questioneer

Reputation: 793

Quick new operator issue

I know this is really easy and I'm looking over something but this is what I have...:

typedef struct
{
    char s1[81];
    char s2[81];
    char s3[81];
}Rec;

int main()
{   

   Rec *a[10];

   a[0] = (Rec*)new unsigned char(sizeof(Rec));
   a[0]->s1= "hello";
   printf("a[0] = %s\n",a[0]->s1);
   delete(a[0]);


   getchar();
   return 0;
}

Now, the line

a[0]->s1= "hello";

is complaining about the expression must be a modifiable lvalue. I am pretty sure it's how I'm casting it in my new operator line and has it needs to be a long value or something but I'm not sure of the code to do this... easy i know but yeah. Any help would be much appreciated

Upvotes: 0

Views: 102

Answers (5)

Etienne de Martel
Etienne de Martel

Reputation: 36985

I guess that you've done a lot of C in your life. Keep in mind that C++ is different language, which happen to share with C most of its syntax and some of its standard library. That means something that is perfectly fine in C might be quite ugly (or even dangerous) in C++.

With that said, let's rewrite your code in a more "C++-ish" way:

#include <iostream> // std::cout, std::endl
#include <string> // std::string

struct Rec // typedef is implicit for structs in C++
{
    std::string s1; // use std::string instead of char arrays
    std::string s2;
    std::string s3;
}; // don't forget the semicolon!

int main()
{   

   Rec * a[10];

   a[0] = new Rec; // allocates the right amount of memory, no need to cast
   a[0]->s1 = "hello"; // std::sring handles the assignment for you
   std::cout << "a[0] = " << a[0]->s1 << std::endl; // use iostreams
   delete a[0]; // delete is an operator, not a function, no need for parentheses

   getchar(); // warning, this is not portable
   return 0;
}

As you see, new is not an "improved malloc". It's typesafe (no cast needed), it's safer to use (it allocates the exact amount of memory required, no need for sizeof), and it also does something that malloc cannot do: it invokes the class' constructor (just as delete invokes a destructor).

In C++, as in C, allocation is distinct from initialization. While in C you could just memset the block to zero, in C++ object construction can be a bit more complex. As such, you should never use malloc to create objects of classes that have non-trivial constructors (or have fields that don't have non trivial constructors - Rec is such a case). Because new always works, and has additional features, you should use it anyway.

Upvotes: 1

avakar
avakar

Reputation: 32635

Two things. The line

a[0] = (Rec*)new unsigned char(sizeof(Rec));

allocates a single unsigned char that gets initialized to the value sizeof(Rec). You probably meant

a[0] = (Rec*)new unsigned char[sizeof(Rec)];

or better yet

a[0] = new Rec;

Second, you cannot assign a string literal to an array of chars, you need to copy the characters one by one, e.g.

char s[80];
s = "hello"; // won't work
strcpy(s, "hello"); // correct

You should however use std::string in this case.

Upvotes: 3

OSH
OSH

Reputation: 2937

a[0] is a pointer to an array of chars that can't be modified -- a[0] will always point to the same address. you need to use strcpy to copy from your "hello" string to a[0]

Upvotes: 0

K-ballo
K-ballo

Reputation: 81379

The problem is not with your casting. Your new expression allocates a single unsigned char and initializes it to the sizeof(Rec) instead of allocating enough space as new unsigned char[sizeof(Rec)]; would do. That said, the types of s1 and "hello" are different and you can't assign one with the other. You should be using something like strcpy, but since you tagged this C++ then you would be better off using std::string. Also, why don't you just call new Rec;?

Upvotes: 0

Benjamin Lindley
Benjamin Lindley

Reputation: 103733

You cannot assign to char arrays like that. Either use strcpy, or change your char arrays to std::string.

strcpy(a[0]->s1, "hello");

Why are you doing this:

a[0] = (Rec*)new unsigned char(sizeof(Rec));

instead of this:

a[0] = new Rec;

Upvotes: 5

Related Questions