This repository has been archived by the owner. It is now read-only.

Creating buffer from buffer should work like creating buffer from array #609

Closed
isaacs opened this Issue Feb 1, 2011 · 9 comments

Comments

Projects
None yet
2 participants

isaacs commented Feb 1, 2011

new Buffer(new Buffer("hello")).toString() !== "hello"

I think that's not so great.

Fix: https://gist.github.com/805571

assaf commented Feb 1, 2011

+1

Accidentally constructed one buffer from another, and spent too much time trying to figure out why tests are failing.

@ghost

ghost commented Feb 1, 2011

Why do you consider https://github.com/ry/node/issues/issue/532 bad idea? It would probably make things clearer, and I believe eventual bugs will be more apparent in the code, too

assaf commented Feb 1, 2011

@Herby: my intent was to convert a String to a Buffer, not noticing the value I was passing around was a Buffer not a String. #532 would not have helped.

isaacs commented Feb 1, 2011

re #532 on that other issue.

It would probably be a good idea to throw if the object argument is not an array or a buffer, or support any Array-ish. With the patch in this pull req, new Buffer({length:5,0:10,1:1,2:2,3:3,4:4}) still returns 5 uninitialized bytes.

@ghost

ghost commented Feb 1, 2011

Oh, definitely it would. It would give you fail-fast error that you're using invalid argument (buffer should be passed to fromBuffer, not to fromString if 532 was implemented). And from the dev point of view, it is much clearer to have one method for creating buffer from a buffer and second one for creating it from a string -- each having their own finely defined responsibility and cases to cover. Overloading makes things unclear.

assaf commented Feb 1, 2011

@Herby the issue I ran into was lack of argument checking. Adding argument checking would have solved the problem, whether it's argument checking on fromX method or argument checking on overloaded constructor.

Once added, it just boils to a matter of style.

@ghost

ghost commented Feb 1, 2011

@assaf: Yes. Still thinking fromBuffer is cleaner, but as isaacs pointed out, general converter constructor is more in line with existing Javascript.
@isaacs: Ignore the subbuffer, it was stupid idea, I edited it out, as you see. Supporting any arrayish is good thing, if it should be in this style.

isaacs commented Feb 1, 2011

Updated https://gist.github.com/805571 to reflect this discussion. Thanks, guys.

isaacs commented Feb 7, 2011

Closed by 2e6a263 Support array-ish args to Buffer ctor

Any array-ish thing (whether a Buffer, an Array, or just an object with
a numeric "length") is interpreted as a list of bytes.

coolaj86 pushed a commit that referenced this issue Apr 15, 2011

Closes GH-609 Support array-ish args to Buffer ctor
Any array-ish thing (whether a Buffer, an Array, or just an object with
a numeric "length") is interpreted as a list of bytes.

This issue was closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.