Reworked HashDictionary #494
Conversation
| for ((index, item) in iterator) | ||
| expect(item, is equal to(list[index])) | ||
| expect(iterator hasNext?(), is equal to(false)) | ||
| secondIterator := list iterator() |
There was a problem hiding this comment.
You forgot to free secondIterator
There was a problem hiding this comment.
@thomasfanell I've seen many tests where nothing at all is freed. I thought I was just being pedantic sometimes. But you're right, will fix.
| @@ -3,15 +3,30 @@ import structs/[HashBag, HashMap, ArrayList] | |||
| HashDictionary: class { | |||
| _myHashBag: HashBag | |||
| _capacity: Int | |||
| size ::= this _myHashBag size() | |||
There was a problem hiding this comment.
Would it be a good idea to call this count instead, as in our other collections? (just this class, don't change anything in HashBag)
|
Fixed according to comments. |
| @@ -104,7 +104,7 @@ HashDictionaryTest: class extends Fixture { | |||
| dictionary add("First", 1337) | |||
| expect(dictionary getAsType("First", Int) == 1337) | |||
| expect(dictionary getAsType("First", String) == null) | |||
| expect(dictionary getAsType("Second", Int) == null) | |||
| //expect(dictionary getAsType("Second", Int) == null) //FIXME Can we do this without obnoxious Pointer-To-Int warnings? | |||
There was a problem hiding this comment.
getAsType: func <T>(key: String, T: Class) -> T {, which means that dictionary getAsType("Second", Int) will return an Int. If you compare an Int to null, you will get the warning. If the test would follow the warning The Class parameter for Covers must be Cell<Cover> by calling dictionary getAsType("Second", Cell<Int>) the warning should go away, but then you also need to use Cell when adding things to the dictionary, or things will go badly.
There was a problem hiding this comment.
Ah, this should be solved as I did with Future. Will fix.
marcusnaslund
changed the title
|
The updated |
| expect(dictionary get("TestClassValue", this defaultClass) stringVal == "String", is true) | ||
| expect(dictionary get("Nonexistent", this defaultClass) stringVal == "Default", is true) | ||
| expect(dictionary get("TestClassValue", this defaultClass) intVal == 1, is true) | ||
| expect(dictionary get("Nonexistent", this defaultClass) intVal == 0, is true) | ||
| }) | ||
| this add("ArrayList", func { |
There was a problem hiding this comment.
It's not an ArrayList anymore, so you should change this thing as well.
|
Updated: |
|
|
||
| init: func(=val) | ||
| init: func ~withtype(=val, =type) |
There was a problem hiding this comment.
What do you need this for, anyway?
There was a problem hiding this comment.
Oh, the sdk uses spaces, I see.
This is to please the get functions in HashDictionary - I need to store the classtype T in order to check whether the requested type actually exists for a particular key.
|
|
||
| init: func(=val) | ||
| init: func ~withtype(=val, =type) |
There was a problem hiding this comment.
I don't think we should add this just to get HashDictionary (which we're not even using) working. Besides, Cell should already know what T is, it shouldn't need to be stored like this. I'm going to let this sit for a while until someone comes up with a better idea.
There was a problem hiding this comment.
We could store the classtype in HashEntry instead perhaps?
|
Updated with cleanup of |
|
Yes, split it up. |
marcusnaslund
changed the title
|
Couldn't find a need for |
| @@ -1,97 +1,90 @@ | |||
| import structs/[HashBag, HashMap, ArrayList] | |||
|
|
|||
| HashDictionary: class { | |||
| _myHashBag: HashBag | |||
| _hashBag: HashBag | |||
| _capacity: Int | |||
There was a problem hiding this comment.
Doesn't look like this is actually used for anything at all. Fix now, or fix later? Your call.
|
Updated and ready. |
Reworked HashDictionary
Adds missing tests for
HashDictionaryandVectorList.Some cleanup of
HashDictionary. More can be done (it usesArrayListandHashBagfrom the sdk for example) but we can look at that if/when we will use this class.Right now it doesn't seem to be used (see #460 )