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

Add fromArray method to Buffer #368

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

xlaywan
Copy link

@xlaywan xlaywan commented Apr 26, 2022

Useful to use in postupgrade in combination with buffer.toArray() in preupgrade

Useful to use in pre/post upgrade in combination with `b.toArray()`
Comment on lines +149 to +162
/// Construct a buffer from an array.
public func fromArray<T>(arr: [T]): Buffer<T> {
let count = arr.size();
let buffer = Buffer<T>(count);

var i = 0;
label l loop {
if (i >= count) break l;
buffer.add(arr[i]);
i += 1;
};

buffer
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Why not simply

Suggested change
/// Construct a buffer from an array.
public func fromArray<T>(arr: [T]): Buffer<T> {
let count = arr.size();
let buffer = Buffer<T>(count);
var i = 0;
label l loop {
if (i >= count) break l;
buffer.add(arr[i]);
i += 1;
};
buffer
};
/// Construct a buffer from an array.
public func fromArray<T>(arr: [T]): Buffer<T> {
return { count = arr.size(); elems = Array.thaw(arr) }
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks definitely better!
But can it create an instance of the Buffer class? Motoko Playground shows error:
image

It was intended to be a static method like let buf = Buffer.fromArray<Nat>([1, 2, 3]);
Probably it will work as an instance method like:

let buf = Buffer.Buffer<Nat>(0);
buf.fromArray([1, 2, 3]);

image

Not sure we can leave the name fromArray in this case, because in other files in base lib from* methods are static.

Link to the playground https://m7sm4-2iaaa-aaaab-qabra-cai.raw.ic0.app/?tag=2337373062

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right, Motoko classes have only one constructor usually…

Maybe first add buffer.addArray (which can be implemented efficiently using Array.thraw, especially if the buffer is empty before), and then the fromArray function.

@crusso
Copy link
Contributor

crusso commented Jun 14, 2022

This looks closely related to #387. Perhaps it can be closed now? @matthewhammer ?

@matthewhammer
Copy link
Contributor

The discussion here seems stalled on the most efficient way to do the operation.

In a closely related PR, we are proposing the same method functionality and name, but with the naive implementation. Maybe we can start there and refine?

@ggreif ggreif force-pushed the master branch 2 times, most recently from d52aecd to 08507fc Compare October 21, 2022 12:22
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.

4 participants