Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a Trait to organize functions #1

Open
dessalines opened this issue Oct 3, 2022 · 6 comments
Open

Use a Trait to organize functions #1

dessalines opened this issue Oct 3, 2022 · 6 comments

Comments

@dessalines
Copy link

Hey, one of the creators of lemmy here, this looks like a really interesting project that could help us. We have a lot of tables, and generating a lot of the simpler Crud-type functions in code would make this really useful.

Instead of pub struct UpdateTodo and exporting that, I highly recommend creating a trait. Here's what we do in our code:

pub trait Crud {
  type Form;
  type IdType; // We use diesel-newtypes to make sure all the Ids are unit structs, to make sure people aren't using a `PostId` when they should be using a `CommentId` for example... using `i32` can be dangerous.
  fn create(conn: &mut Connection, form: &Self::Form) -> Result<Self, Error>
    where Self: Sized;
  fn read(conn: &mut Connection, id: Self::IdType) -> Result<Self, Error> 
  ...

Then your generated code would be:

impl Crud for XXX {
  type Form = XXXForm;
  fn read(...

And people could easily import Crud to get all the functions.

@Wulf
Copy link
Owner

Wulf commented Oct 4, 2022

Ah, neat! I like that you're using PostId instead of i32. I wanted to add column type overrides in the generation config for cases like this.

Could I ask why you prefer traits though? If it's just a preference, I wouldn't mind if dsync had both options!

@dessalines
Copy link
Author

Couple reasons:

  • All of these generated tables have the same distinct set of functions, so it makes sense to do a trait for them.
  • You only have to import the trait to be able access all the functions, rather than having a ton of imports.
  • Possibly have some default functions / shared behavior.

@Wulf
Copy link
Owner

Wulf commented Oct 25, 2022

hey @dessalines, sorry I dropped the ball here -- it's been a busy few weeks. Do you mind creating a PR for this? I think trait-based generation is a good idea moving forward :)

@dessalines
Copy link
Author

dessalines commented Oct 25, 2022

I pry wouldn't have time for it, with my work on lemmy unfortunately. but somebody else could also take a stab at it.

@AnthonyMichaelTDM
Copy link
Contributor

AnthonyMichaelTDM commented Nov 29, 2022

The issue I see with trying this is that a trait can't cover instances where there are multiple primary keys

at least not without some major refactoring of the codebase (passing a collection (that can have an arbitrary number of elements each with different data types, which I'm not sure exists for rust) instead of having a parameter per primary key)

@AnthonyMichaelTDM
Copy link
Contributor

on second thought, using macros could work as macros can be variadic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants