#Seeking feedback to write better Gleam

1 messages · Page 1 of 1 (latest)

safe jungle
#

Hello, I'm back asking for more feedback on my code. I have attached a file with a working example (I was going to use Gleam Playground but the link itself exceeds the post length 😯)

I have been hacking away at my project for --checks commit logs --almost two months(!?) and, while I am making progress, I feel like I'm still writing bad code. So, to prepare something for feedback, I've pulled aside the current problem I'm working on into its own module. This code exemplifies working with formdata with wisp. It's not 1:1 with my project code but the point is to show how I'm writing my code and solving problems with Gleam.

I'll take any and all feedback, but some questions I have in mind are:

  • Are there places I should be using use that I'm missing?
  • Am I using gleam/result functions efficiently and properly?
  • Can I make my code more legible.

The actual project can be found here, for anyone interested: https://git.sr.ht/~nebunez/leafcutter_webclient

Thanks in advance!

brazen osprey
#

hi! can't spot any super obvious bad practices in your code!

what you can do is replace some of the result.unwrapping and try logic with simple case statements. I personally find they make the code more legible. f.e.:

fn handle_deactivate(cab_id_str: String, profile: Profile) -> Profile {
  case int.parse(cab_id_str) {
    Error(_) -> profile
    Ok(cab_id) ->
      Profile(
        ..profile,
        cabinet_configurations: list.drop_while(
          profile.cabinet_configurations,
          fn(cab_config) { cab_config.cabinet_id == cab_id },
        ),
      )
  }
}

same in the handle_cabinet_configuration_update function

onyx vessel
#

I would probably introduce a custom error type like

type ProfileUpdateError {
  InvalidCabinetId(String)
  MissingCabinetId(Int)
  InvalidFieldName(String)
}

and then do result.replace_error in a few places to use that error instead of Nil as the error

#

here's a version with that and some other changes that clean things up a bit

safe jungle
#

Thanks for the feedback, both of you. I'm taking another pass to address these edits and recommendations. Just kinda burnt out today on this project so might take a day or two to get the juices going again.

safe jungle
#

@onyx vessel I like that you used Dict for the cabinet_configurations lookup. I actually had this thought, too. However, The Profile Record matches the db schema, which holds the cabinet_configurations as a list in a one-to-many relationship. Let's just say that can't change (maybe it can but that's not an easy option right now).

Given that constraint, would you perform the lookups on the list as I have it, or would you implement data transformations between the database representiation (List) to an easier to lookup form (Dict)?

I've worked with DAO patterns in C# that provide transformed objects of the database records, but I am not sure if that's the best thing to do in Gleam.

safe jungle
#

Unfortunately I just don't have time to take another pass at this example; I have to move right into the implementation into my project. Just FYI in case anyone was waiting on an update from me here.