justinian/dice

add to RollResult interface: Int() int

frenata opened this issue · 4 comments

I'm not particular about the name of said function, but I tried to find a simple way to retrieve 'just the total' from a StdResult and saw that there was an exported function Total() int, which since it's not a part of the interface requires the client to perform a type assertion along the lines of:

result, _, err := Roll("3d8")
if err!= nil { //handle it }
total := result.(StdResult).Total()

This isn't terrible, but for the purposes of when a client really just wants the simple integer result of a roll, it'd be nice to just call result.Int(). The main problem, though, is that while StdResult.Total() and VsResult.Successes() make sense as the 'simple integer' returns from a Result, I'm clueless as to whether that question even makes sense when it comes to EotE, a system I'm not familiar with.

If there's simply no reasonable simple return from EotE or this is an undesired addition, that's fine; otherwise I'm happy to make the necessary changes and send a PR if given some direction as to what kind of return that implementation should provide.

Hmm. This is a tough question. Originally, (and mostly because EotE was a requirement from the start for me) RollResult was just a fmt.Stringer because it was supposed to be opaque, something that you could only print unless you knew the type (which of course you shouldn't from outside the package and maybe tests).

Now that they're all RollResult, it does make sense to be more inspectable for details - and I think EotE is the only system I can think of where a straight up integer doesn't really make sense.

I think I'm fine with EotE being a weird edge case, since it's just a strange rolling system. (I mean, custom dice? Really?) - I guess just return the success/failure axis as an integer and ignore the others in that case? Most tools probably will also at least have the option to look at the string version of the result, so that seems good enough.

So in EotE's case: just return EoteResult.S?

I'll be happy to work on adding it, in that case, since you're amenable to the change. I realize it was a tough question--it's doesn't quite make sense for EotE and the information is gettable, but it feels awkward to ask users of the library to do a typecast to retrieve the integer.

Interesting note about the history...are you using the dice package elsewhere, or only in the /cmd/roll binary? Obviously these concerns are irrelevant for anything that just prints out the details. I'm looking at using it as a way to conveniently store a dice formula, then resolve it into a final number as needed, within the context of a wider application.

Yup, EoteResult.S would be best.

I'd be happy add that myself, but you'll probably get to it before I do. Definitely awkward to require typecasting - actually, once the Int() method is on the interface, it would probably even make sense to not export these types, and let them be opaque to the caller. Then again, maybe the caller wants to call the EotE function directly sometimes. Not sure which is better.

As for history, yeah, this library is actually a re-implementation of a Python version I wrote a while back. I re-wrote it as a go library for my Slack dice bot. The original Python version was part of my GPG-verified dice email bot. I look forward to seeing what you do with it - better non-Stringer programmatic access is definitely a good idea for tools like that.

Fixed by PR #7