Skip to content

Commit

Permalink
Fix ModifiedButNotUsed doc
Browse files Browse the repository at this point in the history
The code snippet was calling ".toBuilder" on something that was already a Builder (which I’m 99% sure does not compile?).

The documentation "or the Builder modified in place" which sounds to me like it implies it is a preferred solution. So should be tagged as ".good".

Finally, renamed "setFoo" to "withFoo", as that seems like the convention for functions that "return a modified copy, but don’t touch input". Added "@CheckReturnValue" to drive home the point that if you are calling this method, you still need to *also* use the returned value.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=231218053
  • Loading branch information
pferaud authored and ronshapiro committed Jan 31, 2019
1 parent 66ab61e commit af46227
Showing 1 changed file with 5 additions and 4 deletions.
9 changes: 5 additions & 4 deletions docs/bugpattern/ModifiedButNotUsed.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,19 @@ something is done with the return value:
}
```

As protos are immutable, the return value must be used:
As protos are immutable, either the return value must be used:

```java {.good}
MyProto setFoo(MyProto proto, String foo) {
@CheckReturnValue
MyProto withFoo(MyProto proto, String foo) {
return proto.toBuilder().setFoo(foo).build();
}
```

or the Builder modified in place:

```java {.bad}
```java {.good}
void setFoo(MyProto.Builder protoBuilder, String foo) {
protoBuilder.toBuilder().setFoo(foo);
protoBuilder.setFoo(foo);
}
```

0 comments on commit af46227

Please sign in to comment.