Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: Go 2: refining Go struct type #21604

Closed
henryas opened this issue Aug 25, 2017 · 9 comments
Closed

proposal: Go 2: refining Go struct type #21604

henryas opened this issue Aug 25, 2017 · 9 comments
Labels
FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change
Milestone

Comments

@henryas
Copy link

henryas commented Aug 25, 2017

Overview

Under this proposal, users may refer to a data type in two ways: by its signature and by its concrete implementation.

Go's interface handles the "by signature" reference. As long as any object satisfies the required signature, that object can be used. There is no change to the current implementation of Go's interface under this proposal.

However, there are times when we want to separate two distinct objects with identical signature, because they may represent different things. We would normally use struct for this, but Go's struct has many limitations. Hence, this proposal proposes some enhancement to Go's struct implementation.

Problems and Solutions

Construction and Copying

Problem

When using a struct, there is no way to enforce how the struct is supposed to be constructed, whether copying is allowed, and how it should be copied. The current workaround is to write documentation and pray that the users will read and honor its intended usage. The compiler will not enforce this. One such example is the sync package.

Solution

  1. Support proper constructor
    Reserve New as a special keyword for constructor

    func New() MyType {
        //implementation..
    }
    
    func New(arg1 string) MyType {
        //implementation..
    }
    
    func New(arg1 string, arg2 int) MyType {
       //implementation..
    }
    

    Usage:

    var myType MyType //this will call New() (default constructor). If New() is not provided, it will panic.
    myType := MyType("My string") //This will call New(string)
    myType := MyType("My string", 10) //This will call New(string, int)
    

    Note that it is intentionally kept in the similar format to type casting, in case if one needs to refactor MyType, such as from:

    type MyType string
    

    to

    type MyType struct {
        //implementation..
    }
    
  2. Support proper copy constructor.
    Use Clone as the special keyword for copy constructor.

    type MyType struct {
        //implementation..
    }
    
    func (m MyType) Clone() MyType {
        //implementation..
    }
    

    Usage:

    //passing by value. If there is no Clone method, it will default to normal shallow copying.
    //If Clone method is defined, it will call the clone method. This is also a good opportunity to 
    //panic if MyType is not supposed to be copied (eg. contains Mutex), by simply call panic 
    //inside the Clone method 
    func DoSomething(myType MyType) {
        //implementation
    }
    

Composition

Problem

Given the following definition:

type Base struct {
    //implementation...
}

type Sub struct {
   Base
}

func DoSomething(base Base) {
    //implementation..
}

It is currently impossible to do this..

var sub Sub
DoSomething(sub) \\error
DoSomething(sub.Base) \\This is the only way but it gets tedious if sub does not directly holds Base. 

Solution

Support proper inheritance just like its interface counterpart. So if Sub contains Base, then Sub must be usable for any Base type. Ideally, we should be able to do this:

DoSomething(sub)

Let me know what you think.

Thanks.

Henry

@gopherbot gopherbot added this to the Proposal milestone Aug 25, 2017
@ianlancetaylor ianlancetaylor changed the title Proposal: Refining Go Struct Type proposal: Go 2: refining Go struct type Aug 25, 2017
@ianlancetaylor ianlancetaylor added LanguageChange v2 A language change or incompatible library change labels Aug 25, 2017
@ianlancetaylor
Copy link
Contributor

Frankly, my first reaction is simply that Go is not C++. In C++, you design types. In Go, you write code.

@henryas
Copy link
Author

henryas commented Aug 25, 2017

I have my fair share of reservation about C++ and I would hate to see Go turning into C++. However, just because I dislike C++, it does not mean we should do everything opposite of C++. In fact, the proposal contains nothing about C++ and its specific features. (Not everything about C++ is bad by the way)

All I ask is greater control over struct instantiation. That's what the proposal is mainly about. Instead of putting warnings in the documentation "Do not copy" or "Please instantiate using this function first" or "Call this function before calling any other functions", the behavior will now be enforced by the compiler.

Types are important. In fact, there is a reason why Go opts for a stronger and more elaborate type system than C. I don't mean we should go overboard like Haskell, but I don't think that it is too much to ask for greater control over struct's instantiation.

@as
Copy link
Contributor

as commented Aug 25, 2017

type Base struct {
    //implementation...
}
type Sub struct {
   Base
}
func DoSomething(base Base) {
    //implementation..
}
var sub Sub
DoSomething(sub) \\error

Let's not give c++ credit for this. The plan 9 C compiler supported promoting unnamed substructures and resembles composition much more accurately. This is one feature I initially missed when starting to use Go, but interfaces solve the problem well enough in most cases that it's not necessary, at least for me.

http://www.vitanuova.com/inferno/papers/compiler.html

	typedef
	struct	lock
	{
		int    locked;
	} Lock;

	typedef
	struct	node
	{
		int	type;
		union
		{
			double dval;
			float  fval;
			long   lval;
		};
		Lock;
	} Node;

	Lock*	lock;
	Node*	node;
The declaration of Node has an unnamed substructure of type Lock and an unnamed subunion. 
One use of this feature allows references to elements of the subunion to be accessed 
as if they were in the outer structure. 

