Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a5855b6) 24.87% compared to head (ce47e42) 24.87%.

@@           Coverage Diff           @@
##           master     #993   +/-   ##
=======================================
  Coverage   24.87%   24.87%           
=======================================
  Files         168      168           
  Lines       18908    18908           
=======================================
  Hits         4704     4704           
  Misses      14204    14204           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Yep, key hashing definitely faster than property name lookup, plus we were doing 2 property name lookups/traversals. Initially via pm.hasNode(_key) which was discarding the node pointer and simply returning a bool, and then another property name lookup during the call to GetPropertyValue(name). Plus I guess a couple of transitions from python code to JSBSim C++ code and back as well.

    def __getitem__(self, key: str) -> float:
        _key = key.strip()
        pm = self.get_property_manager()
        if not pm.hasNode(_key):
            raise KeyError("No property named {}".format(_key))
        return self.get_property_value(_key)

    def get_property_value(self, name: str) -> float:
        """@Dox(JSBSim::FGFDMExec::GetPropertyValue) """
        return self.thisptr.GetPropertyValue(name.encode())

Yes the code of __getitem__ was implemented before FGPropertyNode was included in the Python binding. At that time, the problem was that get_property_value always returns a value whether the property exists or not. So to inform users if they were requesting the value of a property that did not exist, the existence was first tested via has_node prior to calling get_property_value.

Indeed, now that FGPropertyNode is available the sequence of testing the existence of the property then returning the value of that property is now feasible with a single name lookup, which is faster. And it is even faster with a cache. Your patch hits 2 birds with a stone 🐦 👍

Ah, was wondering why it was coded that way.

I'm used to the more aggressive form, "to kill two birds with one stone" 😉

I'm used to the more aggressive form, "to kill two birds with one stone" 😉

Well, I'll try to pretend my error was made on purpose because aviation as a whole needs to be friendlier to the environment 😄