Generic service via interfaces...

[Darko Luketic]

I'm stuck and I hoped it wouldn't come to that.

I wanted to have interfaces for various databases aka "I wanted to support multiple databases" (not debatable).
The idea was have ModelInterface (UserModelInterface, CategoryModelInterface etc) which would wrap the model with getters setters
StorageInterface which would CRUD the modelinterfaces
and finally services which would implement higher level and more convenient functions and use storage interfaces to store data.

Well up to the point where I started creating services everything went mostly smooth.
But I hoped I could keep the services database-agnostic.
However I can't.

https://github.com/dalu/forum/tree/f39df77f5003f71f08f473970b3df1fbd29a5a43

as you can see in line 19 and 20
https://github.com/dalu/forum/blob/f39df77f5003f71f08f473970b3df1fbd29a5a43/server/service/user.go#L19

when I use a concrete model.User (aka line 20) everything works without error.
but when I use var m storage.ModelInterface and then call a member function of the interface (SetName)
Go panics because nil pointer dereference

=== RUN   TestNewUserService
--- FAIL: TestNewUserService (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x80 pc=0x5ba9f8]

goroutine 5 [running]:
testing.tRunner.func1(0xc420068b60)
        /usr/lib/go/src/testing/testing.go:622 +0x29d
panic(0x5f1520, 0x721990)
        /usr/lib/go/src/runtime/panic.go:489 +0x2cf
git.icod.de/dalu/forum/server/service.(*UserService).CreateUser(0xc420055f40, 0x62f170, 0x8, 0x630677, 0xd, 0x62e844, 0x6, 0x7ffdb1d66ba1, 0xc420031f68)
        /home/darko/go/src/git.icod.de/dalu/forum/server/service/user.go:19 +0x28
git.icod.de/dalu/forum/server/service.TestNewUserService(0xc420068b60)
        /home/darko/go/src/git.icod.de/dalu/forum/server/service/user_test.go:23 +0x1f9
testing.tRunner(0xc420068b60, 0x63af98)
        /usr/lib/go/src/testing/testing.go:657 +0x96
created by testing.(*T).Run
        /usr/lib/go/src/testing/testing.go:697 +0x2ca
exit status 2
FAIL    git.icod.de/dalu/forum/server/service   0.005s

However I don't know what the problem was that I had previously an interface *does have* allocated memory but not in the same way the concrete model that fits the interface does so they're not compatible, which lead me to some headaches.
So when an interface has memory allocated, why can't I use it as if it has memory allocated?

As it is right now I'll have to dump all the StorageInterface and ModelInterface interfaces because it doesn't make sense to use them since I'm bound to a specific model/db at the topmost level,which is "service".
"And this is why we can't have nice things".
Removing those would also lower the memory footprint when querying for multiple results (slices, find/readall).
It's sad, I really wanted to, but I can't.

So lesson learned:
- keep models the way they are, conrete for the expected database. A mongodb model is different from a pgsql model is different from a cassandra model
- storage/repository can be concrete and it should just do CRUD
- interface only at the top level, aka service
- there is no 1 fits all in Go

routes <> handler <interface> service <concrete> storage/repository <concrete> model

effectively
MySQLUserService
MongoDBUserService
CassandraUserService
RethinkDBUserService
but
type UserService interface {
//...
}

Question, is there any sane way to solve it without throwing it all away? Hope dies last. Any trick to make it still work with interfaces on the service level without needing to create a concrete database bound instance of a model/struct?

[Justin Israel]

The zero value for an interface is nil. It has no concrete implementation on its own. So what concrete logic would you expect a nil interface to call? If you are trying to have per-database specific concrete model implementations then something has to provide that concrete implementation to your function. How are you intending the database implementation to be selected in this application? Your service accepts a storage interface but not a model interface. So what provides the model?

 

As it is right now I'll have to dump all the StorageInterface and ModelInterface interfaces because it doesn't make sense to use them since I'm bound to a specific model/db at the topmost level,which is "service".
"And this is why we can't have nice things".
Removing those would also lower the memory footprint when querying for multiple results (slices, find/readall).
It's sad, I really wanted to, but I can't.

So lesson learned:
- keep models the way they are, conrete for the expected database. A mongodb model is different from a pgsql model is different from a cassandra model
- storage/repository can be concrete and it should just do CRUD
- interface only at the top level, aka service
- there is no 1 fits all in Go

routes <> handler <interface> service <concrete> storage/repository <concrete> model

effectively
MySQLUserService
MongoDBUserService
CassandraUserService
RethinkDBUserService
but
type UserService interface {
//...
}

Question, is there any sane way to solve it without throwing it all away? Hope dies last. Any trick to make it still work with interfaces on the service level without needing to create a concrete database bound instance of a model/struct?

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Ayan George]

On 05/28/2017 02:40 PM, Darko Luketic wrote:
> I'm stuck and I hoped it wouldn't come to that.
>

As Justin said, interfaces on their own don't do anything and their zero
value is nil.

Beyond that I'd consider thinking more carefully about your
interface names. Whenever I see the word "interface" inside of an
interface name it makes me wary.

Notice that the standard library doesn't have much of that -- the only
one I can think of is sort.Interface. Instead you see Reader, Writer,
CookieJar, Valuer, Pinger, Signer, Decrypter, etc...

It seems like the best interface names are nouns that describe behavior
that other types can comply with. You should be able to define a type
and ask your self, "is this type a Writer," or, "is this type a Decrypter?"

Notice how, "is this type a UserStorageInterface," doesn't quite roll
off the tongue as easily. :)

It seems like using a simple and descriptive noun reflects a clearer
mental model for what you are trying to abstract. When I see
"interface" used in an interface name it tells me that you're not trying
to abstract behavior; you're using interfaces purely as a way to wedge
functions into some code.

-ayan

[Egon]

Don't put organize by structure, this is what seems to be causing the most of the damage, 

I would suggest implementing as:

assets/*

cmd/root.go

cmd/server.go

storage/mongo/user.go

storage/id.go

user/user.go

user/service.go

forum/server.go

main.go

User package would look like:

package user

import (

"git.icod.de/dalu/forum/server/storage"

)

type User struct {

ID             storage.ID `bson:"_id,omitempty" json:"id"`

Name           string     `bson:"name" json:"name"`

Email          string     `bson:"email" json:"email"`

Password       []byte     `bson:"password" json:"-"`

ActivationCode string     `bson:"activation_code,omitempty" json:"activation_code,omitempty"`

RecoveryCode   string     `bson:"recovery_code,omitempty" json:"recovery_code,omitempty"`

Roles          []string   `bson:"roles" json:"roles"`

}

type Storage interface {

Create(m *User) error

Update(id string, m *User) error

ReadOne(id string) (*User, error)

ReadOneBy(query map[string]interface{}) (*User, error)

ReadAll(sort string, start, limit int) ([]*User, error)

ReadAllBy(query map[string]interface{}, sort string, start, limit int) ([]UserModelInterface, error)

Delete(id string) error

DeleteBy(query map[string]interface{}) error

}

type Service struct {

storage Storage

}

func NewService(storage Storage) *Service {

return &Service{storage}

}

func (s *Service) CreateUser(name, email, password string) error {

user := &User{}

user.Name = name

user.Email = email

// ...

return s.storage.Create(m)

}

func (s *Service) VerifyActivationCode(email string) {

// ...

}

It will make your whole code easier to manage. Names will become much nicer and clearer. Lots of interfaces will disappear.

For storage.ID, it can be implemented as interface{ Set(v string); String() string } or struct { I uint64; B bson.ObjectId } (with custom marshalers)...

+ Egon

[Kevin Conway]

Before we put the focus on criticism of the OP's code organisation, it's well worth investigating the likely code bug first.

>but when I use var m storage.ModelInterface and then call a member function of the interface (SetName) Go panics because nil pointer dereference

Based on this problem description and the code snippet you provided, it looks like you are referencing the storage interface incorrectly. `var m storage.UserModelInterface` will declare a variable `m` of type `storage.UserModelInterface` and assign to it a zero value (nil). I believe what you meant to do was reference the store attached to the `UserService` instance. In that case, the correct reference would be `s.storage.SetName`.

--

[Darko Luketic]

Humm.. I wrote a reply but it seems it only reached Justin and not this group.
Also I have not received other replies via email.

Ok...

Ayan, no that's alright it was storage.UserModelInterface
and I know what you're all saying but I saw no other way to make them type agnostic and I'm a fan of explicitly calling things what they are especially when I'm experimenting.
And it's not really the point.

I tried Justin's idea or rather the idea that resulted because of Justin's reply
and
https://github.com/dalu/forum/tree/6bb47c27c606b30f91412b48b1af561abe0f5558/server/storage/mongo/service
it's not thread safe

because it operates on the passed instance of the concrete model.
In https://github.com/dalu/forum/blob/6bb47c27c606b30f91412b48b1af561abe0f5558/server/storage/mongo/service/user_test.go#L24
I initialize a service with a new user storage and a pointer to a (mongo)model.User
In https://github.com/dalu/forum/blob/6bb47c27c606b30f91412b48b1af561abe0f5558/server/storage/mongo/service/user.go#L21
I copy the pointer for the lack of better means to create a new copy, shallow copy (a := *b) doesn't work because it's an interface

So I think I'll just call it quits and go concrete up to the service level.
Yeah Egon ID is the lowest common denominator (that brainfart moment on reddit was me ;) ). And it would probably work but I'm not risking it for this one :)
For bson I can use the getter/setter interfaces but idk if there's something like it for other databases, so I'll just go abstract (or interface) on the service level.
And the storage/repository and model will be concrete.
UserService will be an interface
ThreadService will be an interface and so on
and there will be MongoUserService PostgresUserService and so on.
and implementing new databases will be a pain in the butt.
But that's Go for you :)

Thanks for feedback all.

[Ayan George]

On 05/29/2017 04:44 PM, Darko Luketic wrote:
>
> Ayan, no that's alright it was storage.UserModelInterface and I know
> what you're all saying but I saw no other way to make them type
> agnostic and I'm a fan of explicitly calling things what they are
> especially when I'm experimenting. And it's not really the point.
>

Totally -- I realize that naming isn't the point of the post. I figured
I'd point it out. I won't belabor it (any further at least) but I
recommend reconsidering your approach to this. If you really explicitly
call things what they are, do you also add "Struct" to your type names?

IMHO, type naming is a subtle but important part of building
abstractions and reflects and affects how you think about the structure
of the code. That's all I've got -- I'll shut up about it from here on. :)

-ayan

[Egon]

On Monday, 29 May 2017 23:44:32 UTC+3, Darko Luketic wrote:

Humm.. I wrote a reply but it seems it only reached Justin and not this group.
Also I have not received other replies via email.

Ok...

Ayan, no that's alright it was storage.UserModelInterface
and I know what you're all saying but I saw no other way to make them type agnostic and I'm a fan of explicitly calling things what they are especially when I'm experimenting.
And it's not really the point.

I tried Justin's idea or rather the idea that resulted because of Justin's reply
and
https://github.com/dalu/forum/tree/6bb47c27c606b30f91412b48b1af561abe0f5558/server/storage/mongo/service
it's not thread safe

because it operates on the passed instance of the concrete model.
In https://github.com/dalu/forum/blob/6bb47c27c606b30f91412b48b1af561abe0f5558/server/storage/mongo/service/user_test.go#L24
I initialize a service with a new user storage and a pointer to a (mongo)model.User
In https://github.com/dalu/forum/blob/6bb47c27c606b30f91412b48b1af561abe0f5558/server/storage/mongo/service/user.go#L21
I copy the pointer for the lack of better means to create a new copy, shallow copy (a := *b) doesn't work because it's an interface

So I think I'll just call it quits and go concrete up to the service level.
Yeah Egon ID is the lowest common denominator (that brainfart moment on reddit was me ;) ).

I completely missed that :D, Would have loved to see it.

 

And it would probably work but I'm not risking it for this one :)

With regards to the error, others already answered it, so I didn't bother with it... and answered your last question.

I know it may seem like I'm doing minor nitpicking w.r.t. value, naming and organization; but after 4+ years and researching the subject has lead to me that,

Once you properly understand those, many of the other problems just disappear... or the solution to them becomes obvious.

The organization I showed isn't without precedent... sure, I can understand if you decide not to go with that structure; often, the devil you know, will be faster to develop with.

Naming wasn't bad, it was stuttery, but not bad. The main problem was package organization.

On these topics:

* http://www.carlopescio.com/2011/04/your-coding-conventions-are-hurting-you.html

* https://medium.com/@benbjohnson/standard-package-layout-7cdbc8391fc1

* https://en.wikipedia.org/wiki/Form_follows_function

* https://github.com/marcusolsson/goddd

* https://forum.golangbridge.org/t/comparing-the-structure-of-web-applications/1198

* https://vimeo.com/108441214