frp icon indicating copy to clipboard operation
frp copied to clipboard

Package naming is too generic for a library

Open velovix opened this issue 5 years ago • 6 comments

Package names like server, client, and config make a lot of sense for internal development of FRP, but are too generic when FRP is used as a library in another system. When reading non-FRP code, it's not clear that these names are related to FRP.

To address this, a new package could be created, maybe named frp, where all interfaces intended to be used as a library are stored. Code could be migrated to this new package or we could use type aliases, like this:

package frp

import (
    "github.com/fatedier/frp/server"
    "github.com/fatedier/frp/client"
    "github.com/fatedier/frp/models/config"
)

type Server = server.Service
type ServerCommonConf = config.ServerCommonConf

func NewServer(cfg ServerCommonConf) (*Server, error) {
    return server.NewService(cfg)
}

type Client = client.Service
type ClientCommonConf = config.ClientCommonConf
type ProxyConf = config.ProxyConf
type VisitorConf = config.VisitorConf

func NewClient(cfg ClientCommonConfig, pxyCfgs map[string]ProxyConf, visitorCfgs map[string]config.VisitorConf, cfgFile string) (*Client, error) {
    return client.NewService(cfg)
}

This would give us a lot of control over which APIs should be exposed for library usage and which should not, and would lead to less ambiguous naming for users. Compare server.Service to frp.Server.

I'm not sure whether type aliasing or code migration is more preferable. My first impression is that I would rather migrate the code as I think it would lead to more code clarity. Internal types like server.Control would stay where they are, as I don't think there's any reason why a library user would need them.

If this or an alternative proposal that addresses the same problem looks good, I would be happy to send a pull request.

velovix avatar Aug 24 '19 19:08 velovix

I think it's common to import github.com/fatedier/frp/server and call server.NewService or import github.com/fatedier/frp/client and call client.NewService

frp is included in the import path, you know it when you import this package.

You can also change import path to this format:

import (
    frps "github.com/fatedier/frp/server"
    frpc "github.com/fatedier/frp/client"
)

So it looks like frps.NewService and frpc.NewService

I prefer not to affect current code structure.

fatedier avatar Aug 25 '19 03:08 fatedier

frp is included in the import path, you know it when you import this package.

That's true, but it requires that you look at the import path to find out what server or client we're talking about in code, which seems like an ergonomics problem to me.

As an example, the standard library uses the package name net/http, so users type http.Client and http.Server. What makes net/http unique isn't that it provides a server and client, lots of packages do that. What makes it unique is that it offers HTTP implementations of network concepts. I think the same reasoning holds for FRP.

I also think that these names don't fit with this guideline from the Package Names blog post:

Don't steal good names from the user. Avoid giving a package a name that is commonly used in client code. For example, the buffered I/O package is called bufio, not buf, since buf is a good variable name for a buffer.

You can also change import path to this format:

This is a decent solution, and the one I will probably use if the current package naming scheme stays the way it is.

I prefer not to affect current code structure.

I respect that, and I understand that FRP is a tool first and a library second. I'm hoping that the aliasing solution would be a reasonable middle ground, because it allows for current code structure to remain the same while exposing a select group of APIs in a way that's more consumable as a library.

velovix avatar Aug 26 '19 17:08 velovix

OK. Maybe we can create a new package sdk/frp with client.go, server.go and config.go.

Type alias and nested structs are all optional.

fatedier avatar Aug 27 '19 03:08 fatedier

Unfortunately, we're no longer using FRP, so I don't have a good use-case to work on these kinds of issues anymore. Please feel free to close this issue if you don't foresee anybody else wanting to use FRP as a library.

velovix avatar Oct 28 '19 18:10 velovix

Just keep it open, it may help other peoples with same requirement.

Thanks for all your contribution.

fatedier avatar Oct 29 '19 02:10 fatedier

支持 go mod 模式引入吗

0x76long avatar Aug 04 '21 03:08 0x76long