Code review comment for lp:~tapaal-approx/tapaal/tapaal-approx-new

Revision history for this message
Kenneth Yrke Jørgensen (yrke) wrote :

Just a quick question about the following lines:

910 s.append((query != null) ? (query.useSymmetry() ? "Yes" : "No") : "");
911 s.append(DELIMITER);
912 s.append((query != null) ? getVerificationMethod(query) : "");
913 + s.append(DELIMITER);
914 + s.append((query != null) ? getApproximationMethod(query) : "");
915 + s.append(DELIMITER);
916 + s.append((query != null) ? query.approximationDenominator() : "");

For me it seems more natural to make one if one query and then do all the append stuff:

if (query != null) {
  //build string
  s.append(query.useSymmetry() ? "Yes" : "No");
  s.append(....)
  ...
}

The other way seams a little strange to me (unless there is something special about how query works, which really should be fixed then). (i know i was done this way in the original code as well, but i still think it needs to be fixed)

review: Needs Fixing (code)

« Back to merge proposal