eaigner / hood

Database agnostic ORM for Go
MIT License
710 stars 52 forks source link

Another propose about index definition. #41

Closed coocood closed 11 years ago

coocood commented 11 years ago

How about we define a interface for a table then define the table property in method?

like

type Table interface{
    Properties() map[string]interface{}
} 
eaigner commented 11 years ago

I like it. I like it a lot! Don''t know why this didn't occur sooner to me.

coocood commented 11 years ago

And this can also resolve the plural table name issue.

eaigner commented 11 years ago

But I would not specify a single interface with a map property, but rather an interface for each property, e.g.

type Indexed interface {
    Indexes() []Index
}

type Pluralize interface {
    Pluralize() bool
}
eaigner commented 11 years ago

But above all, let's agree upon an API before implementing anything!

coocood commented 11 years ago

I thought about define multiple interfaces too, and compared with the two approach. I can't see one is obviously better than the other, eather way is acceptable.

For pluralize issue, I think it's better to do it this way, more flexible type CustomName interface { TableName() string }

coocood commented 11 years ago

The go's convention to name a interface is like "Writer", "Reader", "Closer". So I suggest to name the interface like "IndexDeclarer", "TablenameDeclarer", The Method can be like "DeclareIndexes", "DeclareTablename"

Or “SomethingDefiner", "SomethingSupplier" , "SomethingProvider".

eaigner commented 11 years ago

Combined and overly verbose names are a Java disease, and have nothing to do with Go. The interface describes a property of the table, and does not do anything on it's own (like a Writer that actually writes).

coocood commented 11 years ago

Great, now I can keep syncing with your branch. But I just can't resist the convenience of defining single column index in tag, so on my project code, I just add these lines at the end of "addFields" method:

if v, ok :=parsedSqlTags["index"]; ok{
    unique := v == "*"
    m.Indexes = append(m.Indexes, NewIndex(toSnake(fd.Name),unique,toSnake(fd.Name)))
}
eaigner commented 11 years ago

Damn. I could have implemented the table indexes better. Instead of

func (table *Captcha) Indexes() []*hood.Index {
    return []*hood.Index{
        hood.NewIndex("captcha_cid_index", true, "cid"),
    }
}

I could have implemented it much more concise like this:

func (table *Captcha) Indexes(indexes *Indexes) {
  indexes.Add("captcha_cid_index", true, "cid")
}

Guess I'll change it again to that.

eaigner commented 11 years ago

Should only be a small change though, and not affect your branch.

coocood commented 11 years ago

Don't worry about my branch, feel free to change anything.

eaigner commented 11 years ago

Fixed in 5aa4514cc8b2a68d3f2acef73d551eab6e4cf20f