ArrayMap.py

[hssahota2]

No description provided.

[nedtwigg]

Great start!

[nedtwigg]

More good progress!

[nedtwigg]

Excellent work! This looks great. Two problems:

• ArraySet is missing plusOrThis, which we will need
• Both classes should have an @classmethod empty which returns a singleton. Anytime we use these classes, we should never call their constructor, we should create an empty object and then add things to it. Otherwise, they will not stay storted. This chat describes a design pattern for achieving this.

[nedtwigg]

Almost there!

[nedtwigg]

You don't want to mutate the existing structure, and you want to make sure that ArraySet stays sorted.

selfie-python-wip/jvm/selfie-lib/src/commonMain/kotlin/com/diffplug/selfie/ArrayMap.kt

Lines 257 to 284 in c41e68c

	fun plusOrThis(key: K): ArraySet<K> {
	val idxExisting = binarySearch(key)
	if (idxExisting >= 0) {
	return this
	}
	val idxInsert = -(idxExisting + 1)
	return when (data.size) {
	0 -> ArraySet(arrayOf(key))
	1 -> {
	if (idxInsert == 0)
	ArraySet(
	arrayOf(
	key,
	data[0],
	))
	else ArraySet(arrayOf(data[0], key))
	}
	else -> {
	// TODO: use idxInsert and arrayCopy to do this faster, see ArrayMap#insert
	val array = Array(size + 1) { if (it < size) data[it] else key }
	if (key is String) {
	array.sortWith(STRING_SLASHFIRST as Comparator<Any>)
	} else {
	(array as Array<K>).sort()
	}
	ArraySet(array)
	}
	}

[nedtwigg]

Interesting approach! Here are the tradeoffs:

• selfie python
  • you can call the constructor with any data, and it will be sorted O(n log n).
  • you can call plusOrThis and it will do an O(n) search, followed by an O(n log n) sort in the constructor if it wasn't found
• selfie kotlin
  • you cannot call the constructor, you can only get an empty and plus things from it
  • plusOrThis does an O(log n) search to find out if something has to be added or not. If it does need to be added, the search tells you where it needs to be added, so the constructor doesn't need to do any checks.

Basically, by making the constructor private, we can rely on the data always being sorted, by induction from the empty starting set. That means we never need to do an n log n sort, and can instead always do a log n search.

Your code is simpler and easier to read. There's a good argument to be made that this python is better than the Kotlin. Wait until there's a performance problem, then speed it up.

But if we have allowed the constructor to be public, then it's too late. People might be passing unsorted data in, so we can't make the switch.

It's okay to do the n log n sort instead of the log n search, we can improve performance later, but it's important to hide the constructor.

[nedtwigg]

Great work! Let me know if you're okay with the changes I made above.

There are a few TODOs left in the code:

• a missing binary search which could speed things up
• using a special sort order when the keys happen to be strings

But we don't need to fix them now. Whenever you're satisfied with this PR, click merge and we can move on to the next item in your epic - TypedPath.