documentation: convert and map option behavior

[iamKunal]

I am able to use map for mapping a field within model to struct inside domain directly but doing the same via a convert does not work.

I also tried using a pointer to the field (as argument to the convert function) but it is still of no use.

On a side note, I am using this as an alternative to mapstruct in Java (https://mapstruct.org/)

Domains&Models

type SubA struct {
 SomeString1 string
 SomeInteger2 int64
}

type Domain struct {
 Sub *SubA
}


type Model {
 SomeString1 string
 SomeString2 string
}

YML

generated:
  setup: ./mapping.go
  output: ../gen/mapper.gen.go

Setup Go

type Copygen interface {
	// map Model.SomeString1 Domain.Sub.SomeString1
	Mapper(model *Model) *Domain
}


// convert Model.SomeString2 Domain.Sub.SomeInteger2
func StringToInt64(s string) int64 {
	v, err := strconv.Atoi(s)
	if err != nil {
		v = 1
	}
	return int64(v)
}

Output

Generation

package copygen

import (
	"strconv"
)

func StringToInt64(s string) int64 {
	v, err := strconv.Atoi(s)
	if err != nil {
		v = 1
	}
	return int64(v)
}

// Mapper copies a *Model to a *Domain.
func MapAvailableIndexToIndexCard(tS *Domain, fS *Model) {
	// *Domain fields
	tS.Sub.SomeString1 = fS.SomeString1
}

Environment

Operating System: MacOS
Copygen Version: v0.4.0

[switchupcb]

I may need more information from you to consider this an edge case.

Context

Using the map or tag option disables the automatcher, which allows you to manually match fields. In order to re-enable the automatcher, use the automatch option. Options are evaluated in order of declaration, so using automatch .* after declaring map and tag options provides an easy way to re-enable the automatcher for remaining fields.
— https://github.com/switchupcb/copygen#manual

Your example includes a single function Mapper to map a Model.SomeString1 to Domain.Sub.SomeString1. This configuration disables the automatcher, so the MapAvailableIndexToIndexCard function will only assign those fields as intended.

You have defined a conversion function StringToInt64 that converts a Model.SomeString2 to Domain.Sub.SomeInteger2. Therefore, there is no issue in the current output (given that your map function does not involve these fields).

To reiterate:

1. Using map disables the automatcher, which prevents copygen from assigning fields other than Model.SomeString1 to Domain.Sub.SomeString1.
2. A conversion function is defined, but is not used since Model.SomeString2 to Domain.Sub.SomeInteger2 is not matched during the Copy Function Generation.

Solution

I assume that you would like these two fields to be matched in addition to the mapped fields.

If this is the case, you must rewrite your setup file.

Example Setup File 1

type Copygen interface {
	// map Model.SomeString1 Domain.Sub.SomeString1
        // map Model.SomeString2 Domain.Sub.SomeInteger2
	Mapper(model *Model) *Domain
}

Example Setup File 2

type Copygen interface {
	// automatch .*
        // map Model.SomeString1 Domain.Sub.SomeString1
	Mapper(model *Model) *Domain
}

If either of these configurations do not work, please ask me to re-open this issue with further context.

[switchupcb]

Upon further review of the Copygen README, this reopened issue is currently categorized as a user error (due to documentation).

Issue

This user error has occurred because the behavior of the conversion option as a modifying option — as opposed to a matching option — is not clear. A matching option (e.g. map, automatch, tag) determines whether the field will be matched to another field, but a modifying option (e.g. convert) is only apply when a field is matched.

Solution

This issue is fixed once a clarifying statement is added for matching vs. modifier option behavior (including the convert option), unless the user has identified a logical edge case in Copygen's behavior. It must be clear to the user that the convert option only works on fields that are matched.

[iamKunal]

It must be clear to the user that the convert option only works on fields that are matched.

Ohhh. This feels like double work in many cases though.

I had to go ahead with goverter as it was saner in this as well as it raised an error in case any field was not matched.

I'd recommend adding a flag for error as well just in case.