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

Allow decoding from a file handle #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

stecman
Copy link

@stecman stecman commented Dec 12, 2014

This changes the decoder to read from an interface (Rych\Bencode\DataSource) instead of directly from a string. String and FileHandle DataSource implementations are built in. A file can be decoded given a file handle:

$file = fopen('path/to/file.bin', 'r');
$decoded = Bencode::decode(
    new FileHandle($file)
);

When I wrote this back in April, it broke backwards compatibility by changing the method signature of Bencode::decode to require an instance of DataSource, but I've updated it for this PR to implicitly convert strings to Rych\Bencode\DataSource\String instead.

stecman and others added 3 commits December 12, 2014 12:56
Using a string as input to the decoder meant it was required to read the whole length of data to
decode into memory, which meant decoding was a potentially very heavy on memory. This change
abstracts out the input to the decoder, and adds a DataSource called String to replace it. This
change breaks backwards compatibility.
This data source provides a way to decode a bencoded a file with only a handle to that file. This
avoids the entire file into memory.
The addition of the DataSource interface originally changed the signature
of `Bencode::decode` to use a type hint. This broke backwards compatibility
(the function previously took a string) and was clunky to use. This change
implicitly converts non-DataSource inputs to `DataSource\String`.
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

Successfully merging this pull request may close these issues.

1 participant