Skip to content

Commit

Permalink
Make Sequence base class.
Browse files Browse the repository at this point in the history 
This lets us share functionality between List and Range (and
other user-defined sequence types).
  • Loading branch information
@munificent
munificent committed Feb 16, 2014
1 parent 4d8bf4b commit f8a9d7f
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 53 deletions.
38 changes: 21 additions & 17 deletions builtin/core.wren
View file
@@ -1,4 +1,22 @@
class List {
class Sequence {
map (f) {
var result = []

This comment has been minimized.

Sign in to view

Copy link
@kmarekspartz

kmarekspartz Feb 16, 2014

Contributor

Should this be new this.type so that this doesn't return a list each time? Implementing classes would then need to provide an add method.

This comment has been minimized.

Sign in to view

Copy link
@munificent

munificent Feb 16, 2014

Author Member

Hmm, good question. Probably, yes.

Want to file a bug so we don't forget?
for (element in this) {
result.add(f.call(element))
}
return result
}

where (f) {
var result = []

This comment has been minimized.

Sign in to view

Copy link
@kmarekspartz

kmarekspartz Feb 16, 2014

Contributor

Same here.
for (element in this) {
if (f.call(element)) result.add(element)
}
return result
}
}

class List is Sequence {
toString {
var result = "["
for (i in 0...count) {
Expand All @@ -16,20 +34,6 @@ class List {
}
return result
}

map (f) {
var result = []
for (element in this) {
result.add(f.call(element))
}
return result
}

where (f) {
var result = []
for (element in this) {
if (f.call(element)) result.add(element)
}
return result
}
}

class Range is Sequence {}
60 changes: 32 additions & 28 deletions src/wren_core.c
View file
Expand Up @@ -41,7 +41,25 @@

// This string literal is generated automatically from core. Do not edit.
static const char* libSource =
"class List {\n"
"class Sequence {\n"
" map (f) {\n"
" var result = []\n"
" for (element in this) {\n"
" result.add(f.call(element))\n"
" }\n"
" return result\n"
" }\n"
"\n"
" where (f) {\n"
" var result = []\n"
" for (element in this) {\n"
" if (f.call(element)) result.add(element)\n"
" }\n"
" return result\n"
" }\n"
"}\n"
"\n"
"class List is Sequence {\n"
" toString {\n"
" var result = \"[\"\n"
" for (i in 0...count) {\n"
Expand All @@ -59,23 +77,9 @@ static const char* libSource =
" }\n"
" return result\n"
" }\n"
" \n"
" map (f) {\n"
" var result = []\n"
" for (element in this) {\n"
" result.add(f.call(element))\n"
" }\n"
" return result\n"
" }\n"
" \n"
" where (f) {\n"
" var result = []\n"
" for (element in this) {\n"
" if (f.call(element)) result.add(element)\n"
" }\n"
" return result\n"
" }\n"
"}\n";
"}\n"
"\n"
"class Range is Sequence {}\n";

// Validates that the given argument in [args] is a Num. Returns true if it is.
// If not, reports an error and returns false.
Expand Down Expand Up @@ -953,16 +957,6 @@ void wrenInitializeCore(WrenVM* vm)
NATIVE(vm->numClass, ".. ", num_dotDot);
NATIVE(vm->numClass, "... ", num_dotDotDot);

vm->rangeClass = defineClass(vm, "Range");
NATIVE(vm->rangeClass, "from", range_from);
NATIVE(vm->rangeClass, "to", range_to);
NATIVE(vm->rangeClass, "min", range_min);
NATIVE(vm->rangeClass, "max", range_max);
NATIVE(vm->rangeClass, "isInclusive", range_isInclusive);
NATIVE(vm->rangeClass, "iterate ", range_iterate);
NATIVE(vm->rangeClass, "iteratorValue ", range_iteratorValue);
NATIVE(vm->rangeClass, "toString", range_toString);

vm->stringClass = defineClass(vm, "String");
NATIVE(vm->stringClass, "contains ", string_contains);
NATIVE(vm->stringClass, "count", string_count);
Expand All @@ -985,6 +979,16 @@ void wrenInitializeCore(WrenVM* vm)
NATIVE(vm->listClass, "[ ]", list_subscript);
NATIVE(vm->listClass, "[ ]=", list_subscriptSetter);

vm->rangeClass = AS_CLASS(findGlobal(vm, "Range"));
NATIVE(vm->rangeClass, "from", range_from);
NATIVE(vm->rangeClass, "to", range_to);
NATIVE(vm->rangeClass, "min", range_min);
NATIVE(vm->rangeClass, "max", range_max);
NATIVE(vm->rangeClass, "isInclusive", range_isInclusive);
NATIVE(vm->rangeClass, "iterate ", range_iterate);
NATIVE(vm->rangeClass, "iteratorValue ", range_iteratorValue);
NATIVE(vm->rangeClass, "toString", range_toString);

// These are defined just so that 0 and -0 are equal, which is specified by
// IEEE 754 even though they have different bit representations.
NATIVE(vm->numClass, "== ", num_eqeq);
Expand Down
4 changes: 1 addition & 3 deletions test/list/map.wren
View file
@@ -1,5 +1,3 @@
var a = [1, 2, 3]
var inc = fn (x) { return x + 1 }
var b = a.map(inc)

var b = a.map(fn (x) x + 1)
IO.print(b) // expect: [2, 3, 4]
1 change: 1 addition & 0 deletions test/list/type.wren
View file
@@ -1,4 +1,5 @@
IO.print([] is List) // expect: true
IO.print([] is Sequence) // expect: true
IO.print([] is Object) // expect: true
IO.print([] is Bool) // expect: false
IO.print([].type == List) // expect: true
8 changes: 3 additions & 5 deletions test/list/where.wren
View file
@@ -1,8 +1,6 @@
var a = [1, 2, 3]
var moreThan1 = fn (x) { return x > 1 }
var moreThan10 = fn (x) { return x > 10 }
var b = a.where(moreThan1)
var c = a.where(moreThan10)

var b = a.where(fn (x) x > 1)
IO.print(b) // expect: [2, 3]

var c = a.where(fn (x) x > 10)
IO.print(c) // expect: []
3 changes: 3 additions & 0 deletions test/range/map.wren
View file
@@ -0,0 +1,3 @@
var a = 1..3
var b = a.map(fn (x) x + 1)
IO.print(b) // expect: [2, 3, 4]
1 change: 1 addition & 0 deletions test/range/type.wren
View file
@@ -1,6 +1,7 @@
var range = 2..5

IO.print(range is Range) // expect: true
IO.print(range is Sequence) // expect: true
IO.print(range is Object) // expect: true
IO.print(range is String) // expect: false
IO.print(range.type == Range) // expect: true
6 changes: 6 additions & 0 deletions test/range/where.wren
View file
@@ -0,0 +1,6 @@
var a = 1..3
var b = a.where(fn (x) x > 1)
IO.print(b) // expect: [2, 3]

var c = a.where(fn (x) x > 10)
IO.print(c) // expect: []

7 comments on commit f8a9d7f

@kmarekspartz
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, would Collection be a better name? Alternatively, Iterable.

@munificent
Copy link
Member Author

Choose a reason for hiding this comment

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

Collection feels wrong to me because that implies that the class stores values, which isn't always the case. (For example, Range doesn't.) Iterable is the more typical name. It's what Java and Dart use, but it's also an odd word. I thought I'd try Sequence and see how it feels. (For what it's worth, Clojure uses ISeq for this, and Scheme uses "sequence".)

