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

Add UseSnakeCaseNamingConvention extension method #259

Closed
wants to merge 2 commits into from

Conversation

khellang
Copy link
Contributor

@khellang khellang commented Nov 1, 2017

As mentioned in npgsql/npgsql#1690.

I wanted to just open this up early 😄

Had to update to the 3.0.0 CI package. I'll strip off that commit once a stable 3.0.0 is out.

I'll see if I can add some tests as well. Do you have any pointers on how you'd like them written?

@roji
Copy link
Member

roji commented Nov 2, 2017

Hey again @khellang!

There's nothing in this PR which is actually Npgsql-specific - it's just generic EF Core code which which modifies table/column names on the model, and could be used with SQL Server/Sqlite just as well (it just happens to use a public function exposed by Npgsql). So I'd see this more as a completely separate nuget (e.g. EntityFrameworkCore.SnakeCaseNaming?), rather than something done inside the Npgsql provider.

Note that EF Core supports a concept of conventions (unlike your extension method which happens to be named "Convention"), which may be a more appropriate way to implement what you want. You may want to look into that.

@khellang
Copy link
Contributor Author

khellang commented Nov 2, 2017

There's nothing in this PR which is actually Npgsql-specific - it's just generic EF Core code which which modifies table/column names on the model, and could be used with SQL Server/Sqlite just as well

Yes, I realize that, but since it's really common (idiomatic, even?) to use snake_case naming with Postgres, and the default PascalCase naming convention requires quotes everywhere, I figured it would be a nice addition to this package. It would be even better if EF supported different naming schemes out of the box (at which point, this method could be deprecated and delegate to the EF version).

it just happens to use a public function exposed by Npgsql

And that's not a coincidence, right? It's really handy for the reasons mentioned above.

So I'd see this more as a completely separate nuget (e.g. EntityFrameworkCore.SnakeCaseNaming?), rather than something done inside the Npgsql provider.

Installing a totally separate package and assembly just to get a single method to change the naming convention to snake_case sounds a bit overkill. At that point I'd rather just copy the code.

Note that EF Core supports a concept of conventions (unlike your extension method which happens to be named "Convention"), which may be a more appropriate way to implement what you want. You may want to look into that.

I don't think that's the case for naming, but I'd be happy to be proven wrong, though. See

dotnet/efcore#214
dotnet/efcore#4445
dotnet/efcore#5159
https://stackoverflow.com/questions/37493095/entity-framework-core-rc2-table-name-pluralization
https://stackoverflow.com/questions/34700281/entity-framework-7-pluralize-table-names-with-code-first-approach

These are just from a quick Google search, and all the official responses suggest this is the way to customize the names.

Also related: #21

@roji
Copy link
Member

roji commented Nov 2, 2017

I definitely agree it would be good if EF supported pluggable naming schemes, and this has been requested several times (e.g. dotnet/efcore#5159). Even if snake case is common (but not universal) in PostgreSQL, it is also common in other databases.

So basically my main objection about including this in Npgsql is that this kind of thing either belongs in EF Core itself, or externally. Why not try submitting a PR to EF Core? If you show the complexity of correctly doing snakecase (including all the acronym work etc.), and argue that it's a shame to have to duplicate this code for every user, I'm hoping they may accept it (even if they might ask for a slightly different implementation).

So I'd see this more as a completely separate nuget (e.g. EntityFrameworkCore.SnakeCaseNaming?), rather than something done inside the Npgsql provider.

Installing a totally separate package and assembly just to get a single method to change the naming convention to snake_case sounds a bit overkill. At that point I'd rather just copy the code.

You're absolutely right that a nuget package with an assembly just for this one extension method is very heavy - but there's also the option of simply adding the a source file to your "content" dir. This removes the additional assembly, and is just a way to drop a source file (containing your extension) into the consumer's project. I still thing it's better for this to be in EF Core, but if not there's always this option.

@khellang khellang closed this Nov 2, 2017
@khellang khellang deleted the snake-case-extension branch November 2, 2017 16:01
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

Successfully merging this pull request may close these issues.

2 participants