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

Refactoring roadmap #27

Open
Jalle19 opened this issue May 5, 2015 · 7 comments
Open

Refactoring roadmap #27

Jalle19 opened this issue May 5, 2015 · 7 comments
Labels

Comments

@Jalle19
Copy link
Contributor

Jalle19 commented May 5, 2015

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.
  • add some getters to CTvheadend to retrieve e.g. the global CHTSPConnection object, this way we wouldn't need all the "passthrough" methods in CTvheadend (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.
  • move all classes under a tvheadend namespace as a separate subdirectory, and unify the naming scheme by dropping the C and CHTSP prefixes from our classes. => ksooo: only class CTvheadend left in global namespace after Source cleanup #334
  • Use classes instead of structs for our internal objects (timers, channels, events) and add proper comparison operators to them in order to get rid of the UPDATE macro.
  • attempt to separate the Kodi API as clearly as possible from the tvheadend namespace, this means that e.g. PVR_CHANNEL structs are not passed down from client.cpp. Instead, the corresponding channel is looked up and CTvheadend operates on SChannel (or a class Channel, see earlier point) instead.
  • Replace CStdString
  • replace C style casts with C++ style casts.

@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.

@Jalle19 Jalle19 changed the title [RFC] Refactoring roadmap Refactoring roadmap May 5, 2015
@Jalle19 Jalle19 added the RFC label May 5, 2015
@ksooo
Copy link
Member

ksooo commented May 5, 2015

Full ACK to all points. 👍, esp. for the incremental approach.

Not sure about the "separate subdirectory" for types in namespace tvheadend. What else will we have beside "tvheadend"?

@ksooo
Copy link
Member

ksooo commented May 5, 2015

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

@ksooo
Copy link
Member

ksooo commented May 5, 2015

One more todo: Get rid of CStdString, replace with std::string

@ksooo
Copy link
Member

ksooo commented May 5, 2015

One more: Replace c casts with c++ casts

@Jalle19
Copy link
Contributor Author

Jalle19 commented May 11, 2015

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).

@Jalle19
Copy link
Contributor Author

Jalle19 commented Jun 13, 2015

Just a heads up, I've started working on some of the stuff here

@Jalle19
Copy link
Contributor Author

Jalle19 commented Jan 30, 2016

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.

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

No branches or pull requests

2 participants