This system is a bit inspired by an earlier patch I threw together in
a half hour to prevent the actual logic code from calling state-changing
functions by splitting the interface class (implements SlaveBase) from
the actual FTP client implementation (used by the interface class).
The advantage here is that the future internal code cannot call error
or finished directly (and cause state violations in SlaveBase - e.g.
double-error call) because there is no trivial way to access them.
Instead the internals return a new Result class if they need to return
anything. This is largely inspired by how Error handling works in Go.
The caller of an internal function needs to check the result or explicitly
disregard it. The caller may then decide to ignore an internal error result
(e.g. chmod after upload failed), create its own higher level result, or
forward it unchanged. On the interface level we then convert the result
to appropriate state changes of the slave.
This is assisted by Q_REQUIRED_RESULT so capable compilers will warn if
a Result is not explicitly ignored as that'd be an indication of missing
error handling in new code, again future proofing the slave.
All things put together this turned out really well I think.
This also adds a primitive test because it broke a million times while
porting. Starts a simple ruby ftpd (with some monkey patching applied).
- there are still a bunch of pending todos littered over the code
- currently the internal class relies on helper functions to forward
interface calls to the interface class, they can all be dropped in favor
of directly calling `q->foo`.
- needs more test scenarios I'd imagine
- there is a larger question if we still need the StatusCode enum, it
was exclusively used to differentiate when the error is remote vs local
so the correct file can be referenced (e.g. "/foo failed to read" vs
"ftp://h/foo failed to read") but since we can now create construct the
correct error strings even in very deep internal call chains this may
no longer be necessary
- I am wondering if maybe returning QScopedPointer<Result> would be nicer,
currently there are a lot of useless copies around
- Result::fail needs being made more explicit, all three arguments should
always need providing and ideally be useful (i.e. QString() the
errorString is fairly ugh
- unrelated: fix unrelated warnings, there's a lot of unsigned<->signed