Open Jalle19 opened 9 years ago
Full ACK to all points. :+1:, esp. for the incremental approach.
Not sure about the "separate subdirectory" for types in namespace tvheadend. What else will we have beside "tvheadend"?
BTW, for the series recording stuff I already started not to squash everything into Tvheadend.[cpp|h]. ;-) => https://github.com/ksooo/pvr.hts/commits/series-recording-support
One more todo: Get rid of CStdString, replace with std::string
One more: Replace c casts with c++ casts
Not sure about the "separate subdirectory" for types in namespace tvheadend. What else will we have beside "tvheadend"?
This is just a habit of mine, I like having the directory structure reflect the code structure as well (having a folder per namespace is similar to having a file per class).
Just a heads up, I've started working on some of the stuff here
I've almost finished factoring out some non-connection related stuff from CHTSPConnection
, I'll put up a new PR to replace the two existing ones.
I'm opening this here instead of creating a PR so we can come to an agreement on what should be done and what not. There are a couple of issues I'd like to address:
separate all classes into separate headers. Currently we have a bunch of.cpp
files that all share the common (massive)Tvheadend.h
header.CTvheadend
to retrieve e.g. the globalCHTSPConnection
object, this way we wouldn't need all the "passthrough" methods inCTvheadend
(see the demuxer related methods for instance). => ksooo: not sure about this one. I think pushing stuff (context) down is to prefer over sharing a global tvh object from which everything can be pulled.tvheadend
namespace as a separate subdirectory, and unify the naming scheme by dropping theC
andCHTSP
prefixes from our classes. => ksooo: only class CTvheadend left in global namespace after #334Use classes instead of structs for our internal objects (timers, channels, events) and add proper comparison operators to them in order to get rid of theUPDATE
macro.tvheadend
namespace, this means that e.g.PVR_CHANNEL
structs are not passed down fromclient.cpp
. Instead, the corresponding channel is looked up andCTvheadend
operates onSChannel
(or a classChannel
, see earlier point) instead.ReplaceCStdString
@ksooo do you agree with these points? If we decide to do this we should probably do it incrementally, starting from the top (separating classes into separate files) to keep the changes managable and reviewable.