Editor has dubious return to ajax(newAjax)

Editor has dubious return to ajax(newAjax)

ghherlinghherlin Posts: 20Questions: 3Answers: 0

In searching for a solution to another problem, I stumbled on this:

/**
 * Get / set the Ajax configuration for the Editor instance
 *
 * @return {Editor} Editor instance, for chaining
 */
function ajax(newAjax) {
    if (newAjax) {
        this.s.ajax = newAjax;
        return this;
    }
    return this.s.ajax;
}

IMHO, the return value should either be this in all branches, or else the comment should say "returns the ajax object" and the first "return" should be eliminated. In fact, the "return" in the condition should be eliminated.

To be clear, here are the 2 rational options:

/**
 * Get / set the Ajax configuration for the Editor instance
 *
 * @return this {Editor} Editor instance, for chaining
 */
function ajax(newAjax) {
    if (newAjax) {
        this.s.ajax = newAjax;
    }
    return this;
}

or

/**
 * Get / set the Ajax configuration for the Editor instance
 *
 * @return this {Editor] ajax object as set or found
 */
function ajax(newAjax) {
    if (newAjax) {
        this.s.ajax = newAjax;
    }
    return this.s.ajax;
}

best regards

George

Replies

  • allanallan Posts: 63,678Questions: 1Answers: 10,497 Site admin

    Hi George,

    That's for letting me know about that. That function is a getter / setter, so it needs to return a different value depending on the parameter passed in. You are right that the comment there doesn't reflect the function correctly and I've patched that locally now.

    On the plus side, the public documentation for this method is correct :).

    Allan

Sign In or Register to comment.