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

default and optional arguments for get(), combined error code #15

Open
3 tasks
kostyfisik opened this issue Oct 24, 2019 · 8 comments
Open
3 tasks

default and optional arguments for get(), combined error code #15

kostyfisik opened this issue Oct 24, 2019 · 8 comments
Assignees

Comments

@kostyfisik
Copy link
Contributor

kostyfisik commented Oct 24, 2019

Summary:

  • default key
  • optional key
  • simplified error handling.

Detailed description:
In my code FiNeR usage looks to have a lot of boilerplate code and still no good error handling. Probably FiNeR can be improved some way to make the life of end users a bit easier. Now I have:

      call fini%get(section_name='cylinder', option_name='rl_min',
     &  val=double, error=error)
      rl_min = -1_dp
      if (error==0) rl_min = double

      call fini%get(section_name='cylinder', option_name='rl_max',
     &  val=double, error=error)
      rl_max = -1_dp
      if (error==0) rl_max = double

      call fini%get(section_name='cylinder', option_name='rl_steps',
     &  val=num, error=error)
      ndefp = 0
      if (error==0) ndefp = num

      call fini%get(section_name='cylinder', option_name='radius',
     &  val=double, error=error)
      rsnm = -1_dp
      if (error==0) rsnm = double

      call fini%get(section_name='cylinder', option_name='alpha',
     &  val=double, error=error)
      alpha = 0_dp
      if (error==0) alpha = double

So, you can see a simple pattern: for every option in my INI file I use some default value, which I set before setting the one I get from FiNeR just to avoid the case of uninitialized variables. So it would be nice, if get() function had a default parameter, so I could reference to real argument only once and be sure that in case of any parsing error the variable would still be initialized with default value.

      call fini%get(section_name='cylinder', option_name='rl_min',
     &  val=rl_min, default = -1_dp, error=error)

      call fini%get(section_name='cylinder', option_name='rl_max',
     &  val=rl_max, default = -1_dp, error=error)

      call fini%get(section_name='cylinder', option_name='rl_steps',
     &  val=ndefp, default = 0, error=error)
     
      call fini%get(section_name='cylinder', option_name='radius',
     &  val=rsnm, default = -1_dp, error=error)

      call fini%get(section_name='cylinder', option_name='alpha',
     &  val=alpha, default = 0_dp, error=error)

So, what about error handling? My code has no one at the moment to keep it maintainable. It would be nice if FiNeR can accumulate different types of error and just provide the report at the end. So at the end of calls to fini%get(...) I would like to do something like

      if (fini%get_number_of_errors(type='parse') > 0) then
        fini%print_error_log(type='parse', unit=6) 
        stop 'Parse error'
      end if

Finally, it would be nice to separate required and optional options in INI file. E.g. if I call

      call fini%get(section_name='cylinder', option_name='rl_min',
     &  val=rl_min, default = -1_dp, optional=.true., error=error)

it should not count an error if there is no rl_min option in cylinder section. However, it should count an error, if there was such an option and FiNeR could not parse it to requested variable type.

@kostyfisik
Copy link
Contributor Author

@szaghi What do you think? Is it a valid feature request?

@szaghi
Copy link
Owner

szaghi commented Feb 3, 2020

@kostyfisik During this week I try to study this feature request, I am sorry for my delay.

@kostyfisik
Copy link
Contributor Author

@szaghi Thank you a lot! If you need more context you can check https://github.com/wave-scattering/amos-try/blob/master/cylinder2mod/model_parameters.f90#L57-L200

@szaghi
Copy link
Owner

szaghi commented Feb 5, 2020

@kostyfisik

I have read more carefully this feature request. It is not simple, thus I have planned to implement it in gradual way: I first start to add a default value to the get method and then I will try to implement the other modification, stay tuned.

@szaghi szaghi self-assigned this Feb 5, 2020
@kostyfisik
Copy link
Contributor Author

@szaghi Note, that it was inspired by FLAP, it can be a good idea to keep it consistent for API names.

Note, that this feature request is about to reduce the boilerplate code needed to have some error proof behaiviour. While default, and optional keys seems to be natural, for error handling there can be some better API. I will add a check list to the first comment to make it easier to track gradual progress.

@kostyfisik
Copy link
Contributor Author

@szaghi Any update on this?

@szaghi
Copy link
Owner

szaghi commented Nov 26, 2020

Hi @kostyfisik

I am very sorry, but I have not yet found the time for this issue, please be patient.

I think that the next two weeks are very busy, but I will hope to do it soon in the mid-Decemeber.

@kostyfisik
Copy link
Contributor Author

@szaghi Thank you a lot for your effort! I was able to get back to my project which uses FiNeR almost after 1 year of it being stalled due to various external reasons. This way mid-December looks to be a perfect time!

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

No branches or pull requests

2 participants