Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct doc block return types. #5084

Merged
merged 2 commits into from Nov 5, 2014
Merged

Correct doc block return types. #5084

merged 2 commits into from Nov 5, 2014

Conversation

dereuromark
Copy link
Member

Lets remove some "mixed" return types. Otherwise you could just as well omit it ;)
Also some more cleanup.

To stay BC I did not modify any code and rather fixed the doc block - even though in some places one should. It would be cleaner to always return string (empty string then instead of null).

E.g.

/**
 * Returns the schema name. Override this in subclasses.
 *
 * @return string|null schema name
 */
    public function getSchemaName() {
        return null;
    }

should be

/**
 * Returns the schema name. Override this in subclasses.
 *
 * @return string Schema name
 */
    public function getSchemaName() {
        return '';
    }

And stuff like

/**
 * Get the last error as a string.
 *
 * @return string|null Last error
 */
    public function lastError() {
        if (!empty($this->lastError)) {
            return $this->lastError['num'] . ': ' . $this->lastError['str'];
        }
        return null;
    }

should be

/**
 * Get the last error as a string.
 *
 * @return string Last error
 */
    public function lastError() {
        if (!empty($this->lastError)) {
            return $this->lastError['num'] . ': ' . $this->lastError['str'];
        }
        return '';
    }

Maybe for 2.6?

PS: Since quite a few of those "return null"s weren't documented, one could maybe even do this here as a bugfix instead of modifying the doc block?

@dereuromark dereuromark added this to the 2.5.6 milestone Nov 5, 2014
markstory added a commit that referenced this pull request Nov 5, 2014
@markstory markstory merged commit d5d8b32 into master Nov 5, 2014
@markstory
Copy link
Member

Removing the null types and adding more consistent types makes sense for 2.6 if it does in the way you described.

@markstory markstory deleted the master-docblock-return branch November 5, 2014 13:02
@dereuromark
Copy link
Member Author

Will do so after the 2.6 merge :) 2.6...master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants