I read an article about how it is better to return...
# codingconventions
a
I read an article about how it is better to return a
Result
than throw an exception. Is that true in all cases? I've found from my experience in other languages that it's a lot easier to write a REST API with some type of special HTTP status exception that you can throw, which will be handled at the top level and respond appropriately to the client. Should libraries use exceptions? I haven't seen any libraries which return `Result`s.
j
This is very much subject to debate. In Kotlin specifically, the design of
Result
is not meant for railway programming, as stated in the corresponding KEEP. I personally prefer using exceptions in general, and particularly for unexpected failures, with an occasional custom result types in specific cases where it makes sense to expect some type of errors. But I'd still say it's a personal preference. Do you have the link to the article by any chance? I'd be interested.
a
i was also reading this after i posted this question https://thekotlindev.com/2021/kotlin-result-pattern/
my understanding was that this was accepted practice. I haven't read the associated KEEP though. this seems to contradict as well with using contracts (
require
, etc.) for validation, since those throw exceptions as well
i have been using them in my library, but the article was in the back of my head while doing it. it often doesn't make sense to return a
Result
type. and i think it's fine to throw exceptions in situations where things haven't gone horribly wrong.
j
Wow, I glanced over the article and I really wouldn't advise using
runCatching
in general application code, though. You can see my answer here about that: https://stackoverflow.com/a/70847760/1540818
a
that's good to know as well, thanks!
^makes the case against exception throwing but leaves some leeway when they will be caught by a global handler which is what it sounds like you want to do with the rest api
j
This article really doesn't make the case against exception throwing in general, actually. It makes the case against checked exceptions for sure, and then carries on explaining when exactly it's recommended to use exceptions vs expressing errors in the type system. This article is pretty much describing my view on the topic, by the way. Thanks for the link!
And in the case of I/O, which was your initial example, the article suggests exceptions, yes
c
Personally I use a custom
Result
type, which I can control. I don’t use
kotlin.Result
. It’s similar but in particular the
Failure
is more tailored to my application’s needs. If I recall correctly, Roman also makes the point that you may not want to pollute low-level code with results. The result idiom is perhaps more application- or business-centric. Since you mentioned
require
, it’s worth pointing out that this checks invariants in your system. The reason you want this to throw is that, if
require
fails, it means your system is in an invalid state and it may be dangerous to continue (e.g. by causing data corruption). This is different from expected “errors” like: • a resource could not be found • user is not authorized • a hardware interface is unavailable • etc To be clear I’m not trying to promote exceptions vs results here. I tend to only use exceptions when programming against an external API (e.g. OkHttp Interceptor), and results elsewhere.
a
Really?
require
throws
IllegalArgumentException
. I thought
check
(
IllegalStateException
) describes more what you're talking about with regards to system invariants
j
The properties of your function arguments are just another kind of invariant technically, unless they are at a library's API boundary
c
Yeah I would consider those more or less equivalent in terms of “we don’t want to progress further”
a
Ok, this is very interesting. So, one place where I used them in this library I'm working on is in a constructor, which can only accept numbers between zero and some upper bound. What would be better to use there? I can't return
Result
obviously, but should I use a custom
Exception
type?
c
require(arg in lowerBound..upperBound) { "Expected arg to be in range ... but found $arg" }
a
yeah that's basically what i have
j
Yeah I think
require
is good in a constructor, if you're checking arguments of course
c
(edited to add a lazy message)
a
This is good to know. I think maybe I don't have the correct understanding of what "Invariant" means. At a previous job, we had an error macro
invariant
which essentially meant "crash now because something is very wrong"
c
Yes that sounds exactly right
j
My personal preference would be to let the constructor succeed and add
checks
to any functions that need the properties to be valid. Making constructors too smart can get painful.
j
An invariant is an assumption that should always be true if the code has no bugs. This is why technically checking an invariant shouldn't be necessary in working code, but we do it to help finding bugs.
@Jacob I would argue the opposite, a lot of things would be much better if wrong objects were not constructed in the first place. It's better to crash ASAP when something is wrong. If you want to support a degraded behaviour, it's another story though
a
Sorry to add more caveats
This is why technically checking an invariant shouldn't be necessary in working code, but we do it to help finding bugs.
Agreed! My constructor can consume "user" data though (it's a library for RSS). If it gets a bad RSS feed, would that count as an invariant? The program using the library could obviously continue on, it would just have to reject the feed
c
Agreed with Joffrey here. If you get an invalid input, terminate the program.
What’s a bad rss feed? Like an invalid url?
As in, the url points to a resource which doesn’t exist? Or the url is malformed (can’t be parsed due to bad syntax)
a
The class I'm referring to captures the
skipHours
tag, which describes which hours the feed cannot be refreshed. So if they input an hour that is not a valid hour of the day (between 0 and 23), then it is invalid
j
I think it's fine to consider it as an invariant of the whole system app+library. This helps finding bugs too, not in your code but in your user's code
Technically in this case the user didn't respect the documentation's contract, and it's an illegal argument, so in any case IMO throwing IAE is the correct thing to do
c
Ah ok interesting. Yeah so this occurs at a boundary of your program right? i.e. you are getting data from an external feed.
This kind of problem, “external data is not spec-compliant” is common and yes I think it’s fine to throw a custom exception here. Up to you
j
If the argument is provided by some data from the external feed, it's something different. But maybe it should be checked in a different place then to do some error handling, so not handling errors earlier could still be considered a bug. Such case is indeed more subject to debate 🙂
c
But this is not an exception that should terminate your program. You need to catch it and surface information to the user saying “This feed has a problem, etc, contact the author”
a
Technically in this case the user didn't respect the documentation's contract, and it's an illegal argument, so in any case IMO throwing IAE is the correct thing to do
Good point!
Ah ok interesting. Yeah so this occurs at a boundary of your program right? i.e. you are getting data from an external feed.
It depends on what the programmer wants to use the library for. I've broken it up into 3 modules: `core`: Contains basic data classes for serialization/de-serialization of RSS feeds into an object `client`: Utilities for consuming RSS `server`: Utilities for producing RSS it's meant to run alongside ktor, but in theory it doesn't have to
The class in question is in
core
. It is sounding like you both agree that a custom exception is a better choice here
c
I work on video streaming, and I used to work with DASH and HLS extensively. We would frequently consume external video streams. DASH especially is a complicated spec, and beyond the spec there is a “best practices for authoring” document. So as you can image people often got this wrong. And if you’re working with a spec-complicant video player, it can be hard to navigate exactly what the right solution is to invalid content. The content may be some legacy content that is still in demand but which the vendor doesn’t want to (or can’t) update. Also the vendor will tell you “but it works with this other video player”. At that point it becomes a business decision whether you want to support non-spec-compliant content. Often you can do it, but you need to be very explicit in your code about the fact that you are working around invalid content. It’s best if this is encapsulated somewhere (behind an interface).
a
That sounds terrible :( I had to deal with similar we-don't-want-to-support-this-but-we-have-to situations at an old job, so I sympathize greatly. Thankfully, the library I'm making is just a fun open source project I want to release and maintain to the public for free. I am looking to comply with spec. I just wanted to know what would be the most effective way for me to handle errors which don't comply with spec. I think
Exception
is definitely the move, but I was using
require
for that after reading the article I linked above
But it seems something custom might get the message across better
c
I think it makes sense to use a custom exception here, yeah. It’s easy to catch and triage. I wouldn’t use
require
for this, because you’re evaluating input at the boundary of your program. Some input is bound to be invalid (consider e.g. user input from a keyboard or whatever). Good luck with your library!
a
thank you :)
k
might help

https://www.youtube.com/watch?v=sSQAo_o40Es&list=PL1ssMPpyqochiZj41oLAtvht4ScUurHJH&index=4

g
My constructor can consume "user" data though (it's a library for RSS). If it gets a bad RSS feed, would that count as an invariant?
Yes i would count it as invariant/logic-error since it depends on user input. Whether user needs special knowledge to provide the proper input or not. i would recommend using IAE, but it always depends on the case. If your exception should contain info more than a simple message ("input: $field is not compliant with spec #1234"), then i would recommend building a custom exception which extends IAE where the consumer can still catch your exception in the same top-level handler. Speculation: I think
kotlinx.serialization
follows similar approach.
198 Views