MechanicalSoup
MechanicalSoup copied to clipboard
stateful_browser: self.url -> self.__state.url for internal consumers
Be consistent and use self.__state.url over the self.url() @property for internal consumers.
Discussed somewhat in #362, the logic being that:
self.url is an interface for external callers to get access to the internal state, and there's no reason to force the internal users to do so.
There's extra cognitive load for readers of the code to follow through the indirection.
Looking through the code more, I see that we have a similar inconsistency with
self.page
andself.form
. One thing to note, however, is that theself.form
property does more than simply returnself.__state.form
: it also checks to make sure a form has been selected. Therefore, we will likely need to retain most references toself.form
, which prevents us from using the__state
attributes consistently, even if we were to convertself.page
.
That's a good question! Are those checks appropriate in these cases?
For instance, select_form()
sets self.__state.form
before evaluating self.form
, so it's redundant.
It looks like all the other ones are necessary (or, at least, meaningful and probably wise): __setitem__()
, new_control()
, and submit_selected()
.
I guess this means I'm wrong and we should go with self.url
in all cases?
Or maybe we should leave well-enough-alone, I dunno!
I agree, there's definitely not an obvious answer here. As you noted, in some places it is important to use self.form
, but in others it's redundant (or even incorrect, e.g. when we're setting self.__state.form
manually in select_form
). So it's clear we're going to have a mix no matter what we do. Perhaps the most consistent way to do it would be to distinguish between "using" and "setting", which I think would mean using the properties everywhere except in select_form
(and obviously the @property
methods as well).
You are certainly not responsible for making this all consistent, so if this is just a headache you want to forget, that's totally okay. :)