add circular buffer exercism #17
Conversation
|
Thanks for updating. I'll have a look. |
| } | ||
|
|
||
| function updateReadPoint() { | ||
| readPoint = (readPoint + 1) % capacity |
|
Updated pull request with some of the requested changes. |
| }; | ||
|
|
||
| function isNull(data) { | ||
| return (data === null || data === undefined) |
There was a problem hiding this comment.
The parens are superfluous here and don't really add anything to the readability so we can probably nix them.
|
Ready for review |
|
|
||
| it('reading an empty buffer throws a BufferEmptyException', function () { | ||
| var buffer = circularBuffer(1); | ||
| expect(function () { |
There was a problem hiding this comment.
You still seem to have the superfluous anonymous function.
There was a problem hiding this comment.
That's expected. See Jasmine Matchers https://github.com/pivotal/jasmine/wiki/Matchers
|
Mainly typo fixes in these commits. |
|
Apologies @wilmoore Could you review again and suggest the appropriate changes. Thanks |
|
|
||
| it("reading an empty buffer throws a BufferEmptyException", function() { | ||
| var buffer = circularBuffer(1); | ||
| expect(function() { |
There was a problem hiding this comment.
The best way I know to explain it is with an example:
function call(fn) {
return fn();
}
function make_money() {
return "$5.00";
}
// superfluous
console.log("superfluous: %s", call(function () {
return make_money();
}));
// treating functions as first-class
console.log("first-class: %s", call(make_money));This will produce output of:
superfluous: $5.00
first-class: $5.00
Same result, but it is obvious which is less code, easier to read and reason about.
The fact of the matter is, whenever a function takes another function, it is doing so because it means to call that function. It doesn't matter which function you pass it, so long as when it is called, it does what you want. In the case of the code we are reviewing, buffer.read is referring to a function. If all you are doing in the anonymous function is calling just that function, you may as well just pass the function reference itself so it will get called directly rather than having to pass a wrapper function where you manually invoke it.
I hope this helps.
There was a problem hiding this comment.
That's expected. See Jasmine Matchers https://github.com/pivotal/jasmine/wiki/Matchers
That's mostly an issue with how they choose to document this case.
|
Ready for review |
|
LGTM. Let's squash the commits to get this merged. |
|
Squashed commits. Ready for review/merge |
|
LGTM. @kytrinyx, please merge (I don't seem to have a merge button). |
|
WAT? You're supposed to be on the javascript team. |
|
D'Oh. I have you on the Javascript team, but forgot to add the javascript repository to that team. It should give you a merge button now. |
|
Thanks @kytrinyx |
* circular-buffer: Add canonical data The circular-buffer exercise was first added in 2014 May at #9, with Ruby and Javascript as the initial implementing tracks: * exercism/DEPRECATED.javascript#17 * exercism/ruby#17 Implementing tracks: * https://github.com/exercism/xcsharp/blob/master/exercises/circular-buffer/CircularBufferTest.cs * https://github.com/exercism/xdlang/blob/master/exercises/circular-buffer/circular_buffer.d * https://github.com/exercism/xecmascript/blob/master/exercises/circular-buffer/circular-buffer.spec.js * https://github.com/exercism/xerlang/blob/master/exercises/circular-buffer/circular_buffer_tests.erl * https://github.com/exercism/xfsharp/blob/master/exercises/circular-buffer/CircularBufferTest.fs * https://github.com/exercism/xgo/blob/master/exercises/circular-buffer/circular_buffer_test.go * https://github.com/exercism/xjavascript/blob/master/exercises/circular-buffer/circular-buffer.spec.js * https://github.com/exercism/xlfe/blob/master/exercises/circular-buffer/test/circular-buffer-tests.lfe * https://github.com/exercism/xlua/blob/master/exercises/circular-buffer/circular-buffer_spec.lua * https://github.com/exercism/xpascal/blob/master/exercises/circular-buffer/uCircularBufferTests.pas * https://github.com/exercism/xpython/blob/master/exercises/circular-buffer/circular_buffer_test.py * https://github.com/exercism/xruby/blob/master/exercises/circular-buffer/circular_buffer_test.rb * https://github.com/exercism/xrust/blob/master/exercises/circular-buffer/tests/circular-buffer.rs All tracks pretty much implement the same tests, except: * Tracks typically have the "read item just written" and "each item can only be read once" tests combined. These tests split the two. * In dynamically-typed languages, buffers may be able to store heterogeneous types. This concept is not expressed in these tests. * In some statically-typed languages, buffers use generics, such that buffers may store any one type of the client's choosing. This concept is also not expressed in these tests. * The final test (ensuring that overwrite drops the right items) was more complex: capacity 5, 3 writes, 2 reads, write, read, 4 writes, 2 overwrites, 5 reads. It's been simplified to capacity 3, 3 writes, 1 read, write, overwrite, 3 reads. Closes https://github.com/exercism/todo/issues/79
The last pull request appears to have been deleted following an attempt to resolve a bad merge.
Here it is again
cc @wilmoore