When an outer structure is used in a context that is only legal for an unnamed substructure, 
the compiler promotes the reference to the unnamed substructure... 
Thus, continuing with the example,

	lock = node;

would assign a pointer to the unnamed Lock in the Node to the variable lock. Another example,
	extern void lock(Lock*);
	func(...)
	{
		...
		lock(node);
		...
	}
will pass a pointer to the Lock substructure.

@henryas
Copy link
Author

henryas commented Aug 25, 2017

Relying on purely on object behavior has its limitation. For instance, given that ...

type Employee interface {
   ID() string
   Name() string
}

//and the various implementation of Employee, such as Staff, Freelancer, SubContractor, etc.

//and the given method
company.AddToPayroll(people ...Employee)

Now what happens when we have this ...

type Table struct {
    //implementation..
}

func (t Table) ID() string {
    //implementation..
}

func (t Table) Name() string {
   //implementation..
}

func (t Table) Price() uint64 {
    //implementation..
}

//and we can do this?!?!
company.AddToPayroll(table) //WHAT?!

@AlekSi
Copy link
Contributor

AlekSi commented Aug 25, 2017

In my experience, that's rarely a problem. If you really want to, you can add a private method to Employee interface to prevent anyone to accidentally implement it in other packages. One example is testing.TB (click TB to see the source code).

Alternatively, you can make an Employee struct with Type field and stop thinking in terms of class hierarchies.

@mrcrgl
Copy link

mrcrgl commented Aug 25, 2017

@henryas it seem that you just think in types instead of implementations. Using interfaces as replacement for a missing inheritance pattern isn't as practical.

My suggestion for your example would look like this:

type Payroll struct {}
type WorkRecord struct {}
type Stuff struct {}

func (s Stuff) Payroll(year, month int) Payroll {
    //implementation..
}

type Freelancer struct {}

func (s Freelancer) Payroll(year, month int) Payroll {
    //implementation..
}

func (s Freelancer) WorkRecord(year, month int) WorkRecord {
    //implementation..
}

type Payroller interface {
   Payroll(int, int) Payroll
}

type WorkRecorder interface {
   WorkRecord(int, int) WorkRecord
}

Given this structs, the implementation could look like this

func (c Company) Settle(year, month int) Settlement {
  for _, p := range c.payrollers {
    pr := p.Payroll(year, month)

    // Check if given payroller implements WorkRecorder
    if workRecorder, ok := p.(WorkRecorder); ok {
      wr := workRecorder.WorkRecord()
      if !c.CheckWorkRecord(pr, wr) {
        // Impl.
      }
    }
  }
}
company.AddToPayroll(payrollers ...Payroller)

Does this work for you?

@henryas
Copy link
Author

henryas commented Aug 25, 2017

I am not saying there isn't any workaround. I am pretty sure workarounds exist. It was just a hypothetical example by the way. I am here suggesting possible improvements to Go. I have no problem with interface. I think it should stay as it is. However, I believe Go struct type could be much more useful with greater leverage over it's instantiation and composition. That's what the proposal is all about.

Coincidentally, not long after I posted the proposal, I encountered a problem which may be relevant to the subject.

I wrote a tool that auto-generates the DAL (Data Access Layer). So given a struct or structs, it will generate the corresponding mapper for CRUD access to the database. It has a Criteria interface that allows complex operations to be performed on the database. It generates the required SQL commands. I usually use it for 1-3 structs in a package and encountered no problem.

My colleague used it on 7-8 structs in a package today. The tool generates the code perfectly, but due to the huge number of structs my colleague had several typo errors upon use. Basically, the query criteria for struct A is used for struct B mapper, etc. Since all criteria have identical signature, the compiler did not detect the error.

So I rewrote the tool. I got rid of the Criteria interface and used concrete implementation instead. One specific criteria for each mapper. The new tool caught more bugs due to usage errors. Still, I am not happy with the result. By using struct, I have no control over how criteria may be instantiated. The best I could do is to write in the docs that criteria must be instantiated through the proper constructors. The problem is that Criteria may be combined and recombined in a countless combination. It is not practical to validate each criteria before each use. Ideally, it should be validated when it is created. There is a real possibility that uninitialized criteria may escape detection into production and messes with the actual database. All this could have been prevented if I have control over the struct construction. Luckily, my criteria is immutable and contains no mutex, channel, etc. So I don't have to worry about copying. I am now considering whether to write a custom linter to check for uninstantiated criteria but it does seem like an overkill for a supposedly simple problem.

@bcmills
Copy link
Contributor

bcmills commented Sep 6, 2017

The two problems in this proposal seem more-or-less orthogonal, and the proposal is missing concrete motivating examples. Some cases from https://golang.org/wiki/ExperienceReports would be helpful, but as it stands this proposal seems much too broad.

@ianlancetaylor
Copy link
Contributor

This proposal is about writing your code in the form of types. Go prefers writing your code in the form of code. This change is a few steps too far for Go. We aren't going to adopt it. Sorry.

@golang golang locked and limited conversation to collaborators Mar 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

7 participants