fastutil icon indicating copy to clipboard operation
fastutil copied to clipboard

Map.ofEntries equivalent for fastutil maps

Open viciscat opened this issue 1 year ago • 24 comments

Adding something like Map#ofEntries to the fastutil maps would be neat in my humble opinion. Right now you gotta do ugly stuff like Int2ObjectMaps.unmodifiable(new Int2ObjectOpenHashMap(Map.ofEntries(...))) I could make a PR if you don't have time to. (btw great library much love ❤️)

viciscat avatar Aug 07 '24 17:08 viciscat

Good point, a PR would be welcome! I have the same problem.

vigna avatar Aug 07 '24 17:08 vigna

sweet! Should the methods be added in the Map interfaces or Maps (with an s) classes

viciscat avatar Aug 07 '24 18:08 viciscat

Good point. I'd follow the JCF and put in in Map. But what concrete class do you intend to return, an Open2OpenHashMap in an unmodifiable wrapper?

vigna avatar Aug 07 '24 18:08 vigna

Not sure honestly... I'm not the data structure champion. Maybe an array implementation? I heard that it's good when you just access and don't write in it. Did not do a benchmark tho. The implementation of Map#ofEntries is just an array that alternates between key and value. Won't work for most maps (unless it's the same primitive type for the key and value)

viciscat avatar Aug 07 '24 18:08 viciscat

Well, if it's immutable and holds at most 10 entries I think an ArrayMap is perfectly fine. It could even be the case of creating an ImmutableArrayMap, but that would mean another 90 classes. Maybe as an internal class of Map, just copying the necessary code from ArrayMap?

vigna avatar Aug 07 '24 18:08 vigna

What if it's more than 10 entries? HashMap?

viciscat avatar Aug 07 '24 18:08 viciscat

Or I could just return an ArrayMap / OpenHashMap in an unmodifiable wrapper depending on the number of entries. That would be the easiest to write.

viciscat avatar Aug 07 '24 18:08 viciscat

Oh you're right. Map.of has at most 10 elements, but Map.ofEntries has variable length.

Yes, you could to ArrayMap up to, say, 8 elements, and, an OpenHashMap otherwise.

vigna avatar Aug 07 '24 19:08 vigna

I'm guessing tests would be wanted? I don't see any test classes for the Map interfaces. Do I create a new one? Do I do it like Primitive2PrimitiveMapTest and Primitive2ObjectMapTest?

viciscat avatar Aug 07 '24 21:08 viciscat

Yes, MapTest sounds right. Like Int2IntMapTest and Object2ObjectMapTest should be enough.

Please double check that I didn't put in Maps stuff that in Java is in Map... Or we might have to rethink the position.

vigna avatar Aug 07 '24 21:08 vigna

Some MapTests have mentions to Maps methods. But they are present to create singletons, not actually tested. MapGenericTest uses and tests Maps stuff tho.

viciscat avatar Aug 07 '24 21:08 viciscat

Hey having a little trouble compiling a jar... Running ant jar tries to compile the drv files and I don't think that's normal 😅

viciscat avatar Aug 08 '24 15:08 viciscat

No, it shouldn't. Did you make -s sources first?

vigna avatar Aug 08 '24 15:08 vigna

Did not include -s since it isn't specified in the readme but yes I did.

viciscat avatar Aug 08 '24 15:08 viciscat

Oh yeah that's just "silent" to avoid the mess. Platform? OS?

vigna avatar Aug 08 '24 15:08 vigna

Windows 11 😬 I'm using WSL2 to run the linux things.

viciscat avatar Aug 08 '24 15:08 viciscat

Hey it's me again, turns out I just can't read errors. It seems some java files are not getting generated correctly? For example Byte2ByteLinkedOpenHashMap.java gets generated with just OpenHashMap.drv inside of it. It might be more WSL shenanigans but asking nonetheless in case it's a known issue. image

viciscat avatar Aug 11 '24 10:08 viciscat

That scripting part is for UN*X variants. That's a WSL issue, I'm sorry but I can't help you...

vigna avatar Aug 11 '24 10:08 vigna

I can't seem to find anywhere in gensources.sh or the makefile how it detects and replaces the drv file that just contains the name of another drv file. Kinda hard to debug when I don't know how it's done

viciscat avatar Aug 11 '24 12:08 viciscat

It might be a problem with different treatment of soft links, as the .drv files of non-plain hash maps are just soft links to the real ones.

vigna avatar Aug 11 '24 12:08 vigna

Hey I made this little workaround in gencsource.sh and it works great it seems. Should I commit that as well or no? (The $1 at the end of the file is replaced by $TARGET_DRV_FILE)


TARGET_DRV_FILE=$1

# A workaround for Windows and WSL. Linked files only contain the "target" file's name on windows.
# So we check the first line to see if it contains ".drv". If so replace the target drv file by that.
FIRST_LINE=$(head -n 1 $TARGET_DRV_FILE)
if [[ $FIRST_LINE == *".drv"* ]]; then
  TARGET_DRV_FILE="drv/$FIRST_LINE"
fi

viciscat avatar Aug 11 '24 15:08 viciscat

So should I keep that fix to myself or commit it?

viciscat avatar Aug 13 '24 19:08 viciscat

The problem is checking that it works everywhere.

vigna avatar Aug 13 '24 19:08 vigna

Ah yeah that makes sense. I won't commit it and just keep it there in this issue. Might help someone

viciscat avatar Aug 13 '24 19:08 viciscat