Refactor lib/torrent.js

[ericwooley]

I have spent a few hours tonight digging through torrent.js attempting to learn enough to change the hashing algorithm, for #430 (comment)

Torrent.js is 1264 lines long, and many of the methods are named after what calls them instead of what they do. I would recommend refactoring and renaming the methods to describe what they do instead of what calls them. Normally this wouldn't be too much of an issue, but given the vastness of the file, it becomes very hard to follow the algorithm when the method names describe what calls them instead of what they do.

I would recommend splitting the file up, but you guys have already done a great job of abstracting functions. A large file may be something that needs to be lived with.

I am not sure what the ramifications would be in renaming the methods, in terms of breaking changes. Many of them are named with an _ implying they are private. But maybe they are used elsewhere anyway?

Maybe defining them like

Torrent.prototype._onParsedTorrent = 
Torrent.prototype._initializeParsedTorrent = function(parsedTorrent) {
...
}

Would be an acceptable means of renaming the methods, without introducing breaking changes?

[feross]

@ericwooley

I agree that torrent.js is gnarly. It's where all the features in dozens of WebTorrent dependencies come together to form a working torrent client! It's actually pretty nice for all the functionality that it's doing, but I agree that it can be much better.

Improving method naming would help. We don't need to worry about renaming private methods that start with _. Those are undocumented and not called by users.

A larger, but more impactful task would be to pull out the torrent downloading "strategies" into their own npm packages. There are currently two strategies ("sequential" and "rarest-first") and that code is some of the messiest. Another advantage, we can send a PR to torrent-stream and get them to share code with us like we've already done with torrent-discovery, torrent-piece, parse-torrent, and fs-chunk-store. Shared code will be better maintained and work better :)

[ericwooley]

@feross I have been working on documenting all the methods of torrent.js so that I can more easily follow whats happening. Perhaps after that I'll attempt to refactor the strategies.

I need to keep looking through to figure out how everything works before I would feel comfortable pulling everything out. I'll do a PR on the docs, first though.

[feross]

@ericwooley Great, happy to look at that PR and merge it if it makes things easier to understand.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.