So as most of you know, I’ve been working to get Windows stuff up and running. An important part of this involves creating a robust Host layer for Windows. Anybody who’s looked at Host.cpp knows it’s a mess of #ifdefs and platform-specific logic, and with Windows this is going to get much worse as I fill out more and more of Host.cpp. Many things on Windows have no analogue on other platforms, some things have an analogue but not a very good one, some things have a partial analogue, etc. After a few attempts to implement some things in Host.cpp I decided it wasn’t going to work, or at the very least it wasn’t going to be maintainable. And I don’t want my work on Windows to affect other peoples’ ability to get the things they need to get done, done.
I submitted a patch last week to move all host FileSystem related code to a separate FileSystem class, and in a few minutes I’m going to post a patch that starts a more significant refactor of Host. The idea here is to (maybe) get rid of the Host class entirely, and instead move functionality in classes that are more specific and descriptive of their purpose. FileSystem was the start of this, and the upcoming patch will implement HostInfo - a static class used only for answering queries about the host operating system. Future patches will split out code for manipulating processes and threads.
There are, at least in my view, a number of benefits to doing things this way:
- Maintaining code in the Host layer becomes easier. Very few (if any) #ifdefs will remain at all.
- Better logical separation of responsibilities. There was not much rhyme or reason before as to what kind of functionality would be behind Host::, and what would be in some other class such as Pipe, or Socket.
- Less risk of breaking other platforms when you make platform-specific changes for your own Host. Adding a new function for MacOSX? You’ll most likely be editing source/Host/macosx/HostInfoMacOSX.mm. Zero chance of breaking another platform.
There is one aspect which may or may not be contentious, and that is that I’ve opted to omit default implementations of functions. As a concrete example, previously we had the function Host::GetDistribution. This is now implemented in HostInfoLinux::GetDistribution(). It is not implemented anywhere else. If you write HostInfo::GetDistribution() while not on Linux, you will get a compile error. This forces you to put the platform specific checks at the call-site. I actually think this is a benefit, because if a function is truly platform specific then you should be calling it from a codepath that is aware of the platform.
I’m open to hearing other viewpoints about this though.
The patch will be up shortly. It’s large, so I hope this email serves to give a summary of its intent.