missing_data and getting config elements

[tgibson11]

Rob,

I was starting to look at missing_data usages, and the first one I came across was this, which I don't think should be replaced with an exception.

pysystemtrade/sysdata/config/configdata.py

Line 96 in 16e7dfb

def get_element_or_missing_data(self, element_name):

Instead, I would like to create a new method, get_element(self, element_name, default=None), that would eventually replace get_element_or_missing_data and get_element_or_arg_not_supplied.

Reasoning:

• It is expected that (some) config elements may not be defined. It isn't really "exceptional"
• Also, it would be rather inconvenient to deal with a potential exception every place we need to get a config element
• The current functions require the caller to check for a named_object, and then do something
• "Doing something" is often (or could often be) simply substituting another value for the named_object
• A default parameter could eliminate a lot of "if result is named_object: do something" conditions

If there are cases where an exception would be appropriate, we could also have get_element(self, element_name) that would raise an AttributeError (edit: or a custom exception) if the element doesn't exist. Unfortunately, Python doesn't allow overloaded methods, but there are other ways (less elegant, IMO) of doing this if desired.

What do you think?

[robcarver17]

That's certainly more pythonic and is in the sprit of eg get on dicts and getattr both of which have a similar pattern.

It seems pointless to have two different methods that do the same thing. The only thing I would caution against is the default value of None. I would prefer using missing_data. This can then be substituted for eg missing_argument if this is appropriate.

[tgibson11]

That is an accurate description of my "2-methods" option.

However, I would disagree with this part:

The usual pattern should be that the default is set to the object missing_data (that was what I was trying to say)...Then of course it checks for missing_data

Rather, the usual pattern would be to provide a default that is usable directly in the subsequent code, not something that has to be explicitly checked for. See the 2 sample usages (broker_factory and monitor app) that I did in the change linked to above. That code uses the single-method option, but the difference in those usages would be trivial.

rather than None (using None as a flag rather than an explicit and descriptive object is an anti pattern I really dislike).

You wouldn't use None as a flag, but you might use it if that was actually the value you wanted (for some reason). I think you'd probably agree that is OK.

==========

The rest of this is academic, but I would like to know what you think.

"None as an anti-pattern"...can you link to an explanation of why this is an anti-pattern? For example, do you think this is an anti-pattern?

    def get_element(self, element_name, default=None):
        if default is None:
            result = getattr(self, element_name)
        else:
            result = getattr(self, element_name, default)
        return result

I would say it's not, because None literally represents "nothing", "null" or "no value". And that is precisely what I want to check for here.

I would argue that most of your named_objects are anti-patterns...but my background is mostly in statically typed languages where you either return one pre-defined type, or fail. So maybe this is just a Python thing that's unusual to me.

[robcarver17]

OK you are right, it should return something that can be used downstream, like an empty list for example when you expect a list of instruments. But that default should never be None.

The None as an anti pattern I read somewhere, I'm not just making it up.

[tgibson11]

But that default should never be None.

Probably, but "never" is a strong word. I could imagine a scenario where you would be passing the config value to another function that accepts None as a parameter.

Of course, that function would be in a 3rd-party library, because it would never be allowed in pysystemtrade :-)

Is it safe to say that I have your blessing to proceed with the "2-methods" implementation?

[robcarver17]

Consider yourself blessed.

[tgibson11]

🙏