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

Identifier generation strategy in reverse engineering #3987

Closed
roji opened this issue Dec 6, 2015 · 12 comments
Closed

Identifier generation strategy in reverse engineering #3987

roji opened this issue Dec 6, 2015 · 12 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@roji
Copy link
Member

roji commented Dec 6, 2015

I can see logic in CSharpUtilities that's responsible for generating a C# identifier out of a database column. In PostgreSQL it seems that the standard case is "snake case" (employee_name). It seems right for the reverse engineering process to convert this to camel case (EmployeeName).

So I was wondering if this is something that Npgsql should do in its own version of CSharpUtilities, or whether the universal CSharpUtilities should detect snake case and perform this task (I'm guessing there are instances where SqlServer or other databases contain snake case).

@natemcmaster
Copy link
Contributor

This is a good question. The identifier generation needs to be revisited.

Currently RelationalScaffoldingModelFactory.GetEntityName handles generating the names. This can be overridden. This does 2 things: (1) ensures unique names for each entity type and (2) ensures these identifiers are CSharp-safe.

Future we will move (2) the csharpification of identifiers into the code-gen step (i.e. RelationalScaffoldingModelFactory will no longer need to return CSharp-safe identifiers.) We haven't designed code-gen API yet.

@rowanmiller
Copy link
Contributor

Triage: We should just handle snake case in our CSharpUtilities. @roji do you want to send a PR for this?

@roji
Copy link
Member Author

roji commented Dec 9, 2015

@rowanmiller sure I'll submit a PR soon.

@roji
Copy link
Member Author

roji commented Dec 9, 2015

Note this probably means that single uncapitalized words are uppercased. If so there's no more need to detect C# reserved words and prefix them with underscore, since volatile becomes Volatile anyway

@divega
Copy link
Contributor

divega commented Dec 9, 2015

Yay! Then at some point we'll have to start looking at the VB keywords instead! 😏

@natemcmaster
Copy link
Contributor

Yup, this was one of the intentions of my comments above. IScaffoldingModelFactory.Create shouldn't be concerned with the targeted code-gen language.

@rowanmiller
Copy link
Contributor

@lajones regardless of extension points, we should update EF to handle snake case

@lajones
Copy link
Contributor

lajones commented May 17, 2016

Fix checked in with a065b93.

@lajones lajones closed this as completed May 17, 2016
@lajones lajones reopened this May 17, 2016
@lajones
Copy link
Contributor

lajones commented May 17, 2016

See comments in #5360. Need to add " " as a word separator.

@lajones
Copy link
Contributor

lajones commented May 18, 2016

Blocked pending discussion between @divega, @rowanmiller and me to decide final design.

@lajones
Copy link
Contributor

lajones commented May 24, 2016

Had discussion on 5/24/16 with @rowanmiller, @divega, @ajcvickers and @bricelam. Approach will be:

  1. Split DB identifier into words. Word separation happens a) if there is any non-letter & non-digit character, or b) if there is a change of case from lower to upper (where intervening digit characters don't change the change of case i.e. Blog123ID results in 2 words: "Blog123" and "ID")
  2. If the first letter of the word is a letter then upper-case it. Lower-case all other letters in the word.
  3. Put the words back together to form the candidate identifier.

The candidate identifier will then be used to produce the final identifier by a) making sure it's a valid identifier for the output language (currently C#) and b) making sure it doesn't clash with existing identifiers as per normal.

@lajones
Copy link
Contributor

lajones commented May 26, 2016

Fix (as designed above) checked in with f2f90ca.

@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests

6 participants