Returning type-safe & non-nil failure in Crystal?

Publish Date: Feb 18 '19

9 8

I am having design doubts in Crystal and would like to hear how others have implemented similar functionality.

The User class has a method to fetch data based on some attribute value and return an instance.

class User
  def self.get_by_login(login : String)
    obj = DB.irrelevant_method(login)
    return obj
  end
end

(I am using superfluous return statements to be more illustrative of intent)

The issue comes in when the database does not find a record to return. I do not want to return a union type such as (User|Nil) or (User|Bool). My idea is to return a User instance to satisfy the return type with the specific error added to an existing errors array I'm already using for validation on the creation side.

class User
  def self.get_by_login(login : String)
    user = User.new
    begin
      user = DB.irrelevant_method(login)
    rescue ex
      user.errors << ex.to_s
    end
    return user
  end
end

This leaves it up to the caller to check if the returned object is valid or has any errors before knowing what to do with it.

Any thoughts or concerns about this approach in Crystal or in general? I've used this pattern before in other languages and it does work, but I've never really felt great about it. I'm curious to hear how others have solved this or similar issues in Crystal or other strongly typed languages.

Comments 8 total

Mikael KlagesFeb 18, 2019

One thing to consider could be the security risks of having separate errors, if a developer doesn't know they have to check for errors or forgets, what will happen when an error occurs?

What some languages gain for free with optional/nullable/joint types is a crash or exception when the returned value is treated as a valid value. (Though this might make it less apparent where the error occured)

Others like go force you to explicitly ignore the error if you don't want to care about it.

Then there's those that would throw an exception at error, which would tell you immediately where the error is. Though this might make the code uglier, or make the developer lazy and just do a catch all.

I think some languages with union types force you to program different execution paths for each possible type, but that might be more exclusive to languages with inferred typing.
- MN MarkFeb 18, 2019
  
  That is a good point.
  One thing I didn't point out is: another purpose of returning the empty User object, beyond satisfying the return type, is that it's a blank object that should be safe to use anywhere.
  If you were to fetch an object and ignored the errors and tried to populate the view anyway, you would get blank values as the objects would have to be initialized with valid values such as empty strings, 0, or whatever is appropriate.
  
  I think this addresses the concern, if I've understood you right.
  - Mikael KlagesFeb 19, 2019
    
    Well, it depends on what the user object will be used for. If you only use it for displaying values then it should be fine.
    
    If there's a chance the caller will be comparing a field to another, there may be unexpected behaviour if they don't check the error.
    
    If the user plans on changing the object them storing it somewhere, then they may end up with lots of near-empty fields.
    
    I must admit I haven't tried crystal, and I don't know the extent of what you're making, so I'm just listing issues that may occur with stuff that seems similar :-P
Blacksmoke16Feb 19, 2019

I think it partially depends on the context of the method/application. IMO, I feel you should consider the implication of what should happen if the method returns nil, not just handling it for sake of handling it.

If this was for an API, I would expect that a 404 exception would get returned to the user saying like "hey this user doesn't exist".

However that logic doesn't have to live within this method. Having this method return Nil | User is perfectly fine; where the check can happen higher up in the process.
Doug BryantFeb 19, 2019

If the intent is to always return a user object, perhaps a different method name such as find_or_initialize_by_login would make the intent more clear. get_by_login makes me think a login object or null/error as a result.
- MN MarkFeb 19, 2019
  
  Renaming to something like "find_or_initialize_by_login" would be more accurate and in at least one way is the right answer. Except that it's not the right answer here which helps me see that it's the behavior of the method that is wrong. Thanks for the alternate angle on it.
Ary BorenszweigFeb 19, 2019

nil in Crystal is used to represent the absence of a value. If a User doesn't exist in the database you should return nil for that case. Doing so will force caller of the method to always deal with the possibility (they can't forget to do it, unlike the errors suggested alternative).

In short: nil is the way to go.
- MN MarkFeb 19, 2019
  
  Succinct and authoritative, thank you, sir!
  
  I've been trying to avoid nil in crystal because I felt it was the "safe" thing to do (also I didn't want to have to call .as(...) so much).
  
  I think I am finally understanding that it isn't the absence of nil values that provides the safety but Nil being a type that the compiler can check for and enforce proper handling of.

Add comment

MN Mark @mjb2kmn

Returning type-safe & non-nil failure in Crystal?

Comments 8 total