Possible bug in Record->getValue(): it sets $out=null before returning even though $out has a value from e.g. "fieldname_init()"

[TheJoeSchr]

Hi, I just tried to debug an error and came up on lines 1980/1999 in Record.php. It seems to be wrong to me, to set $out=null after first painstakingly getting it. Also it makes all my transientValues return null the first time getValue is called on them. I would like to fix this or make an PR, but I don't know how I could unittest this for regression, sideeffects to it.

This is the part I'm taking about: https://github.com/shannah/xataface/blob/32af6da431ccf687cdfd49870040d0308fd30827/Dataface/Record.php#L1980

https://github.com/shannah/xataface/blob/32af6da431ccf687cdfd49870040d0308fd30827/Dataface/Record.php#L1999

1980    $out = null;
                    }
                } else if ( ( $parent =& $this->getParentRecord() ) and $parent->_table->hasField($fieldname) ){
                    return $parent->getValue($fieldname,$index,$where,$sort,$debug);
                } else {
                    $this->_values[$fieldname] = null;
                    $out = null;
                }
            } else {
                $out = $this->_values[$fieldname];
            }
            if ( isset($out) ){
                // We only store non-null values in cache.  We were having problems
                // with segfaulting in PHP5 when groups are used.
                // This seems to fix the issue, but let's revisit it later.
                $this->cache[strval(__FUNCTION__)][strval($fieldname)][$index][$where][$sort] = $out;
            }
1999        return $out;

[shannah]

I agree that $out = null looks wrong. The unit tests are a bit out of date so I don't think they may not be of that much value here. I have been meaning to set up CI with travis but not sure when I'll have time to do that.

[TheJoeSchr]

Thanks for getting back to me!

I decided to take out the line with $out = null in our development branch, we will probably put it into production the next week. So far everything seems good. I will get back, if it does something unexpected.

Are you generally open to PR or don't you have that much time for this project anymore? I sometimes come across other minor stuff, which I sometimes fix or modify according to our use cases. I suspect not all are useful for general public, but some might be.

Thanks for explaining the status about the unit tests, I understand that it takes a lot of time to set them up after the fact, so I will just use good old reading and thinking :)

[shannah]

Yes. Please send the PR. Pull requests are the best way to contribute changes.