0

So I have the following class: 

class smartA
{
public:

int *p;
int size;

smartA(){size=10; p = new int [size];}
smartA (int x){size=x; p = new int [x];}
smartA(int y[], int x ){size=x; p = new int [x]; for (int i=0 ; i<x ; i++) p[i]=y[i];}
smartA(const smartA &a) {*this=a;}
~smartA(){delete [] p;}

void displayA()
{
for (int i=0; i<size; i++)
{   cout<<setw(4)<<p[i];
if (((i+1)%5)==0 && i!=0)
cout<<endl;}
    cout<<endl<<endl;
}

void setsmartA(const int a[], int sizea) 
{size=sizea; p = new int[size]; for (int i=0 ; i<size ; i++) p[i]=a[i];}

};

How can I write a function that merges two smart array objects into a third smart array objects. I am having trouble accessing the elements of each smart array since it has to be a dynamic array.

for example the adding following member function gives me an error:

smartA add(smartA a)
{
smartA c(a.size+size);

int i=0;
for ( ; i<a.size ;i++)
c.p[i]=a.p[i];

for (int j=0; j<a.size+size; j++, i++)
c.p[i]=p[j];

return c;}
Improve this question
11
  • 4
    Is there a reason you can't use std::vector? And what does "merge" mean in this context? Commented Apr 23, 2013 at 15:50
  • What error does it give? Also, you might want to try encapsulating your data. Commented Apr 23, 2013 at 15:50
  • Fix your indentation. Also, never implement your copy constructor in terms of copy assignment (if anything, it should be the other way round). Especially not if you then don't have a custom copy assignment operator. You're basically violating the rule of three. Commented Apr 23, 2013 at 15:50
  • 2
    Your smart-array is not smart enough {*this=a;} memory leak !! Commented Apr 23, 2013 at 15:52
  • add is not a method of smartA so it does not have access to member variables like size and p, if you add it the class it does compile Commented Apr 23, 2013 at 15:53

2 Answers 2

Reset to default
1

How can I write a function that merges two smart array objects into a third smart array objects. [...] The adding following member function gives me an error.

Unless inlined in the class definition, smartA add(smartA a) should be smartA smartA::add(smartA const& a). This because otherwise add would be seen as a generic function outside the class. Notice that it makes sense for you to pass a reference to add instead of a copy.

Also, in the context of arrays it makes sense to overload the operator+ instead of calling an add method. So you might want to implement add in:

friend smartA smartA::operator+(smartA const&, smartA const&);

Finally you have a very big problem in your copy constructor:

smartA(const smartA &a) {*this=a;}

This can cause aliasing and lead to crash or memory issues. You want to take a look at deep copies and The rule of Three.

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

"memory interference" - that's a first time term. I suspect you may have a background in electrical engineering?
It's also not really 'aliasing' (well, in the strict sense, but not the common usage of the term). It's just broken ownership semantics. You know, it's not really a problem if no class owned the pointer.
@sehe, as defined in the wikipedia link: "aliasing describes a situation in which a data location in memory can be accessed through different symbolic names in the program. Thus, modifying the data through one name implicitly modifies the values associated to all aliased names". This is exactly the case. You have two objects that share the same int* pointer.
0

Pass the parameters by reference to a friend function. Return by value.

class smartA
{
  int *p;
  int size;

public:
....
  friend smartA operator+ (const SmartA& sa1, const SmartA& sa2);
};

smartA operator+ (const SmartA& sa1, const SmartA& sa2)
{
  SmartA res(sa1.size + sa2.size);

  for(int i = 0; i < sa1.size; i++)
    res.p[i] = sa1.p[i];

  for(int i = sa1.size, j = 0; i < sa1.size + sa2.size; i++, j++)
    res.p[i] = sa2.p[j];

  return res;
}

I made the members private unlike in your code snippet. It's always best to begin by encapsulating information and expose it if need be, rather than the other way around.

Also, You don't have to make the function into operator+, or a friend for that matter. I just like it for the symmetry it offers. (And the flexibility of the operator working seamlessly with type conversion into smartA, shuold you add those).

Improve this answer

7 Comments

I'm not so sure that's a good solution. Iterators or an operator[] might be better.
I tried implementing your code but I keep getting a debug assertion failed error. "Expression:_BLOCK_TYPE_IS_VALID(pHead->nBlockUse)
@Wilbert, for concatenating two arrays!? I's a philosophical question, IMHO. If the option to concatenate objects wasn't meant to be part of the API I'd go with adding an iterator protocol that complies with the standard algorithm library. But I'd also use a vector instead of implementing my own class for a smart array, if the standard library is involved.
@user2312038, there is a problem with your working environment, or your'e not looking in the right place for the problem. Since I don't see where that error can pop up in my code snippet.
@user2312038, see Jueecy.new's answer for the possible reason you get an error. It may very well be your copy c'tor.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.