Could my code be written cleaner?

[rex1200]

Hey guys,

I've got some lengthy code in my lab that I think could be written a little cleaner but don't really see a way how right now. Does anyone think they can write these functions a little cleaner?

// int size = 0;
// int capacity = 2;
template <class DataType>
void Checkbook<DataType>::writeCheck(const DataType amount)
{
  if (lastCheck[capacity-1] != 0) 
    doubleArray();
  int i;
  for(i=0;i<capacity;i++)
  {
    if (lastCheck[i] == 0) break;
  }
  size++;
  lastCheck[i] = amount;
  balance = balance - amount;
}

template <class DataType>
void Checkbook<DataType>::doubleArray()
{
  DataType* temp = new DataType[2*capacity];
  int i;
  for (i=0;i<2*capacity;i++)
  {
    temp[i] =0;
  }
  
  for (i =0; i<capacity ; i++)
  {
    temp[i] = lastCheck[i];
  }
  delete[] lastCheck;
  lastCheck=temp;
  capacity = capacity *2;
  
}

[keskiverto]

What if you add a value, where amount == 0?

std::vector has both size and capacity. You apparently do have member "size" too. Make use of it.

In doubleArray() you write 0 to entire new array, but then overwrite half of it with the contents of old. If you do trust the size to always point to the next unused element, then you should not need to care what the unused elements contain, i.e. no need to zero-initialize at all.

[Ryan Wilson]

balance = balance - amount;
capacity = capacity *2;

balance -= amount;
capacity *= 2;

:P its cleaner!

[coder777]

I don't get what you're trying to do.
In line 14: i is out of bounds (except i is lager than capacity?)

What is the meaning of lastCheck?

You can easily make one loop from the two in doubleArray():

  for (i=0;i<2*capacity;i++)
  {
  if(i<capacity)
    temp[i] = lastCheck[i];
  else
    temp[i] =0;
  }

[Duthomhas]

No, get rid of the branch.

for (i=0; i<capacity; i++)
{
  temp[i] = lastCheck[i];
}
std::fill(temp+capacity, temp+(2*capacity), 0);

I'm not so sure "doubleArray()" is the best name. Specifically, you are doubling the array's capacity. Maybe "doubleCapacity()"?

There's really nothing egregiously wrong with the code. You should be aware that simply doubling the capacity can quickly make things hard to use. I would advocate adding a specific amount to the array each time.