review: doc: add documentation for CtCase

[andrewbwogi]

No description provided.

[slarse]

I'm not sure I understand what's meant by this. What is 'default' construct, and what is 'case'?

Do you mean that one can empty the case expressions by setting with null?

[andrewbwogi]

switch (var) {
            case 1:
                a = 1;
            default:
                a = 2;
}

There are two statements in this switch block using two different kinds of labels. If setCaseExpressions is set with null, the default label is used for the CtCase, otherwise the case label is used. Maybe the last sentence is the one that should be in the doc, using the word 'label' instead of 'construct'. Is it clearer?

We could add a setDefault(boolean b) method to CtCase but I'm not sure if isDefault() == false && getCaseExpressions() == null is an acceptable state.

[slarse]

Ah, now I understand what you mean. Yes, using the word "label" is clearer.

I would however not say "is used", because that raises the question "is used for what?". I think the intent here is that, if set with null or an empty list, the CtCase represents the default label. Otherwise, it represents a case label.

This is design is not intuitive IMO, the default label is syntactically distinct from the case label and so should be its own metamodel element. But I don't think there's a good enough reason to change this, and as you noted, adding a setDefault(boolean) may cause the invalid state where a default label has case expressions (well you said the opposite but I think that's what you meant!).

Arguably, we could add a parameterless setDefault() which simply delegates to setCaseExpression(null) and thus wouldn't be breaking. But the docs need to be updated regardless.

Note also that setCaseExpression(null) has the same effect, so it should also be documented.

[andrewbwogi]

I'm skeptical about setDefault() because it has an unexpected side-effect of removing the case expression(s). It would add confusion to an already confusing interface.

[slarse]

Yeah I don't think we want to make this any more convoluted than it already is. It was just an idea for a non-breaking change.

If we were to go ahead with a breaking change however, I think that the way to go would be to have a distinct CtDefault as a subtype of CtCase.

[slarse]

See comment.

[slarse]

LGTM, thanks @andrewbwogi!