What do you think?

@kmarekspartz
Copy link
Contributor

Choose a reason for hiding this comment

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

Not to add too much to the hierarchy, but I was thinking Iterable would provide iterator, iteratorValue, where, and map, and Collection would provide add, clear, etc. and be an Iterable. Set would be a Collection but is unordered so it shouldn't be a Sequence. Range would be Iterable but not Collection.

Scala's hierarchy approximately follows this (It doesn't have a Collection, but provides other layers): http://docs.scala-lang.org/overviews/collections/overview.html

@munificent
Copy link
Member Author

Choose a reason for hiding this comment

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

One of the luxuries of being dynamically typed is that we don't have to build a type hierarchy so that users can use them as type annotations. For example, in Scala, if you want a method that takes an object and calls clear and iterator on it, you need some Collection interface that specifies those.

In Wren, since we're duck typed, we only need base classes to share actual implementations of methods. So we have Sequence as a place to share where and map. For things like add and clear, each concrete type will likely have its own implementation, so we probably don't need a base class for them.

This is new territory for me. My background is big complex OOP hierarchies, so I kind of like that stuff. But with Wren, I'm trying hard to deliberately keep things as simple as possible.

@kmarekspartz
Copy link
Contributor

Choose a reason for hiding this comment

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

That's reasonable. Semantically, I just feel weird saying Set is Sequence.

@kmarekspartz
Copy link
Contributor

Choose a reason for hiding this comment

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

For future reference, Smalltalk: http://www.jmcsweeney.co.uk/computing/m206/collections_intro.php

@munificent
Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, Set is Sequence does feel a bit weird, but I feel like "sequence" is otherwise a nicer, more concrete name for that abstraction. I'm half convinced to switch it to "iterable" but let's try "sequence" for a while and see if we warm up to it. It'll be easy to change for a while.

Please sign in to comment.