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

Make our use of Exceptions consistent #48

Open
Patamap opened this issue Jun 9, 2020 · 7 comments
Open

Make our use of Exceptions consistent #48

Patamap opened this issue Jun 9, 2020 · 7 comments
Assignees
Labels
Enhancement New feature request or improvement or optimization. Good First Issue These issues require minimal context and are well-suited for new contributors.

Comments

@Patamap
Copy link

Patamap commented Jun 9, 2020

Make our use of Exceptions consistent - for example remove any boost exceptions handling at higher levels. Any special exception handling should be caught at lower levels, and moja exceptions passed on. The system should use it’s own collection of std exceptions.

Please feel free to contact Jim via Slack, https://app.slack.com/client/T1G1M5HPF/D014VTVKJEA for further questions/comments.

@Patamap Patamap added the Enhancement New feature request or improvement or optimization. label Jun 9, 2020
@Patamap Patamap added the Good First Issue These issues require minimal context and are well-suited for new contributors. label Oct 5, 2020
@AlkaDas991
Copy link

Hello @Patamap I would like to work on this issue, so can you please elaborate what I need to do to fix this issue?

@Patamap Patamap assigned Patamap and AlkaDas991 and unassigned Patamap Mar 21, 2021
@AlkaDas991
Copy link

@Patamap can you please explain me a little bit about the issue

@aornugent
Copy link
Contributor

@mfellows - could you please add a brief description to help @AlkaDas991? I know exception handling was one of your bugbears.

@mfellows
Copy link
Contributor

mfellows commented Apr 9, 2021

Hi Alka, thanks for your interest in this issue -

One example would be spatialtiledlocaldomaincontroller.cpp lines 794-840, you can see the huge variety of exceptions in there - which in theory should be helpful to narrow errors down, but in reality you need a try/catch block a mile long to extract all the information out of the different exception types.

This is particularly a problem in user (module) code where instead of catching all the different exception types in flintexceptions.h, we (well, I...) lazily catch FLINTException, and then all those boost::error_info details get lost. I think it would tidy things up a lot if we either got rid of all the specialized exceptions and stuck with a single one like SimulationError, or required FLINTException subclasses to override what() or some other method that rolls up all the details into some user or developer-friendly text.

There's a fair amount of flexibility for fixing this issue, steps would basically be:

  • have a look through the code (i.e. spatialtiledlocaldomaincontroller) and notice how ugly the exception stuff currently is
  • propose a strategy for improving the situation: could be one of my suggestions above or something completely different, but probably best to let people have a look before investing effort into coding
  • implement whatever solution you come up with

@SlipperyGnome
Copy link
Member

Hey @mfellows @aornugent Can I be assigned this issue, it has been inactive for a while and I think I can implement my solution here.

@aornugent
Copy link
Contributor

That's fine - could you please also coordinate with @AlkaDas991 so that you can both learn together? I'll connect you on Slack.

@SlipperyGnome
Copy link
Member

Hi @mfellows, I discussed this with @aornugent and it would be great if you look into this as well.
So I have thought of shifting all the boost::get_error_info and _spaciallocationinfo parts of SimulationError and LocalDomainError to flintexception.h that way it'll tidy up the spacialtiledlocaldomaincontroller.cpp file as these try/catch blocks have been used everywhere in that file. This way the exceptions.h file and flintexception.h file will have a similar layout and it'll be easier for a developer to go through this file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature request or improvement or optimization. Good First Issue These issues require minimal context and are well-suited for new contributors.
Projects
None yet
Development

No branches or pull requests

7 participants