docstring
This class really needs a docstring.
Mitigating that a little, I truly appreciate the
automated test suite.
It has great educational value
to someone considering making calls into this class.
There is a middle ground, of sorts, offered by
python -m doctest *.py.
It's no substitute for a nitty gritty test like
test_repr()
, which should stay where it is.
But it lets you pick out the most common things
a new user would be likely to do and to have questions about,
and educate the user in a concise way.
It's docs we can believe, even after edits to the
library code, since the machine verifies
their accuracy on every test run.
motivating example
The supplied test suite, while instructive,
alas is very generic.
It would be helpful to show how, IDK, a Flask app,
or some other Use Case, can solve a business problem
using this library more simply than it could with
a competing technique.
(defaultdict(list)
springs to mind.)
DRY
super().__setitem__(key, [value])
self._count += 1
else:
super().__getitem__(key).append(value)
self._count += 1
nit: There's an opportunity to follow this code
with just a single unconditional increment.
temp var
These are just odd:
_ = super().pop(key)
...
_ = super().__getitem__(key)
...
_ = super().pop(key)
I imagine you found it convenient for print(_)
debugging
during early development?
On the plus side, you're following convention,
so linters won't flag it.
Though we're more likely to see it in a tuple unpack context:
a, _, _, d = some_four_tuple
And kudos on insisting that, "_count
is private! Go use len()!"
IP
The repo's README mentions "free to use",
yet confusingly we do not see a LICENSE file.
The repo currently is covered by "all rights reserved"
copyright, since the documents were born under U.S. copyright law.
Recommend you choose "MIT license" each time
you create a new GitHub repository.
The code you chose to post on Stack Exchange,
of course, is in quite a different category.
You chose to publish it under CC BY-SA 4.0 terms.
documented semantics
constructor
I found this behavior confusing / unintuitive.
>>> d = MultiDict({'a': 1})
>>> d['a'] = 2
Traceback (most recent call last):
File "<python-input-6>", line 1, in <module>
d['a'] = 2
~^^^^^
File "/tmp/multidict.py", line 13, in __setitem__
super().__getitem__(key).append(value)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'int' object has no attribute 'append'
It wasn't obvious what a valid ctor argument would look like.
Upon being presented with an invalid argument, it would
be much much better to raise
immediately than to
let some delayed invocation blow up.
In this example it's easily diagnosed because both call sites
are adjacent, but in production code we often have many
intervening lines of code, and often will have calls at
different depths in the call stack, leading to a more
challenging debug session for some hapless maintenance engineer.
deletion
I similarly found this confusing / unintuitive.
Apparently insert & delete are not symmetric with each another.
You had a different concept of what a MultiDict looks like
than I did, partly due to a comparison I drew with a MultiSet.
Writing some English sentences of documentation would
help to align an app engineer's thinking with your own.
>>> d = MultiDict()
>>> d['a'] = 1
>>> d['a'] = 2
>>> del d['a']
>>> del d['a']
Traceback (most recent call last):
File "<python-input-23>", line 1, in <module>
del d['a']
~^^^^^
File "/tmp/multidict.py", line 17, in __delitem__
self._count -= len(self[key])
~~~~^^^^^
KeyError: 'a'
I thought the first del
would delete the \$2\$ element and decrement count from
\$2 \rightarrow 1\$, second del
sends count from
\$1 \rightarrow 0\$, and only an attempted third del
would be invalid app-level code.
But now I see you had a different concept in mind, OK.
Actually, having read a
del test,
now it makes perfect sense to me.
Partly this is due to the documentation's focus
on manipulating protocol headers, so
del headers["Received"]
leads to a different
intuition.
To "shorten" an entry we would need to go
outside what the OP code offers convenient access to.
Though such mutation leaves _count
in the wrong state.
>>> d
MultiDict({'a': [1, 2]})
>>> vals = d['a']
>>> del vals[-1]
>>> d['a']
[1]