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

The code should be broken out in smaller pieces #193

Closed
rbrito opened this issue Oct 15, 2011 · 2 comments
Closed

The code should be broken out in smaller pieces #193

rbrito opened this issue Oct 15, 2011 · 2 comments

Comments

@rbrito
Copy link
Contributor

rbrito commented Oct 15, 2011

Hi.

I was reading the code to implement the recent xvideos IE and it became obvious to me that the code is of a very heterogeneous quality: the techniques used for extraction is, sometimes, using pre-existing python modules, sometimes a module like yours 'trivial' xml parser, sometimes with hand-rolled regular expressions etc.

The coding style is also heterogeneous.

There are also some duplications of code: I can't see any reason why the _simplify_title (just to give one example) is not in the parent InfoExtractor class: most implementations of InfoExtractor would use that, anyway.

Similarly to the report_webpage and report_extraction. Factoring those out can lead to smaller code, fewer duplications, benefits when the basic implementation is improved etc.

Another thing, possibly for a longer term: perhaps the code could be, somehow, split with each information extractor in one file and, to guarantee convenience for updates/downloads, the "real" youtube-dl could be made with a simple concatenation of the files.

This would allow some modularization when we write the code, fewer paging up/down to see other parts of the code, etc. and, most important of all, being easier to be able to hold more of the program inside our heads.

Of course, it doesn't need to be that way, and that's just brainstorming, but the general principle of a "compilation step" could be taken.

Regards.

@phihag
Copy link
Contributor

phihag commented Oct 15, 2011

My trivial HTML parser is still in its infancy stages. Eventually, it will replace all the regular expressions and lxml calls. Since lxml is not in the stdlib, we can't use it, and I haven't found an other good HTML parser.

My trivial JSON parser is just needed for Python 2.5. If we ever stop supporting that, we can switch to the standard JSON. JSON IEs are a good idea since they're using an API that's unlikely to break in the future, and extremely simple.

The report_ functions are not really necessary, we should indeed have a report(message) in the InfoExtractor.

Splitting the IEs into different files is a good idea, and I'll leave this bug open for that change.

_simplify_title is indeed only a placeholder. We can simply remove all references to it once we fix #164 andintroduce a new communication interface between IE and downloader, where the IE just gives one (slightly more complex) info_dict to the rest of the world, like this:

{"playlist_name": "Software languages", "videos": [
  {id:"python", "title": "Python", "versions": [
    {"url":"http://cdn.com/python/hq", "format": "hq", "ext":"ogg"},
    {"url":"http://cdn.com/python/lq", "format": "low-quality", "ext":"flv"}
  ]},
  {id:"python", "title": "C", "versions": [
    {"url":"http://cdn.com/c/lq", "format": "low-quality", "ext":"flv"}
  ]}
]}

@ocisly
Copy link

ocisly commented May 23, 2014

@phihag although the code base still leaves a lot to be desired, I think this issue can safely be closed now as most of the improvements seem to have happened ( info_dicts are now in place, for instance).

@phihag phihag closed this as completed May 23, 2014
blackjack4494 referenced this issue in blackjack4494/youtube-dlc Sep 12, 2020
joedborg referenced this issue in joedborg/youtube-dl Nov 17, 2020
[pull] master from ytdl-org:master
tsukumijima pushed a commit to tsukumijima/youtube-dl that referenced this issue Dec 2, 2020
[MTV/Nick] universal mgid extractor + fix nick.de feed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants