Search code examples
oopgostructembeddinglaw-of-demeter

Does embedding in golang violate law of demeter?


This is what the Effective GO had to say about Embedding in golang

When we embed a type, the methods of that type become methods of the outer type, but when they are invoked the receiver of the method is the inner type, not the outer one

I had a code snippet where I had Struct User defined as follows

type User struct {
    Name     string
    Password string
    *sql.Tx
}

And then I call u.Query("some query here") etc. I had done this specifically so that I could avoid calls like u.Transaction.Query("query"), which clearly violates Law of Demeter. Now after reading the docs and effective go, I am doubtful about merits of the first approach as well. Am I violating Law of Demeter? If yes the how can I avoid it ?


Solution

  • The embedding concept somewhat violates Law of Demeter as it doesn't hide the fact that a type was embedded if the type itself is exported. Note that embedding an unexported type does not violate LoD (you can't refer to unexported fields and methods).

    But this doesn't force you to refer to promoted fields or methods in a way that also violates LoD. Embedding itself is just a technique so that you can "outsource" common, shared code to other types; or from another point of view to make use of other types when creating new ones. The way you refer to the promoted fields or methods of the embedded types is what may violate the law.

    As you said, if you call it as u.Tx.Query(), that is a clear violation of Law of Demeter: you are using the implementation detail that User embeds *sql.Tx.

    But if you call it like this: u.Query() that is ok. This form does not expose or take advantage of the fact that *sql.Tx is embedded. This form will continue to work if implementation changes and *sql.Tx will not be embedded anymore (e.g. it is changed to be a "regular" field or removed completely, and a User.Query() method is added).

    If you don't want to allow access to the field value of the exported embedded type, make it an unexported regular field and add an "explicit" User.Query() method which may delegate to the field, e.g.:

    type User struct {
        Name     string
        Password string
        tx       *sql.Tx // regular field, not embedded; and not exported
    }
    
    func (u *User) Query(query string, args ...interface{}) (*sql.Rows, error) {
        return u.tx.Query(query, args...)
    }
    

    Further notes:

    In the example, if u.Query() is used, the clients using this are not affected if internals of User are changed (it doesn't matter if u.Query() denotes a promoted method or it denotes a method of User, that is: User.Query()).

    If sql.Tx changes, yes, u.Query() might not be valid anymore. But an incompatible sql.Tx is unlikely to happen. And if you're the developer of the changed package, and making incompatible changes, it is your responsibility to change other code that depends on your incompatible change. Doing so (properly updating u.Query()) the client who calls u.Query() will not be affected, the client still doesn't need to know something changed under the hood.

    This is exactly what LoD guarantees: if you use u.Query() instead of u.Tx.Query(), if the User changes internally, the client calling u.Query() does not need to know or worry about that. LoD is not a bad thing. You should not drop it. You may choose what principles you follow, but you should also think and not follow everything a chosen principle dictates all the time at any costs.

    One more thing to clear: LoD does not involve API incompatible changes. What it offers is if followed, internal changes of an entity will not have effect on other entities using the "public" face of the entity. If sql.Tx is changed in a drastic way that Tx.Query() will not be available anymore, that is not "covered" by LoD.