Thanks for the thoughts!
On 11/11/2022 21.54, dn wrote:
PyCharm is warning against using an identifier of all upper-case letters
as a function's parameter, saying "Argument name should be lowercase".
(weak, code smell)
...
PEP-008: makes no mention, and is but a guide anyway.
(that said, if one 'breaks a rule', then a good reason should be required!)
YELLING is an email-interpretation. Does it apply to code? Particularly
as we are talking Python, where an identifier using all upper-case
letters is a convention (not a language-structure, as stated in
contributions 'here') that is widely used.
Accordingly, the purpose of the UPPER_CASE is to communicate the
read-only nature of the data-item to the programmer. Combine this with
"we're all adults here", and if someone is foolish-enough to modify a 'constant', then Python will comply - but (s)he can expect push-back
during a Code Review! So, yes, «weird to see an all-cap parameter» (I
thought so too) but isn't that effect the purpose of the convention in
the first place?
There are several aspects of Python where the 'we're all adults'
thinking must be applied, eg 'private attributes'. Given that
understanding, it doesn't seem necessary to force constant-behavior on
the parameter - although when one does, there are a couple of mechanisms
and more than several projects working on this and other aspects of
enforcing data-types and their characteristics!
That said, and with @Stefan's observations, there are many reasons why
this code is piling-up trouble - and thus, should be avoided...
def build_product_prices_dictionary( WORKBOOK_DEFINITIONS:dict )->dict:
...
price_array = xl.iget_array(
file_name=WORKBOOK_DEFINITIONS[ "file_name" ],
...
(the function def is flagged, as above)
As far as the function is concerned, the dict and its contents are
constants.
(but the dict can't be treated as a global ENV[IRONMENT] object, because
it has to cross into the module's namespace)
By way of background, in recent times, have the habit/standardised code 'template' for config/environment-setting, which instantiates a class
from (at least) JSON/YAML files. Accordingly, the instance is named in lower-case, and particular groups of parameters (as here) can be passed
as an extract-function/property (as a general style, I avoid long parameter/argument lists, preferring a data-collection - which landed me
in this...).
Which re-raises the question the other way around: how does the object's
name indicate its constant/read-only nature to the reader?
"Ouch" says idealism!
Back to this case, which part of a tutorial comparing different methods
to access prices in a product-catalog[ue] (eg hard-coded dict,
flat-file, workbook, database, etc). Some members of the group are
either anti-OOP or OOP-untrained. Hence avoiding my personal 'good, old, stand-by'.
(plus it was Veterans'/Remembrance/Liberation Day, which we don't
observe (much) here (ANZAC Day instead) and I was using some
creative-coding time to [try to] take my mind off it)
The particular @Rob and I both find it peculiar (in the same sense) but as-above, that's (ultimately) the point of the upper-casing.
The idea of moving the workbook-definitions into that module appears to
make sense and could easily be achieved - but in the singular context of
this example. However, as a matter of style, all of an application's environment-setting would best be done in one place - with cmdLN, config-file(s), etc, all being combined, and with the output of a single 'authority' as its objective.
I could try to 'get away with it', but if some Apprentice takes the idea
and later uses it as a 'template', a general grumpiness will result...
(no it's not a learning-objective, but sometimes people take-away
unintended 'side effects', maybe like this).
However, in that spirit, am contemplating use of a DataClass. It will be
as easy to read as any config file, even to non-OOP-ers; has no learning side-effect down-side (that I've spotted, as-yet); and can be 'seen' as
a instance by the workbook module (instance named using lower-case).
This, hopefully also measuring-up to @Thomas' observations of risk that
the current code's intentions be misconstrued later.
I'll admit to (idealists look away now!) not being 'above' the use of
globals, particularly?if only an ENVIRONMENT class - IIRC this
irresponsible behavior being when the environment is fairly trivial (cf multi-layered or large numbers of 'tunable factors').
Because the ENVIRONMENT is returned to the mainline, which subsequently
uses those parameters to call the 'action' functions/methods (and know
which to call), it is not available within the import-ed module's
namespace. Hence the fall-back to passing it/the Workbook sub-set of parameters, as an argument. (see above for normal/traditional 'solution'
to traversing namespaces)
Is passing the dict as an argument/parameter considered to be
incompatible with its designation as a constant?
There's an argument to be made, particularly by those from other
languages, relating to pass-by-reference, pass-by-value; which suggests
that the WORKBOOK_DEFINITIONS parameter is effectively being used on the
LHS of an assignment...
Probably 'nuff said'!
Perhaps the style should be more enum-like, ie the dict's name in
lower-case, with the key-named in upper case, eg
workbook_definitions[ "FILE_NAME" ]
Like @MRAB, I'm thinking that getting tangled-up in the conventions of
constant identification, cf wanting to convey the read-only nature of
the construct is unhelpful (and an unneeded headache from over-thinking).
Changing the dict-keys to upper-case encourages the 'constant/read-only' interpretation, and might cause least distraction to the learning/discussion/comparison objectives...
Advice, comments, critique welcome!
--
Regards,
=dn
--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)