[GitHub] [geode] upthewaterspout opened pull request #3229: Fixing a number of static fields to be immutable

["upthewaterspout (GitHub)" <gi...@apache.org>]

Going over the list of static fields that are marked with @MakeImmutable and fixing most of them. There are a few left in redis and a couple of static arrays in the public API that I'm not sure what to do with.


[ Full content available at: https://github.com/apache/geode/pull/3229 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] upthewaterspout commented on pull request #3229: GEODE-6380: Fixing a number of static fields to be immutable

["upthewaterspout (GitHub)" <gi...@apache.org>]

One annoying thing about java is that there is no way to declare a byte literal. So we can either cast here in the constructor, or when we declare the constants - like this

`
public static final InterestPolicy CACHE_CONTENT = new InterestPolicy("CACHE_CONTENT", **(byte)** 1);
`

If you prefer the above approach, I can change it.

[ Full content available at: https://github.com/apache/geode/pull/3229 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt commented on pull request #3229: GEODE-6380: Fixing a number of static fields to be immutable

["mcmellawatt (GitHub)" <gi...@apache.org>]

Oh ok - no I think between the two I am with you that casting in the constructor is a little more explicit.  Thanks for clarifying!

[ Full content available at: https://github.com/apache/geode/pull/3229 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt commented on pull request #3229: Fixing a number of static fields to be immutable

["mcmellawatt (GitHub)" <gi...@apache.org>]

is there any good reason this would have been a singleton before?  Are ANSIHandlers expensive to create or something?

[ Full content available at: https://github.com/apache/geode/pull/3229 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt commented on pull request #3229: Fixing a number of static fields to be immutable

["mcmellawatt (GitHub)" <gi...@apache.org>]

maybe just preference, but I did like `cleanByteArray` over just `array` here as a variable name.  A little more explicit in usage/intent.

[ Full content available at: https://github.com/apache/geode/pull/3229 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] upthewaterspout commented on pull request #3229: GEODE-6380: Fixing a number of static fields to be immutable

["upthewaterspout (GitHub)" <gi...@apache.org>]

One annoying thing about java is that there is no way to declare a byte literal. So we can either cast here in the constructor, or when we declare the constants - like this

```
public static final InterestPolicy CACHE_CONTENT = new InterestPolicy("CACHE_CONTENT", *(byte)* 1);
```

If you prefer the above approach, I can change it.

[ Full content available at: https://github.com/apache/geode/pull/3229 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt commented on issue #3229: Fixing a number of static fields to be immutable

["mcmellawatt (GitHub)" <gi...@apache.org>]

Might add GEODE-6380 to the PR title for consistency.

[ Full content available at: https://github.com/apache/geode/pull/3229 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt commented on pull request #3229: Fixing a number of static fields to be immutable

["mcmellawatt (GitHub)" <gi...@apache.org>]

Would it be more appropriate to treat ordinal as either a byte or int universally wherever it is used instead of downcasting?

[ Full content available at: https://github.com/apache/geode/pull/3229 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt commented on pull request #3229: Fixing a number of static fields to be immutable

["mcmellawatt (GitHub)" <gi...@apache.org>]

Minor - can "refresh" be pulled into a final shared field? for this line and the one below it?

[ Full content available at: https://github.com/apache/geode/pull/3229 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] upthewaterspout closed pull request #3229: GEODE-6380: Fixing a number of static fields to be immutable

["upthewaterspout (GitHub)" <gi...@apache.org>]

[ pull request closed by upthewaterspout ]

[ Full content available at: https://github.com/apache/geode/pull/3229 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] upthewaterspout commented on pull request #3229: GEODE-6380: Fixing a number of static fields to be immutable

["upthewaterspout (GitHub)" <gi...@apache.org>]

One annoying thing about java is that there is no way to declare a byte literal. So we can either cast here in the constructor, or when we declare the constants - like this

`
public static final InterestPolicy CACHE_CONTENT = new InterestPolicy("CACHE_CONTENT", (byte) 1);
`

If you prefer the above approach, I can change it.

[ Full content available at: https://github.com/apache/geode/pull/3229 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] upthewaterspout commented on pull request #3229: GEODE-6380: Fixing a number of static fields to be immutable

["upthewaterspout (GitHub)" <gi...@apache.org>]

I don't think there is a good reason. It's a small class with one boolean field for state. Really cheap.

[ Full content available at: https://github.com/apache/geode/pull/3229 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org