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.
Top comments (8)
nilin Crystal is used to represent the absence of a value. If a User doesn't exist in the database you should returnnilfor that case. Doing so will force caller of the method to always deal with the possibility (they can't forget to do it, unlike theerrorssuggested alternative).In short:
nilis the way to go.Succinct and authoritative, thank you, sir!
I've been trying to avoid
nilin 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
nilvalues that provides the safety but Nil being a type that the compiler can check for and enforce proper handling of.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.
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.
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
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 | Useris perfectly fine; where the check can happen higher up in the process.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.
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.