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

Scaffold-DbContext: Flag to minimally alter column/table names #5947

Closed
toddtsic opened this issue Jul 1, 2016 · 32 comments
Closed

Scaffold-DbContext: Flag to minimally alter column/table names #5947

toddtsic opened this issue Jul 1, 2016 · 32 comments

Comments

@toddtsic
Copy link

toddtsic commented Jul 1, 2016

Steps to reproduce

Scaffold-DbContext "Server=.\ss2016;Database=tsiccore;Trusted_Connection=True;" -o "Models/DatabaseModels" Microsoft.EntityFrameworkCore.SqlServer

The issue

Database Table named for example: Job_Menus results in:
public partial class JobMenus
{

Column Name for ex: menu_type_id results in:
public int MenuTypeId { get; set; }

Exception message:

### Further technical details

I'm porting pre RC1 project with thousands of references to context objects (with lower case leading characters and with underscores). 

Is there a command line option to allow objects to be named as they are in the SQL Server db?

Thanks,
Todd
@natemcmaster
Copy link
Contributor

natemcmaster commented Jul 1, 2016

Customizing scaffolding output is not currently supported. We are tracking this improvement idea on our backlog: #4038 cc @lajones

@toddtsic
Copy link
Author

toddtsic commented Jul 1, 2016

Ironic, I'm asking for the default behavior to NOT customize (CamelCase).

There is no way I can go through thousands of instances of object references (original objects created by prior version of rev-eng which did not force CamelCase.)

Fingers crossed that team will take this on, its a deal-breaker...

@JuanIrigoyen
Copy link

I completely agree with toddtsic, I think CamelCase should be an option, this version prevents me from applying EF 7 in many projects with hundreds of entities and thousands of fields using underscores

@lajones
Copy link
Contributor

lajones commented Jul 2, 2016

This was a design decision - see #3987.

Unfortunately, it is not overrideable at the moment. We are aware that no choice we make will suit all customers and so the lack of configurability is an issue we should fix. As @natemcmaster mentioned above that issue is tracked with #4038.

BTW - just for precision - internally we tend to talk about camelCase and PacalCase - the difference being that PascalCase has an initial capital letter and camelCase has an initial lowercase letter. I'm aware that there isn't even public agreement on those definitions, but just so we can distinguish them, we refer to what we currently do as PascalCase.

@natemcmaster
Copy link
Contributor

natemcmaster commented Jul 2, 2016

@JuanIrigoyen

I think CamelCase should be an option

PascalCase is the default. I believe what @toddtsic is asking is to allow snake_case as the default.

@toddtsic

Ironic, I'm asking for the default behavior to NOT customize (CamelCase).

PascalCase was a deliberate design choice. See #3987 (comment).

Fingers crossed that team will take this on, its a deal-breaker...

Would hate to lose you over this.

Unfortunately, it is not overrideable at the moment

...techincally, not easily overridable. But it can be done. It requires using ".Internal" APIs (be warned: this means we do not support the API and in may be removed or break), but this sample shows you how to customize the naming of identifiers (fields and class names) in preview2. https://gist.github.com/natemcmaster/353f4ab3efb4514eeaec846df28f0e24

@JuanIrigoyen
Copy link

@natemcmaster, Thank you very much for this contribution, I think this should cause a Pull Request
To adopt EF7 in projects already developed or set your own rules of design is very important to select the type of convention in the tables and fields entities of your model.

For example:

Scaffold-DbContext "Server=.\ss2016;Database=tsiccore;Trusted_Connection=True;" -o "Models/DatabaseModels" Microsoft.EntityFrameworkCore.SqlServer -e = 'CamelCase' –f = ‘CamelCase’

-e |--entityConventionName Configure the convention name of entity class. If omitted, the output code will will use only the same of database (‘CamelCase’, ‘SnakeCase’...)

-f |–-fieldConventionName Configure the convention name of field of entity class. If omitted, the output code will use only the same of database. (‘CamelCase’, ‘SnakeCase’...)

@ErikEJ
Copy link
Contributor

ErikEJ commented Jul 2, 2016

@sjh37 !

@recalde
Copy link

recalde commented Jul 2, 2016

@natemcmaster that gist is just what I needed to keep scaffolding for a large project I've been working on since beta7->beta8->rc1->rc2, thank you!

@sjh37
Copy link

sjh37 commented Jul 4, 2016

You could always use my reverse generator, which supports the PascalCase option, plus many others, to allow you to customise the generated output how you wish:
visual studio gallery

EF 6.x is only supported currently.
Pluralsight course on it here.

@JuanIrigoyen
Copy link

sjh37, thank you, I know this program, I can not use EF 6 because of performance problems with databases of many tables.

@ErikEJ
Copy link
Contributor

ErikEJ commented Jul 4, 2016

@JuanIrigoyen Just use many DbContext's with a small number of tables, instead of puuting all tables in the same DbContext - @sjh37 's Pluralsight course clearly demonstrates the benefit of this approach!

@sjh37
Copy link

sjh37 commented Jul 4, 2016

@JuanIrigoyen What @ErikEJ said.

@JuanIrigoyen
Copy link

JuanIrigoyen commented Jul 4, 2016

I'm sorry, I can not do it. I already have 8 different contexts in my project, if I divide one more parts, force me to restructure my project because my tables have relationships with each other.

My programs now work in EF 7 and have generated my entities without Camel Case with solution @natemcmaster proposed in https://gist.github.com/natemcmaster/353f4ab3efb4514eeaec846df28f0e24,
I understand the approach of dividing the context and its benefits, but do not want to have to change the project just for this.

My test performance are very good with EF7.

I think that EF7 allow entities generate entities with a specific convention could be a good option very useful in projects already developed.

Thank you very much for your help.

@rowanmiller
Copy link
Contributor

Per code listing shared by @natemcmaster we do have the ability to change the naming algorithm.

We do think a command parameter to switch off naming logic would be good though - opened #6018 to track this.

@orloffm
Copy link

orloffm commented Aug 22, 2016

Well, this approach does not allow to de-pluralize the existing table names. For example, if I have a JobMenus table, it is not possible to have a property JobMenus backed by a collection of JobMenu class instances.

@ajcvickers
Copy link
Member

@orloffm Pluralization is being tracked by issue #3060

@codearoo
Copy link

are you kidding me??? The release version did the perfect thing: NO CHANGE FROM DATABASE TABLE NAMES TO CLASS NAMES. Why can't the default go BACK to what it was? Making Camel or Pascal alterations and removing underscores or whatever should be options, but not the defaults, and certainly very absurd to CHANGE the default from RC1 or RC2. Just stalls adoption.

@codearoo
Copy link

I don't seem to be able to find the code for Scaffold-Dbcontext to change this behavior while waiting for Microsoft to do it. Any ideas where it is?

@rowanmiller
Copy link
Contributor

It's not a change that we are planning to take in the product (that would be a much bigger decision). But here is some code that demonstrates how to replace the default name generator with your own (including one that doesn't change the name) https://gist.github.com/natemcmaster/353f4ab3efb4514eeaec846df28f0e24.

