Skip to content

A bit of clean-up in TagMap #10164

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.

Sign up for GitHub

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

Open
dougqh wants to merge 5 commits into
base: master
Choose a base branch
from
Open

A bit of clean-up in TagMap #10164

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
95f5808
A bit of clean-up
dougqh Dec 10, 2025
79092b0
spotless
dougqh Dec 11, 2025
13ad995
Merge branch 'master' into dougqh/tagmap-cleanup
dougqh Dec 11, 2025
8026a5f
Merge branch 'master' into dougqh/tagmap-cleanup
dougqh Dec 11, 2025
854633b
Merge branch 'master' into dougqh/tagmap-cleanup
dougqh Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions internal-api/src/main/java/datadog/trace/api/TagMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
*/
public interface TagMap extends Map<String, Object>, Iterable<TagMap.Entry> {
/** Immutable empty TagMap - similar to {@link Collections#emptyMap()} */
TagMap EMPTY = TagMapFactory.INSTANCE.empty();
static final TagMap EMPTY = TagMapFactory.INSTANCE.empty();
Copy link
Contributor

@amarziali amarziali Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fields declared on an interface are already implicitly static final so IMHO this is redundant

Copy link
Contributor Author

@dougqh dougqh Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I know, but I prefer to be explicit.
I want to see final, so that I know it is obviously correct.

Copy link
Contributor

@amarziali amarziali Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally think that we should omit that since it does not improve the readability. A lot of other projects are explicitly suggesting not to do it (i.e. openjdk -> https://cr.openjdk.org/~alundblad/styleguide/index-v6.html).

Copy link
Contributor Author

@dougqh dougqh Dec 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least for final, I think it is good to be explicit. Especially given that we have a lot of contributors that aren't regularly working in Java, so including static final sets an example that works in both classes & interfaces.


/** Creates a new mutable TagMap that contains the contents of <code>map</code> */
static TagMap fromMap(Map<String, ?> map) {
Expand Down Expand Up @@ -2688,23 +2688,22 @@ public void fillStringMap(Map<? super String, ? super String> stringMap) {
}

@Override
public void forEach(Consumer<? super datadog.trace.api.TagMap.Entry> consumer) {
public void forEach(Consumer<? super TagMap.Entry> consumer) {
for (Map.Entry<String, Object> entry : this.entrySet()) {
consumer.accept(TagMap.Entry.newAnyEntry(entry));
}
}

@Override
public <T> void forEach(
T thisObj, BiConsumer<T, ? super datadog.trace.api.TagMap.Entry> consumer) {
public <T> void forEach(T thisObj, BiConsumer<T, ? super TagMap.Entry> consumer) {
for (Map.Entry<String, Object> entry : this.entrySet()) {
consumer.accept(thisObj, TagMap.Entry.newAnyEntry(entry));
}
}

@Override
public <T, U> void forEach(
T thisObj, U otherObj, TriConsumer<T, U, ? super datadog.trace.api.TagMap.Entry> consumer) {
T thisObj, U otherObj, TriConsumer<T, U, ? super TagMap.Entry> consumer) {
for (Map.Entry<String, Object> entry : this.entrySet()) {
consumer.accept(thisObj, otherObj, TagMap.Entry.newAnyEntry(entry));
}
Expand Down