add set function to arraylist #1075
Conversation
so you can set a value without growing the underlying buffer (like insert function), maybe better than working directly with self.items
|
Thanks for opening a PR :)! A few things however, Since you don't actually increase the Also |
|
i used "l" rather than "self" just because all the other function currently use "l". latest commit compares bounds with l.len rather than items.len. i'm fine to close this PR and just set explicitly through self.items[], though then the caller would have to worry about iterators and self.len, no? maybe it's a non-issue |
|
Oh you are right, those should probably be changed since some of them are self (only a few) and some are Anyways, I think this is useful enough since it does bound check it against the current 'array' but we'll see what others think :). |
|
Please add a test for your new api. Adding tests would help catch bugs like the one @BraedonWooding pointed out. Tests are also a nice way to document the usage of your code. In general tests are good to have and will make your code easier to review and more attractive to merge. |
|
thank you for the recommendation! found a bug in the process of adding tests. latest commit has tests and the bug fix |
|
@clownpriest Note: sometimes travis errors out like this, often it'll work when re-run (note: how under the details for travis-ci/pr it says it '!' error'd, it didn't say it failed), you can force a re-run by closing PR and reopening it or someone with admin rights I think can re-run it :). |
|
|
||
| list.set(9, 99) catch |err| { | ||
| assert(err == error.OutOfBounds); | ||
| }; |
There was a problem hiding this comment.
This error checking is not correct. It should be more like:
if (list.set(9, 99)) {
assert(false);
} else |err| {
assert(err == error.OutOfBound);
}
This is such a common pattern that there should probably be a convenience function in std.debug for it. 🤔
There was a problem hiding this comment.
I would generally recommend @panic("assertion failed") over assert(false).
|
merged in e0092ee |
so you can set a value without growing the underlying buffer (like
insert function), maybe better than working directly with self.items