@codearoo
Copy link

Thanks for that Gist link. That's what I did for now to allow me to more gradually change table class names to how I want them.

I still don't get how it isn't a first thought to allow the app to "do less" rather than forcing some additional behavior that is just some forced customized option. It's nice, but only if you and when you want it.

@divega
Copy link
Contributor

divega commented Sep 27, 2016

@lajones @rowanmiller do you remember if we have an issue tracking a command line option to disable creating C#-like identifiers? I think we have had significant feedback asking for this.

@lajones
Copy link
Contributor

lajones commented Sep 27, 2016

Obviously we have to create identifiers which are valid C#. So am I right in thinking that by "C#-like identifiers" above you mean pascal-cased ones? If so, yes, that was done in response to customers who didn't like the original plan we had - which was that the C# identifier looked as much as possible like the database identifier but obeyed C# identifier rules. See #3987 and #5360.

@codearoo
Copy link

If there is a situation where copying the exact name would not work, I can only imagine that being a field with spaces. In which case, the typical convention would be to replace the spaces with a "_". That would be an expected behavior if someone told the script to "not mess with my names", possibly with some warning output. Not saying it's bad to use Pascal case or whatever, but leave people the choice. :) No thrilled about changing default behaviors, but if you do, then at least give a way out.

@codearoo
Copy link

