Is there a way to simplify this code or at least m...

# getting-started

[Ayfri]

Is there a way to simplify this code or at least make it more beautiful ?

val List<Int>.minmaxDifference get() = maxOf { it } - minOf { it }
// chestRoom is a set, blockX/Y/Z are Int
// CHEST_ROOM_MIN_SIZE & MAX_SIZE are const val Int

if (chestRoom.map { it.blockX }.minmaxDifference in CHEST_ROOM_MIN_SIZE..CHEST_ROOM_MAX_SIZE) return false
if (chestRoom.map { it.blockY }.minmaxDifference in CHEST_ROOM_MIN_SIZE..CHEST_ROOM_MAX_SIZE) return false
if (chestRoom.map { it.blockZ }.minmaxDifference in CHEST_ROOM_MIN_SIZE..CHEST_ROOM_MAX_SIZE) return false

[ephemient]

I would probably write one of

inline fun <T, R : Comparable<R>> Iterable<T>.minmaxOrNullBy(selector: (T) -> R): Pair<R, R>?
chestRoom.minmaxOrNullBy { it.blockX }?.let { (min, max) -> max - min in CHEST_ROOM_MIN_SIZE..CHEST_ROOM_MAX_SIZE }

inline fun <T> Iterable<T>.largestDeltaBy(selector: (T) -> Int): Int
chestRoom.largestDeltaBy { it.blockX } in CHEST_ROOM_MIN_SIZE..CHEST_ROOM_MAX_SIZE

instead

[Ruckus]

I'm not sure what your idea of beautiful is, but I think the biggest problem with the code is it's not immediately obvious what it's doing, and you have to reason it through to realize you're just doing a box bounds check.

I would probably make a bounds object of some sort, and then define a fun Set<ChestRoom>.calculateBounds(): Bounds, and then

return bounds.dx in MIN..MAX && bouds.dy in MIN..MAX && <http://bounds.dz|bounds.dz> in MIN..MAX

[ephemient]

you could choose to be clever and write something like

arrayOf(::blockX, ::blockY, ::blockZ)
    .any { largestDeltaBy(it) !in CHEST_ROOM_MIN_SIZE..CHEST_ROOM_MAX_SIZE }

but that doesn't really help clarity in this case, IMO

[Ayfri]

okay I see, it's a bit more clearer at least

[Ruckus]

But then my lack of understanding could just be my inexperience in the domain, so take what I say with the appropriate portioning of salt.

[bezrukov]

I would consider switching from blockX/blockY/blockZ in the model to just an array, most likely you have similar repetitive code in other places
In this case all the code will be simplified without a largestDeltaBy (if you still need I would suggest an amplitudeOf { key } name):

return (0 until AXES).none { axe ->
    chest.maxOf { it.block[axe]} - chest.minOf { it.block[axe] } in  CHEST_ROOM_MIN_SIZE..CHEST_ROOM_MAX_SIZE
}

return (0 until AXES).