5
\$\begingroup\$

If I draw something in Illustrator and extract it with AI2Canvas I want it to be customizable so I wrote this simple program. It adds two variables: one to resize and one to shift the path coordinate.

#include <fstream>
#include <regex>
#include <string>

int main(int argc, char *argv[]) {
  std::string path = argv[1];
  std::ifstream input(path);

  std::size_t found_name = path.find_last_of("/\\");
  std::size_t found_extension = path.find_last_of(".");
  const int steps = found_extension - found_name - 1;
  std::ofstream output(path.substr(found_name + 1, steps) + ".txt");

  std::regex pattern("(\\d+(\\.\\d+)?), (\\d+(\\.\\d+)?)");
  std::string in;
  while (!input.eof()) {
    std::getline(input, in);
    output << std::regex_replace(in, pattern, "($1 * resize.x) + pos.x, ($3 * resize.y) + pos.y")
           << '\n';
  }
  return 0;
}

Test file Raw: https://pastebin.com/fDh9qbd8

Test file Resolved: https://pastebin.com/R2gpXD8Q

\$\endgroup\$
  • 1
    \$\begingroup\$ Instead of modifying each pair of coordinates, you could also insert a single ctx.translate and ctx.rotate before the drawing commands. \$\endgroup\$ – Roland Illig Apr 27 '18 at 18:03
  • \$\begingroup\$ “Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems.” — Jamie Zawinski \$\endgroup\$ – JDługosz Apr 27 '18 at 20:49
4
\$\begingroup\$

Overall, good code. There is a trap code fell into though.

while (!input.eof())

One of the frequently encountered problems with iostreams. Check the output if it has one extra newline. I'm sure you'll find it. intput.eof() is true when EOF is read, not before it.

Instead, use stream >> variable as a condition or the function that behaves like that. The expression returns a reference to stream itself, but the stream is convertible to bool to see if some failure occured. Basically, the expression above is all these steps:

  1. Try to read.
  2. Set failure flags if error encountered
  3. Return the status

Put in a condition, it is the exact combination one needs to perform more or less controlled input. One needs to check if the read succeeded before doing anything. Some other functions in standard library behave like operator>>, std::getline() included.

As a result of above, the first line of the loop would just become a condition, and everything else intact.

while (std::getline(input, in)) {
    output << std::regex_replace(in, pattern, "($1 * resize.x) + pos.x, ($3 * resize.y) + pos.y")
       << '\n';
}

Smaller things follow.

Usage message would be nice. Usually frequently used utilities are put away from the code. Though in case of single argument programs it might not be much of a problem.

std::ofstream output(path.substr(found_name + 1, steps) + ".txt");

Is a little bit alerting. Since this is end user program (e.g. for a human), they might get something wrong.

Some missing consts here and there.

return 0 is redundant.

\$\endgroup\$
  • \$\begingroup\$ I don't have much experience with regex, but it looks correct and seem to run correctly too. I hope I didn't miss the elephant in the room. \$\endgroup\$ – Incomputable Apr 27 '18 at 18:01
  • 1
    \$\begingroup\$ @RolandIllig, I guess I amended my post just before you posted the comment :) I added more words into the sentence about operator>>. I just wanted to base my explanation off of operator>>, then build from there. \$\endgroup\$ – Incomputable Apr 27 '18 at 18:04
  • \$\begingroup\$ How about while (input.good() && std::getline(input, line)) {}? \$\endgroup\$ – yuri Apr 27 '18 at 18:11
  • \$\begingroup\$ Regarding the regex it probably could be simplified to ([0-9.]+), ([0-9.]+) \$\endgroup\$ – yuri Apr 27 '18 at 18:15
  • \$\begingroup\$ @yuri, it seems like operator bool is a superset of good() (1, 2, 3). They have different tables, look at the first link. Thus good() is redundant. I could perform parsing of standard, but that will take more than a day :) \$\endgroup\$ – Incomputable Apr 27 '18 at 18:15
2
\$\begingroup\$

a short tip: use Raw strings to avoid the nested escapes.

Use auto (almost everywhere).

const int steps = found_extension - found_name - 1;

and by using auto you won’t have this unsigned to signed implicit conversion cluttering up your warnings window. You do compile without generating warnings, right?

\$\endgroup\$
  • \$\begingroup\$ Yes, I do compile without warnings. I come back to cpp after 4 month, I started learning JavaScript in the meanwhile, so when I was declearing that variable I just put const, than the compiler droped an error and I realize that I was writing cpp, so I added int and left It has It is... Sad story \$\endgroup\$ – DiDi Apr 27 '18 at 21:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.