Code review comment for lp:~josharenson/unity8/doc_args

Revision history for this message
Albert Astals Cid (aacid) wrote :

I'm confused about the auto_ptr use

Wouldn't

std::auto_ptr<ApplicationArguments> appArgs;
if (parser.isSet(windowGeometryOption) &&
  parser.value(windowGeometryOption).split('x').size() == 2) {
  QStringList geom = parser.value(windowGeometryOption).split('x');
  appArgs.reset(new ApplicationArguments(geom.at(0).toInt(), geom.at(1).toInt()));
} else {
  appArgs.reset(new ApplicationArguments());
}
ApplicationArguments& qmlArgs = *appArgs;

make more sense as

ApplicationArguments qmlArgs;
if (parser.isSet(windowGeometryOption) &&
  parser.value(windowGeometryOption).split('x').size() == 2) {
  QStringList geom = parser.value(windowGeometryOption).split('x');
  qmlArgs.setSize(new ApplicationArguments(geom.at(0).toInt(), geom.at(1).toInt()));
}

or if you don't want to expose setSize (which is indeed fine since if you don't make it a q_invokable it won't be available from the qml side)

QSize size;
if (parser.isSet(windowGeometryOption) &&
  parser.value(windowGeometryOption).split('x').size() == 2) {
  QStringList geom = parser.value(windowGeometryOption).split('x');
  size = QSize(new ApplicationArguments(geom.at(0).toInt(), geom.at(1).toInt()));
}
ApplicationArguments qmlArgs(size);

?

Any of those two versions seem easier to read to me

// Note code not warranteed to compile :D

review: Needs Information

« Back to merge proposal