|
Feedback is appreciated
Over the years I have witnessed multiple shoddy implementations of item containers.
One that returned a Boolean on success or fail (this was on the right track in this way, but it was not specific enough. why did it fail?)
One wouldn't check max amounts and overflow.
One WOULD check max amounts and not overflow, but the remaining item disappeared.
One wouldn't check if given ind(ex/ices) were valid.
One wouldn't check if an item was in the slot the client said it was in.
One wouldn't properly handle errors.
One would keep anArray<Short>
for item ids, anArray<Int>
for an item amount and anArray<Int>
for item charges (even if the item actually had no charges). This... system. It was present in ALL item containers. Equipment, Inventory, Bank, Store, etc. THAT was a real pain in the ass to read and make big changes to.
Welcome to Container.
I got the idea of creating Container after years of witnessing the atrocities listed above.
Container is an interesting different way to go about handling Container operations. Well, compared to what I have seen out in the wild. So, I figured I would rewrite some common code in to something a bit different and... Modern? Isn't functional programming all the rage these days?
Container tries to solve a lot of the logistical problems that people have when working with Containers, while also promoting a safer more direct way of handling possible errors.
Here's some features that give credit to the Safety and Clarity of Container:
- With
Container
, the safety and clarity of your code go hand in hand. There is a plethora of return types, as well as a neatOption
-like functional functions that allow you to handle each scenario individually without having to use a when statement every time.- There is an
Option
inspired return typeContainerResult
with functional methods that allow you to handle the results how you like.- You can handle them on an individual basis.
- You can handle them on a more generic basis (Handle either
Success
orFailure
)- There are functions that handle the basics (Shifting, adding and removing items, etc) for you.
- There are a few basic verification functions (verify that 1 item X is in slot Y, verify that X items Y are in slot Z), as well as one you can use for yourself.
- You have to completely change the calling code to handle it in the "unsafe" way (
Container.unsafe
), as the equivalents between the two dont return the same things in nearly all cases.- The code you write is very likely to be much easier to read than before.
- NEW V2: You can use generic classes with Container!
Here's some cons of using Container:
- No way to force you to handle all the different
ContainerResult
, though that isn't really necessary as Container lets you handle what you want to handle.- It's less performant (not noticeable in realistic scenarios) than the more "traditional" ways of handling this kind of stuff, but safety always comes at the cost of performance when talking about code.
Container is an interesting and very different way to go about handling Containers and what they can return, at least compared to what I have seen out in the wild. I figured this might bring them a bit more in to the modern world.
Spoiler for links:
Due to the nature of being developed in Kotlin, Container is meant to be written in Kotlin. This is mainly thanks to Extension functions and last-param lambdas. I don't guarantee anything feels good to write in Java!
Spoiler for examples:
Yep this was my feedback, BaseItem really shouldn't be dependent on ItemDefinition, you can see the costs of it in your test which while exclusively testing Container still has dependencies on Item and ItemDefinition, you said you're pretty new to tests so I recommend looking into mocking and TDD
Also I'm just not a fan of the way you format code, especially when this is aimed for newer guys to the language
Does not seem particularly friendly to read than the standardCode:/** * Returns: Success.VerifyItem, Failure.BadIndex, Failure.ItemIdMismatch or Failure.NotEnoughItemAmount */ fun verifyAtLeast(itemToVerify: BaseItem, slotIndex: Int): ContainerResult = verify(itemToVerify, slotIndex) { if (!it.sharesItemIdWith(itemToVerify)) return@verify false if (it.itemAmount < itemToVerify.itemAmount) return Failure.NotEnoughItemAmount true }
A personal opinion but omitting curly braces while looks nice, does decrease readability and increase chances of mistakes.Code:/** * Verifies an item is contained at a set index * @param itemToVerify The item to check for * @param slotIndex The containing slot index to check * @return ContainerResults Success.VerifyItem, Failure.BadIndex, Failure.ItemIdMismatch or Failure.NotEnoughItemAmount */ fun verifyAtLeast(itemToVerify: BaseItem, slotIndex: Int): ContainerResult { verify(itemToVerify, slotIndex) { if (!it.sharesItemIdWith(itemToVerify)) { return@verify false } else if (it.itemAmount < itemToVerify.itemAmount) { return Failure.NotEnoughItemAmount } else { return@verify true } } }
But as always, great to see you putting guides and snippets out for kotlin
Thank you for the feedback, it does help! I will work on some of this when I go to work later today. Here's my immediate thoughts.
The friendliness was in reference to the code one would write rather than the source code, though I do think the source code is quite friendly in most areas. I will make changes to make it more... "standard", though some choices were a personal choice.
I haven't done too much with docstrings, so yours makes more sense. I doubt I'll be putting this up on a website so there won't be much need for the param strings (as those are named as self explanatory names and really shouldnt need documentation), but I'll revise .
As for the lambdas, I should probably do it how you showed if theres only 1 multiline lambda in the hypothetical string of lambdas (and I will make changes to reflect that as it makes more sense to not 4AM me), but when I string multiple lambdas together leaving the curly at the end of the last line (single line and multiline lambdas) rather than a new line makes more sense visually to me.
And if that was in reference to if statements, I only omit curlies when I know that the check will only need 1 output like.
I think this would look a bit weird if the } at the end of filter just had its own line. Personal opinion, though.Code:if (!isValidSlot(fromSlot)) return Failure.BadFromIndex(fromSlot)
Code:fun doThing(onList: List<Any?>) { onList .map { it } .filter { when (it) { is Number -> true is String -> true else -> it is Iterable<*> } } .forEach { println(it) } }
Update: 2019/03/14 - v1.14-1
- Added a lot of documentation to the source code
- Fixed a few logical bugs leading to Exceptions (missed testing a very specific scenario, added a test for that scenario)
- Stripped BaseItem and ItemDefinition to prepare for genericizing
Link to release: https://github.com/nbness2/ItemConta...es/tag/v1.14-1
My next step is to make this as generic as I can. Of course I can't make it COMPLETELY generic, as I have to know what properties to access when adding an item, removing an item, anything having to do with accessing properties. Or maybe I can...
Updated this post to include the new inline code tag, will be slowly updating my more recent relevant posts with it as well!
What's the inline code tag? The inline code tag is a code that is very similar to the code block tag [code]code here[/code], but you can do it in the same line without making a lot of space for potentially a very small amount of text. Here's an example of some inline code.
An inline function to print "Hello inline code!":fun helloInline() = println("Hello inline code!)
Here's how you use inline code tag: [inline]inline code here[/inline] ->inline code here
Update v2.0-0: 2019/05/04
- Generified
Container
- Added
ItemAccessWrapper
. This is required to put inside aContainer
, so theContainer
knows how to access your class!- Generified the applicable
ContainerResult
, the ones that contain aT
- Made
Container
List
-y- Updated main post
ContainerUtil: SOURCE/JAR
Spoiler for example:
keep up the good work
« Previous Thread | Next Thread » |
Thread Information |
Users Browsing this ThreadThere are currently 1 users browsing this thread. (0 members and 1 guests) |
Tags for this Thread |