Another way to think about it is that, had you included an option to the user to create the names in the different ways you coded, we wouldn't be wasting time with this discussion. :)

@divega
Copy link
Contributor

divega commented Sep 27, 2016

@lajones I am using "C#-like identifiers" as a shortcut to refer to identifiers that look as much as possible to what you would use in a C# program, i.e. not only make them valid C# identifiers but also follow common conventions such as "PascalCase".

Before you did that work to produce "C#-like identifiers" we had a short discussion about having a simple command line flag to opt-out of the behavior, but we decided against it in the first release. So the question is if we have an issue in the backlog or elsewhere to add that flag. I have searched but I cannot find it.

Obviously we have to create identifiers which are valid C#.

FWIW, we can decide on this later, but I am not sure it is true. I can see us stopping processing the identifiers altogether when users opt-out. That means if they use table or column names that are not valid C# identifiers they would need to fix the code themselves.

@lajones
Copy link
Contributor

lajones commented Sep 27, 2016

@divega No - no extra bug that I am aware of - we did discuss that flag but decided to wait for @rowanmiller 's input - which resulted in the rules at comment #3987 (comment). Then we re-designed that later with your input.

Note: there is one other thing which the code does (other than "pascalize" the identifiers) and that is to ensure that none of the identifiers are already in use. If we skipped that step and there was a clash then it could be hard for users to figure out which one was meant to be which - but we can't do that step until we have the proposed identifier including any "pascalization" which might have been done. So, yes, we'd definitely need to discuss if you wanted to change that.

@divega
Copy link
Contributor

divega commented Sep 27, 2016

@lajones thanks for the information. Reactivating this issue to track/discuss a simple out-flag.

@divega divega reopened this Sep 27, 2016
@codearoo
Copy link

So in the same philosophy, but not exactly the same item.. it would be nice if the Scaffolding scripts would not care if it can't find a primary key or even work with a view. If you just remove the requirement, you'd allow people to gain from the automation that it does do. It could just spit out warnings in the code or something, showing the developer that they can't do certain things, or they have to fix them but better than ended up with nothing output.

And in order for me to use a VIEW, I had to temporarily create a TABLE of the view I wanted, just so the scaffolding could find it.. (on and I have to set a primary key too) and then I manually changed the resulting code to just use the VIEW (which obviously has no "primary key" cause it's just created by SQL) and it works great. Of course I'm not going to write methods to update anything in here. I don't even need to keep the [Key] attribute for my purpose which is just to get the output from this view.

@divega
Copy link
Contributor

divega commented Sep 29, 2016

@codearoo At first glance I see more than one potential feature requests in your comment, e.g. having a way to take views into consideration on scaffolding, scaffold classes with warnings/to-do comments when we can't identify keys, support read-only entities without keys, etc. Definitively it would be a good idea to try to articulate this in its own issue or set of issues once you have checked they are not tracked elsewhere. As a comment at the tail of this thread it will likely get lost.

@codearoo
Copy link

Will do.

@rowanmiller rowanmiller changed the title Scaffold-DbContext: Forcing CamelCase, possible to override? Scaffold-DbContext: Flag to minimally alter column/table names Oct 3, 2016
@rowanmiller rowanmiller added this to the 1.2.0 milestone Oct 3, 2016
@farlop
Copy link

farlop commented Dec 14, 2016

We have custom library that generates raw SQLs (for internal use) depending on the name of the properties in the POCO classes. So, we need the behavior to be consistent with EF6 and respectful with the database names. Really don't mind if it's default behavior or trough a parameter, as long as it's possible.

@ajcvickers
Copy link
Member

Only thing we decided to do here is add a flag to disable name-changing logic, which is covered by #6018, so closing this as a duplicate,

@ajcvickers ajcvickers removed this from the 2.0.0 milestone Apr 17, 2017
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests