I suggest calling the configuration query "static boolean byteOrderIsBigEndian()". Then it fits better alongside addressSize().
Now that we're down to picking identifiers we're about done. Here are some observations on names relative to Unsafe, for those who like watching paint dry on bikesheds. It's very common to use raw ints as enumeration constants, via this trick of defining "static final" names constants. Andrew, you extend this trick to raw booleans (you can only have two names then but it works). I think that follows the naming pattern of "Unsafe.addressSize", etc. But I also would (slightly) prefer an accessor that returned raw boolean, and whose name clearly documents the sense of the boolean. So a name like isBigEndian or isLittleEndian, returning a forthright true or false, would be reasonable. One nit (and maybe this is why you did it the other way first): Accessors often have get/set prefixes, but Unsafe only uses those sort of prefixes for actual load and store operations. For configuration parameters, there are no get/set. Since "is" is a variation of "get" (for boolean accessors), it looks (in Unsafe) like something that is loading a boolean, but it's not. So I see why adding the "get" or "is" makes a wrong note. — John P.S. Whatever polarity of boolean we pick, let's use it consistently. If we have "boolean bigEndian" somewhere, lets not allow "boolean littleEndian" parameters if we can help it. Using true/1 to signal big-endian seems slightly better than false/0 (though I can't say why). On Mar 5, 2015, at 12:45 AM, Andrew Haley <a...@redhat.com> wrote: > > On 05/03/15 01:01, David Holmes wrote: >> I'm only glancing at parts of this ... >> >> On 5/03/2015 6:21 AM, Andrew Haley wrote: >>> John's suggestion works well; the code is smaller and neater. >>> >>> http://cr.openjdk.java.net/~aph/unaligned.jdk.3/ >> >> This doesn't make sense to me: >> >> 929 // The byte ordering of this machine >> 930 public final static boolean LITTLE_ENDIAN = false; >> 931 public final static boolean BIG_ENDIAN = true; >> >> as they won't necessarily represent "this" machine! And the usage: >> >> ! private static final ByteOrder byteOrder >> ! = unsafe.getByteOrder() == Unsafe.LITTLE_ENDIAN ? >> ByteOrder.LITTLE_ENDIAN : ByteOrder.BIG_ENDIAN; >> >> suggests getByteorder() is really isBigEndian() (which the hotspot part >> confirms) - so then the two constants can be removed completely: >> >> private static final ByteOrder byteOrder >> = unsafe.isBigEndian() ? ByteOrder.BIG_ENDIAN : >> ByteOrder.LITTLE_ENDIAN; > > Indeed, I was thinking of giving that method a better name. Works for me. > > Andrew.
