Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
adisidev opened this issue May 5, 2024 · 5 comments · Fixed by #167
Closed

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

adisidev opened this issue May 5, 2024 · 5 comments · Fixed by #167

Comments

@adisidev
Copy link
Collaborator

adisidev commented May 5, 2024

std::cerr << "Writing " << quadtree_filename << ".svg" << std::endl;

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

@adisidev
Copy link
Collaborator Author

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 5b78bf6, but I'm having second thoughts about it, and we should set some kind of precedent for future similar decisions.

@adisidev
Copy link
Collaborator Author

adisidev commented Jul 30, 2024

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
Copy link
Collaborator Author

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

@adisidev
Copy link
Collaborator Author

This is related to #124. Merging both issues to this one. Changing priority to high, it's a good idea to make main.cpp more uncluttered.

@adisidev
Copy link
Collaborator Author

I've now mainly fixed this in #167. More work to be done in #199 and #197.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant