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

Model to schema naming conventions #76

Closed
aarongodin opened this issue Oct 18, 2017 · 8 comments
Closed

Model to schema naming conventions #76

aarongodin opened this issue Oct 18, 2017 · 8 comments

Comments

@aarongodin
Copy link

Coming from ActiveRecord, the Model name to table name convention is great to see. I did notice that there is one difference, and I wanted to bring it up as it was the only difference I saw between Granite's and ActiveRecord's. It can be seen in this error:

icr(0.23.1) > SongThread.all
relation "songthreads" does not exist (PQ::PQError)

The Rails Guides outline this convention for ActiveRecord:

Database Table - Plural with underscores separating words (e.g., book_clubs).
Model Class - Singular with the first letter of each word capitalized (e.g., BookClub).

In this case, you'd expect the schema relation to be called song_threads.

Here's my SongThread class for completeness to the example:

require "granite_orm/adapter/pg"

class SongThread < Granite::ORM::Base
  adapter pg

  field bpm : Int32
  field title : String
  field primary_account : Int32
  field secondary_account : Int32

  timestamps

  def primary_account
    Account.find @primary_account
  end

  def secondary_account
    Account.find @secondary_account
  end
end
@drujensen
Copy link
Member

I agree, we should change this.

We should also look at adding a pluralize helper since adding an s is not ideal. we should just add a couple rules like if the word ends with an s, ch, sh then add es. However, we shouldn't go overboard on this and add a custom dictionary of words. It should be a straightforward predictable algorithm.

@marksiemers
Copy link

Regarding the snake case table names - I think that fix is easy:
Here: https://github.com/amberframework/granite-orm/blob/master/src/granite_orm/table.cr#L30

# Currently
{% name_space = @type.name.gsub(/::/, "_").downcase.id %}

# Change to
{% name_space = @type.name.gsub(/::/, "_").underscore.id %}

Regarding dealing with pluralization, I think leveraging an existing library is a good idea: https://github.com/mosop/string_inflection

@robacarp
Copy link
Member

I’d second pulling in a proper inflector. There are undoubtedly times where pedantic inflection is mote confusing to the majority of people (eg Media, Data), but more often than not I appreciated the ease of Rails inflection. In other projects I’ve attempted to live with a cheaper solution and regretted it.

@drujensen drujensen changed the title Model to schema naming conventions Model to schema naming conventions- support better pluralization Nov 26, 2017
@eliasjpr
Copy link
Contributor

eliasjpr commented Jan 5, 2018

There is a nice inflector created here https://github.com/luckyframework/lucky_inflector

@robacarp
Copy link
Member

robacarp commented Jan 6, 2018

@eliasjpr As it stands with Granite, a fair amount of inflection assumptions are made in macros. I think relationships will need a major rewrite out of macro land before any real inflection library can be used. In the interim, I think fully configurable associations such as #103 is a workable solution.

@faustinoaq
Copy link
Contributor

faustinoaq commented Jan 7, 2018

Also crecto has a nice query syntax 👉 https://github.com/Crecto/crecto#query-syntax Unrelated comment 😅

@eliasjpr
Copy link
Contributor

@robacarp I agree with you on #103

@drujensen drujensen changed the title Model to schema naming conventions- support better pluralization Model to schema naming conventions Jan 27, 2018
@drujensen
Copy link
Member

Opening a new ticket #129 regarding the inflector

This issue was closed.
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

6 participants