Labels
Zend Framework: Zend_BitTorrent Component Proposal
| Proposed Component Name | Zend_BitTorrent |
|---|---|
| Developer Notes | http://framework.zend.com/wiki/display/ZFDEV/Zend_BitTorrent |
| Proposers | Christer Edvartsen Darby Felton, Zend liaison |
| Revision | 0.1 - 11 October 2007: Initial proposal made 0.2 - 5 November 2007: Added use cases and some info about Zend_BitTorrent_Torrent 0.3 - 20 December 2007: Changed component name and updated the proposal a bit (wiki revision: 18) |
Table of Contents
1. Overview
Zend_BitTorrent is a component that can be used to encode and decode data in the BitTorrent format. Zend_BitTorrent will contain convenient methods for accessing methods in the Zend_BitTorrent_Encoder and Zend_BitTorrent_Decoder classes.
The most common usage of a component such as this one would be to decode .torrent files. The information found in torrent files are the name of the file(s) included in the torrent along with file sizes and more info used by BitTorrent clients when downloading torrents.
The Zend_BitTorrent_Torrent class contain ways to generate .torrent files. It can generate files based on other torrent files, or a path to a file or a directory.
2. References
3. Component Requirements, Constraints, and Acceptance Criteria
- This component will encode any encodable PHP variable (int, string, array).
- This component will decode a BitTorrent encoded string back to the original PHP counterpart.
- This component will decode entire BitTorrent files (typically .torrent)
- This component will generate .torrent files based on a path to a file or a directory
- This component will mimic the official btmakemetafile script when generating .torrent files.
4. Dependencies on Other Framework Components
- Zend_Exception
5. Theory of Operation
When decoding it will first figure out what kind of variable the BitTorrent encoded string represents and then iterate through the string decoding all nested parts (if any). When encoding it will follow the simple rules of the BitTorrent specification.
6. Milestones / Tasks
- Milestone 1: [DONE] Initial design of the classes
- Milestone 2: [DONE] Working prototype
- Milestone 3: [DONE] Unit tests exist and work
- Milestone 4: Prototype and tests are checked into SVN
- Milestone 5: Initial documentation exists.
7. Class Index
- Zend_BitTorrent
- Zend_BitTorrent_Encoder
- Zend_BitTorrent_Encoder_Exception
- Zend_BitTorrent_Decoder
- Zend_BitTorrent_Decoder_Exception
- Zend_BitTorrent_Torrent
- Zend_BitTorrent_Torrent_Exception
8. Use Cases
| UC-01 |
|---|
Encode a string using the Zend_BitTorrent class:
| UC-02 |
|---|
Encode a string using the Zend_BitTorrent_Encoder class:
| UC-03 |
|---|
Decode a torrent file using the Zend_BitTorrent class:
| UC-04 |
|---|
Decode a torrent file using the Zend_BitTorrent_Decoder class:
| UC-05 |
|---|
Use the factory to generate a new .torrent file
| UC-06 |
|---|
Use the factory to generate a .torrent file from a path
9. Class Skeletons
I'm not that familiarly with Bittorrent development, so I really want to know what is the deal with this class.
This is not a critic! ![]()
Thank you
The most common usage would be to decode torrent files to get information about the files that are included in the torrent. All sites that offer .torrent files most likely use some sort of library to fetch this info from the torrent files, and I thought it would be nice to have this in the framework.
As I wrote in the first comment here I have a bittorrent tracker written in PHP that can be added later that would make the component more complete.
Apart from being able to read from files, I also would like to see either a separate function (akin to simplexml_load_string and simplexml_load_file) to load the torrent file data from a string. I have seen many implementations that always assume one wants a file and never give this option.
What do you mean? That one wants to give the decode method the content of the torrent file instead of the path to the file?
If that is what you mean you can just do a Zend_Bittorrent::decode(file_get_contents('/path/to/file)); since torrent files are just an encoded associative array.
I just updated the proposal with information about the Zend_Bittorrent_Torrent class that can be used to generate .torrent files.
I have added the code to the laboratory at http://framework.zend.com/svn/laboratory/library/Zend/. Feel free to play around with it. ![]()
Hi Christer,
Looks interesting. Note that it would be Zend_BitTorrent, with a capital T.
You might also try to split up the Zend_BitTorrent_Torrent::buildFromPath() method into a couple other methods, because it's pretty long at the moment.
-Matt
This looks like a promising component, and I'd very much like to see it in Zend Framework. Following are the results from my initial review for consideration:
The prolific use of static methods irks me a bit, as we might be better served by having instance methods, but perhaps it can be shown that static methods will not be a problem.
- How might the component support caching?
- How to support instances having different behaviors (e.g., caching, logging)?
- What about extending the classes for custom behavior? (e.g., Zend_BitTorrent_Encoder::encodeList() uses self)
I would recommend different placement of the require_once statements for lazy-loading classes, especially those for exceptions.
There is no need to use Zend_Loader::loadClass() within this component. Just use require_once.
What about having exception classes inherit from Zend_BitTorrent_Exception?
Don't forget to use the one true brace convention, with method and class opening braces on the next line.
Per-class suggestions:
Zend_BitTorrent
- decode()
- Recommend removal of "=== true"
- Enumerate the types that encode()/decode() accept/return (e.g., integer|string|array)
Zend_BitTorrent_Encoder
- encode()
- Recommend use of switch(gettype($var)) instead of if...else if...
- encodeInteger(), encodeString(), encodeList(), encodeDictionary()
- Use gettype() once, no need for is...() functions
Zend_BitTorrent_Decoder
- decodeFile()
- Recommend removal of "=== true"
- decode()
- Recommend different regular expression preg_match('/^\d+:/', $string)
- decodeInteger(), decodeString()
- Recommend different exception messages for different exception cases
- decodeList(), decodeDictionary()
- Is there really a need to re-encode some parts? Why not just strlen() the encoded part before decoding it?
Zend_BitTorrent_Torrent
- What benefits does the static factory method have over the "new" operator?
- What about extending the class?
- On what basis have the values of the class constants been selected?
- Protected variable names should begin with underscores (e.g., $_protectedMember)
- Does isReadyToBeBuilt() return true even if the object has already been built?
Thanks for the comment. I'll try to answer everything (I'm a bit short on time, traveling to Spain tomorrow for a week of climbing).
The component might be better off without the static methods and having instances of the classes instead so users can more easily extend the classes.
I'm not sure how other components in ZF deal with caching. I suppose there is not too much hassle to have caching functionality in the component and just let the user enable/disable it. What do you think would be the best option? To implement it in the component or having users extend the classes?
When having instances I can also drop the is*() function calls in the methods that require variables of specific types. If I skip that function call and keep the methods static I would have to make them protected and force the user to only use the generic encode() method. That way I know the input to the protected methods are correct. That would probably work just fine. It does not produce that much overhead. But if we have an instance we could just check the input in the constructor and use that information to skip the is*() calls and have all methods public. Personally I tend to like to be able to call encodeString() if I know my data is a string and not a generic encode() method.
When you say "different placement of the require_once statements for lazy-loading classes" I'm not sure I understand correctly. Would you want them in the code where the exceptions are thrown instead of calls to Zend_Loader? If so, I agree.
The main reason for reencoding data in the decoder is because of how strings are handled. Lets say we want to decode "3:foobar". The decoder will give you (string) "foo" and just discard the rest of the input since the prefix of the encoded string is 3 (the length of the original string). Because of that we can't do a strlen($str) since that would return 6, and not 3 which is the correct length. This might be one good reason for implementing caching in the component instead of having users do it themselves.
What benefits does the static factory method have over the "new" operator?
Not much really. I know some users think it's easier to use factory methods because if often results in fewer lines of code and less possibilities for errors. Personally I think it's a bit cleaner to have a factory method but I'm not saying it's better or worse than having a public constructor.
What about extending the class?
Seems like ditching the factory method and sticking with __construct might be a better idea, yes.
On what basis have the values of the class constants been selected?
On no basis at all actually. There were just the first that came to my mind. I just read the part about class constants in the Coding Standard document and saw how it should be done. I could just change them to "TORRENT_CREATE_FROM_FILE" and so on. Would that be better?
Does isReadyToBeBuilt() return true even if the object has already been built?
Seems like it. I could just see if it was already built and if so, return false. That might be more logical since there is no reason for building the torrent twice.
If I have skipped a questions it's just because I agree and have nothing more to add. ![]()
I will get to the changes when I get back from my vacation.
Don't hesitate to add more comments if something is unclear.
Thanks
With respect to caching, I think that we can find candidates for caching opportunities during incubation (development in trunk/incubator). We should probably not spend much time on premature optimization at this point. Just something to keep in mind. ![]()
I don't see any need to drop the type-specific encode...() methods such as encodeString() from the public interface. As you noted, it will likely be desirable to call these methods directly when working with known types.
Regarding the use of require_once, yes, I mean to suggest that these statements be located immediately prior to the use of such classes, as in "lazy loading". For example, in a method that throws an exception:
Regarding constants, we have both names and values. I recommend that names begin with the general and end with the specific (e.g., PATH_CONFIG, LOCATION_COUNTRY). I don't see much problem with the names of the constants you have selected. I was wondering more about the values of these constants, and how these have been chosen.
Have a great vacation, and I look forward to your return! ![]()
Christer, is it your intention to get this in to core for the 1.5 release? If so, let us know as soon as the proposal is ready for core team review and we'll turn it around as quickly as possible so you can get it in incubator and the hands of our users. The other possibility is to have it in incubator during 1.5, where it will likely benefit from the general attention we'll be getting around the release.
It's probably better to let it stay in incubator during 1.5 as you say. I currently don't have too much time to spend on this. Since we use ZF at the place I work I might be able to convince my boss to let me work on ZF related stuff during my work hours though, but anyways I think it might be better to not rush with the component.
I have updated the proposal a bit. It now includes all source code of the component. The component is in the laboratory as well.
There are some changes that are not in svn yet because of some issues with my laptop. One of the changes I can remember is that I have prefixed the protected properties in the Zend_BitTorrent_Torrent class with an underscore. There are some other minor changes but I can't access my laptop now to see what it is as of now.
Thanks for this, Christer! ![]()
I had another comment and a question I wanted to share:
We should consider moving this component to the Zend_Service_* "namespace", especially as this component could be used in affiliation with remote services (e.g., a torrent tracker). What do you think about contributing the tracker component you've written? The API and/or implementation can be improved during its lifetime in the laboratory and/or incubator, if you're concerned about that.
ZF Home Page
Code Browser
Wiki Dashboard
I have a tracker component lying around but right now it's not generic enough to fit in the Zend Framework. The tracker could maybe be added to this component at a later time as Zend_Bittorrent_Tracker...