jgraham909 / bloggo Goto Github PK

Hi Jeff,

The disclaimer: All this is subjective... feel free to reject all suggestions. Also, I am not yet using revmo. I wanted
to see if my approach is even good.

1. I was thinking of steamlining the code a bit by providing the handler to the DB a different way.

Please check out:

Post
DB

I made up the post struct (it's not going to the pull request)

Suggestions:

1. Access a global handles to the DB.

This:

    func (article *Article) All(s *mgo.Session) []*Article {
        articles := []*Article{}
        coll := article.Collection(s)
        coll.Find(nil).Sort("-Published").Limit(10)

or this:

    func (p *Post) SetAuthor(author string) {  // no mgo session in param
        bucket, dberr := db.Bucket()
        bucket.Post.Update(  // access a collection via a global handler

2. make use of defer. I think this is clean way of closing our connections.

defer bucket.Close() // close connection when we leave scope

3. HIGHLY subjective

    func (article *Article) All(s *mgo.Session) []*Article {
    func (p *Post) SetAuthor(author string) {  // no mgo session in param

I prefer using a single character to indicate the pointer in the func signature. revel does it the same way.

That's about it. I'm still looking through the code. I wonder if we should seed the blog with data.

I would love to hear your thoughts. Again, feel free to reject all suggestions.

Thanks,
Rex

Hey, Jeff.

How do you think the mechanism of saving / updating info can be improved? Now it looks like this:
Model

type Article struct {
    Title string
    Posted time.Time
    Body string
    Author bson.ObjectId
    Smth smth
}

func (article *Article) Save(s *mgo.Session) error {
    ...
    _, err := coll.Upsert(bson.M{"_id": article.Id}, article)
    ...
}

So to use it we have to do:
Controller

func (c App) Update(article *models.Article) revel.Result {
    ...
    // Make sure that the user hasn't modified system fields
    article.Posted = ...
    article.Author = c.User
    article.Smth = smth
    article.Save(c.MongoSession)
    ...
}

So in case, we add new fields to our Article struct like this:

type Article struct {
    ...
    Smth smth
    Smth1 smth1
    Smth2 smth2
}

we have to rewrite all the fragments of updating / saving info:

...
article.Smth = smth
article.Smth1 = smth1
article.Smth2 = smth2
article.Save(c.MongoSession)

to make sure that users cannot cheat the system by sending values they are not expected to.

What do you think if we change the code a little bit the following way:
Model

func (article *Article) Save(s *mgo.Session, change *bson.M) error {
    ...
    _, err := coll.Upsert(bson.M{"_id": article.Id}, change)
    ...
}

func (article *Article) GetInitialChange() *bson.M {
    return &bson.M {
        "Title": article.Title,
        "Body": article.Body,
        "Posted": article.Posted,
        "Author": article.Author,
    }
}

func (article *Article) GetBasicChange() *bson.M {
    return &bson.M {
        "$set": bson.M {
            "Title": article.Title,
            "Body": article.Body,
        },
    }
}

So our controller then is:

func (c App) Create(article *models.Article) revel.Result {
    ...
    article.Posted = time.Now()
    article.Author = c.User
    article.Save(c.MongoSession, article.GetInitialChange())
    ...
}

func (c App) Update(article *models.Article) revel.Result {
    ...
    article.Save(c.MongoSession, article.GetBasicChange())
    ...
}

Now we can add new fields to the Article structure and do not worry about a necessity to change our controller.

I have a change using https://github.com/gedex/inflector instead of specifying all the inflections in app.conf for the table names.

I am curious if you'd be interested in accepting the change, or perhaps finding another inflector library.

Hi Jeff, solely a newbie's comment:

->user.go:26
func (user *User) Collection(s *mgo.Session) *mgo.Collection {
return s.DB("bloggo").C("users")
}

IMHO I think that there should be a way to add a abstraction layer at the module in order to get a less explicit way to use collections into models, ie,
First, the database name should be set at the app.conf file and maybe could be a binding function like Collection() but defined at module and it could bind any model's struct to a mgo.collection in the model layer, ie:
AddDataModel(*User)

Also coll var is defined in every struct method, I think that it could be a global pointer in order to call methods on this way:

db : Global Pointer to mgo session
db.User : Pointer to User Collection
db.User.find()
db.User.delete()

Well, I come from a Python Background, maybe this approach couldn't be applied.

Hi Jeff,

I ran into an issue running the bloggo app. http://pastebin.com/cJ6pF9Yn

I get that error after hitting the index page.

I can take a look at it further, but was wondering if you come across the same issue?

Thanks,
Rex