Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 550
  • Last Modified:

How to add a new object in a dynamic array in C++

Hi, I have trouble with adding a new object in a dynamic array. When compiling my program I get a warning stating that "Warning W8004...: 'temp' is assigned a value that is never used in function TransaktionsLista::laeggTill(Transaktion &)". But where have I gone wrong in my code (or in my head)?
Please keep it simple as I'm new to programming.
void TransaktionsLista::laeggTill ( Transaktion &t )
{	
  if ( antalTrans > 0 ) 
  {
    Transaktion *temp = 0 ;
    temp = new Transaktion[antalTrans+1] ;
    for ( int i = 0; i < antalTrans; i++ )
      temp[i] = trans[i] ;
      temp[antalTrans] = t ;
      delete [] trans ;
      trans = temp ;
      antalTrans++ ;
  }
  else 
  {
    trans = new Transaktion[1] ;
    trans[0] = t ;
    antalTrans++ ;
  }	
}

Open in new window

0
PiaPia
Asked:
PiaPia
  • 6
  • 4
  • 2
  • +2
1 Solution
 
evilrixSenior Software Engineer (Avast)Commented:
Why not make your life a whole lot easier by just using a standard C++ vector?
http://www.cplusplus.com/reference/stl/vector/
0
 
evilrixSenior Software Engineer (Avast)Commented:
>> Why not make your life a whole lot easier by just using a standard C++ vector?

Your example code (assuming I've read it correctly) could be simplified to this...

trans.push_back(t);
0
 
magicdlfCommented:
I agree with evilrix, a vector will solve all your problem.
In additional, your code looks correct for me, though the performance is very poor because you copy the array again and again.
0
Industry Leaders: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
PiaPiaAuthor Commented:
Sorry guys, I'm not allowed to use vectors.
0
 
stsanzCommented:
Take care of brackets in your "for" loop :

Did you mean :
    for ( int i = 0; i < antalTrans; i++ )
    {  // <--
      temp[i] = trans[i] ;
      temp[antalTrans] = t ;
      delete [] trans ;
      trans = temp ;
      antalTrans++ ;
    }  // <--

As it is written in your code, with no brackets,
only the line "temp[i] = trans[i] ;" will be iterated in your loop. Is it what you intended ?
0
 
evilrixSenior Software Engineer (Avast)Commented:
>> Did you mean
I very much doubt it... that will delete the trans BEFORE all the items have been copied to temp.

>> only the line "temp[i] = trans[i] ;" will be iterated in your loop. Is it what you intended ?
It is copying the array... I very much suspect this is what the code is meant to do.
0
 
stsanzCommented:
Ok sorry, indentation fooled me.

As evilrix said, for performance reasons you should not reallocate and copy the array at each iteration, espacially if your "trans" array will contain hundreds or thousands of elements.

You should allocate a whole bunch of elements at a time, more that is needed at the start, and reallocate the array only when the number of transactions overrides the allocated amount.
0
 
evilrixSenior Software Engineer (Avast)Commented:
>> As evilrix said, for performance reasons you should not reallocate and copy the array at each iteration,
I never said that -- but that isn't what's happening either. It's reallocated each time the function is called to extend the size of the array. A better strategy is to double the size of the array each time you reach the bounds and need to extend it... only extending it once you each the new limit of course.

PiaPia. What is Transaktion? If it's just a POD (plain old data) object you can avoid the for loop by just using memcpy to copy the data from one array to the other (note, you shouldn't do this if it is not a POD).

>> "Warning W8004...: 'temp' is assigned a value that is never used in function TransaktionsLista::laeggTill(Transaktion &)"
I have to be honest, I can see no valid reason for that warning. Are you sure the code you posted is the same code you are building? Can you move this code into a little stand alone test program that you can post here so we can try it ourselves?
0
 
stsanzCommented:
The warning comes from the two following lines :

Transaktion *temp = 0 ;
temp = new Transaktion[antalTrans+1] ;

Rewrite them as :
Transaktion *temp = new Transaktion[antalTrans+1] ;

and it will be ok.

0
 
ikeworkCommented:
Hey guys I think we're confusing PiaPia, (s)he is new eh ;)

PiaPia, the bottom-line is, your code is fine, it compiles and it works!

1) the warning is not a problem as its name says, stsanz showed you a way to get rid of it.

2) The code is not the most optimized version, but it depends on your assignment whether that matters or not.
The code works, if you want it to be more optimised, Rix gave you a hint how to do it.

Please dont award points to this posts, it was just a summarise of the things already said, just tried to make it more digestable ;)


ike
0
 
magicdlfCommented:
ike, I compiled her program myself, just as evilrix said, the code won't cause the warning. PiaPia, just take evilrix's advice and show us the code if possible.
0
 
ikeworkCommented:
>> the code won't cause the warning

How do you know? Do you use the same compiler? Nobody actually asked what compiler PiaPia uses.
To me stsanz explanation of this warning sounds reasonable, but we cant be sure, unless PiaPia tries it or somebody else with the same compiler/version does.

The point is, the warning can be ignored, the code as it is, is working (unless we oversaw something).
0
 
stsanzCommented:
I guess PiaPia uses Borland because I've googled "Warning W8004" and found the explantion there :
https://forums.embarcadero.com/thread.jspa?threadID=28406
http://stackoverflow.com/questions/1399692/w8004-compiler-warning-bds6-c-c

0
 
PiaPiaAuthor Commented:
The last couple of hours I've been trying to get my program to work, that is why I've been quite over here.

Stsanz solution to change:
Transaktion *temp = 0 ;
temp = new Transaktion[antalTrans+1] ;

to this code:
Transaktion *temp = new Transaktion[antalTrans+1] ;

Well, it eliminates the warning, but is it correct, I wonder.
I thought that you always have to start with putting your pointer to null. Or am I wrong?
0
 
evilrixSenior Software Engineer (Avast)Commented:
>> Well, it eliminates the warning,
Weird compiler... are you compiling with warning level set at the highest setting? If so that might explain it.

>> but is it correct, I wonder.
Perfectly correct.

>> I thought that you always have to start with putting your pointer to null. Or am I wrong?
Nope, it's entirely up to you. If it's not going to be assigned a value immediately it is good practice but that's all.
0
 
evilrixSenior Software Engineer (Avast)Commented:
BTW: Another way you might improve this, which is dependent on Transaktion being a POD is to create the array using malloc and use realloc to resize it as you need to. This will just extend the size of the memory allocated as required and avoids the necessity to copy the data from the old to the new array. Be careful to ensure you read how malloc/realloc works carefully though, a failed allocation returns NULL and you need to handle this correctly. Don't forget to use free() to release the memory once you are done (you don't need to call this between each realloc).

http://en.wikipedia.org/wiki/Plain_old_data_structures
http://www.cplusplus.com/reference/clibrary/cstdlib/realloc/
0

Featured Post

Upgrade your Question Security!

Add Premium security features to your question to ensure its privacy or anonymity. Learn more about your ability to control Question Security today.

  • 6
  • 4
  • 2
  • +2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now