mgastner / cartogram-cpp

Cartogram generator in C++
MIT License
8 stars 4 forks source link

Move control output to functions (rather than in main.cpp) #172

Open adisidev opened 4 months ago

adisidev commented 4 months ago

https://github.com/mgastner/cartogram-cpp/blob/99f478ae5f199fd0e4c3099e49bb6d6c1b91c2bd/src/main.cpp#L287

For example, here, instead of manually printing "Writing " ... .write should do it automatically.

adisidev commented 1 month ago

I have already moved all the writing control output to within the functions, but this is something that we should probably discuss further idealogically @mgastner

For instance, I made this change in 5b78bf6a495e18f1f829790fd65971b55932493c, but I'm having second thoughts about it, and we should set some kind of precedent for future similar decisions.

adisidev commented 1 month ago

Further, I believe we should do the same with error handling:

    // Read visual variables (e.g., area and color) from CSV
    try {
      cart_info.read_csv(arguments);
    } catch (const std::system_error &e) {
      std::cerr << "ERROR reading CSV: " << e.what() << " (" << e.code() << ")"
                << std::endl;
      return EXIT_FAILURE;
    } catch (const std::runtime_error &e) {

      // If there is an error, it is probably because of an invalid CSV file
      std::cerr << "ERROR reading CSV: " << e.what() << std::endl;
      return EXIT_FAILURE;
    }

This should be done within read_csv.

As a final bonus, we could do it with the timing function (though I'm not sure if this is desirable, and whether it actually reduces readability). We can make a friend class, ScopedTimer which we can call like so:


void InsetState::fill_with_density(double plot_density, bool enable_timing) {
    ScopedTimer timer(time_tracker_, "Fill with Density");
}

As soon as scope is exited, timing "Fill with density" will stop.

adisidev commented 1 week ago

Potentially, we could use decorators for this @nihalzp https://refactoring.guru/design-patterns/decorator/cpp/example