ZipFile::Builder and ProgressBar


#1

Hi Jules,

would you consider adding the following changes to the ZipFile::Builder class to allow monitoring with a ProgressBar. It’s not perfect as it only shows the progress of processed files but not file size, but from a quick glance including the latter looks quite more complicated.

class Builder
{
// [...]
  bool writeToStream (OutputStream& target, double* progress = 0) const;
// [...]
};

bool ZipFile::Builder::writeToStream (OutputStream& target, double* progress) const
{
  const int64 fileStart = target.getPosition();

  if (progress != 0)
    *progress = 0;

  int i;
  for (i = 0; i < items.size(); ++i)
  {
    if (! items.getUnchecked (i)->writeData (target, fileStart))
      return false;

    if (progress != 0)
      *progress = i / (double) items.size();
  }
  const int64 directoryStart = target.getPosition();

  if (progress != 0)
    *progress = (items.size()-0.5) / (double) items.size();

  for (i = 0; i < items.size(); ++i)
    if (! items.getUnchecked (i)->writeDirectoryEntry (target))
      return false;

  const int64 directoryEnd = target.getPosition();

  target.writeInt (0x06054b50);
  target.writeShort (0);
  target.writeShort (0);
  target.writeShort ((short) items.size());
  target.writeShort ((short) items.size());
  target.writeInt ((int) (directoryEnd - directoryStart));
  target.writeInt ((int) (directoryStart - fileStart));
  target.writeShort (0);

  if (progress != 0)
    *progress = 1;


  return true;
}

Alternatively to the given code, progress could be a public member, but then writeToStream couldn’t be const any more.

Chris


#2

Yes, that sounds like a good idea, I’ll have a look.


#3

Hi Jules,

I just saw the changes you made, but wouldn’t it be better to have a default nullptr in the function prototype:

-bool writeToStream (OutputStream& target, double* progress) const;
+bool writeToStream (OutputStream& target, double* progress = nullptr) const;

As it is now, it breaks existing code (though it’s not that severe).

Chris


#4

What is this nonsense with a pointer to double?

writeToStream (OutputStream& target,
               std::function <void (double progress)> progressFunction)
{
//...

#5

@ckk: I’m trying to avoid using default parameters anywhere in my code these days (an idea picked up from the google coding standards)

@Vinnie: one day when everyone’s using c++11, it’ll be great to change the code to use lambdas for things like this! In the meantime, I’m sticking to the simple pattern that’s used elsewhere for progress updates…


#6

Wow, I wish I knew about this sooner:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml


#7

+1