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

Splitting the ews.hpp into multiple smaller files #95

Open
SebastianBecker2 opened this issue Jan 12, 2018 · 6 comments
Open

Splitting the ews.hpp into multiple smaller files #95

SebastianBecker2 opened this issue Jan 12, 2018 · 6 comments
Milestone

Comments

@SebastianBecker2
Copy link
Collaborator

SebastianBecker2 commented Jan 12, 2018

My VS2017 is begging me to have smaller portions of code per file.
It takes up to 30 seconds to re-parse the code after changes (to update highlighting and provide other features like the renaming).

I never worked on a header only library so I'm not sure about the possible downsides.
But shouldn't it be easy to add more hpp files which are included in the ews.hpp just like the ews_fwd.hpp?

@raldus
Copy link
Contributor

raldus commented Jan 12, 2018

A dream comes true. ;)

@bkircher
Copy link
Collaborator

bkircher commented Jan 12, 2018

My VS2017 is begging me to have smaller portions of code per file.
It takes up to 30 seconds to re-parse the code after changes (to update highlighting and provide other features like the renaming).

I see. Well, I don't mind changes.

I'm never worked on a header only library so I'm not sure about the possible downsides.

  1. Downside could be too many files. Especially for developers of the library. Always searching for the definition of a class. Or introducing circular dependencies.
  2. Another downside is: Installation can be more complicated for the end-user. Now it is: drop these couple of files or this one header file in your project and you're done. With too many files you want to use CMake and a make install step. Which is not too bad, but you loose some flexibility.

But I think when a split-up is done right these downsides can be neglected.

@bkircher
Copy link
Collaborator

There are multiple possibilities

  1. We could split up by each and every class like ews_service.hpp, ews_task.hpp, ews_caleandar_item.hpp
  2. We could split-up by only a few files, like ews_internal.hpp, ews.hpp and maybe a couple of files more if needed, like ews_autodiscover.hpp or whatever

Did I miss something? Any suggestions? Preferences?

@SebastianBecker2
Copy link
Collaborator Author

SebastianBecker2 commented Jan 15, 2018

I took a shot at it but it really isn't as easy as i hoped it would be.

I successfully moved all enums and most enum related functions to ews_enum.hpp. I had to move the definition of the exception class to another file too, since it's constructor is used in some of the related functions.
This reduced the length of the ews.hpp by about 6500 lines.
You can check the result here:
https://github.com/SebastianBecker2/ews-cpp/tree/split_files

From what I've seen, splitting into categories is prone to cause circular dependencies.
But moving every class into their respective files would cause a huge amount of files.

What do you think about ews_enum.hpp? Is that something worth creating a PR for? Or just leave it as is? Or look for another way?
I'm not super happy with it myself but I figured, getting the response_code enum + helper functions (4k lines) out of the ews.hpp is a good start.

@obatysh
Copy link

obatysh commented Mar 3, 2022

We would like to use ews-cpp in our project and splitting seems a nice idea. Would you accept a PR if we try to do it ourselves?

@idolum idolum added this to the 0.11 milestone Mar 4, 2022
@idolum
Copy link
Member

idolum commented Mar 4, 2022

@obatysh Sounds great! As you can see, the others considered various ideas for splitting the ews.hpp. Just give it a try. If you have a PR, I would be glad to review it. Moving the enums to a separate file, as Sebastian tried once, seems to me as a good first approach. But if you have other ideas, just let us know, so that we can provide feedback.

@idolum idolum modified the milestones: 0.11, 0.12 Nov 28, 2022
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

5 participants