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

feat: provide custom constructor setting #96

Merged
merged 3 commits into from
Nov 4, 2023
Merged

feat: provide custom constructor setting #96

merged 3 commits into from
Nov 4, 2023

Conversation

lampajr
Copy link
Contributor

@lampajr lampajr commented Nov 3, 2023

Fixes #93

New feature: allow users to provide a custom constructor in order to initialize the target obj.

Usage:

        package execution

        // goverter:converter
        type Converter interface {
            // goverter:init NewOutput
            // goverter:ignore ID
            Convert(source Input) Output
        }

        type Input struct {
            ID int
            FirstName string
            LastName string
        }
        type Output struct {
            ID int
            FirstName string
            LastName string
        }
        func NewOutput() Output {
            return Output{
                ID: 0,
            }
        }

Generated converter:

        func (c *ConverterImpl) Convert(source execution.Input) execution.Output {
        	var executionOutput execution.Output = execution.NewOutput()
        	executionOutput.FirstName = source.FirstName
        	executionOutput.LastName = source.LastName
        	return executionOutput
        }

Tests:

Added some test scenarios under /scenario, all those matching constructor*.yml

@lampajr lampajr changed the title Lampajr gh93 init feat: provide custom constructor setting Nov 3, 2023
@lampajr
Copy link
Contributor Author

lampajr commented Nov 3, 2023

Hey @jmattheis,

I gave this a shot, I hope it can help 🙂

Open points:

  • Right now I used init as goverter setting, maybe we could use constructor or any other idea?
  • The constructor can be used to set some defaults, but based on the actual generation if that field (defaulted) is not ignored.. it is going to be overridden even if nil, maybe we could thinkg about an additional setting not-override (or whatever) to skip a field assignment if nil in the source, wdyt? Obv I think this could be a followup issue if you think makes sense

Copy link

codecov bot commented Nov 3, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (13d3233) 97.47% compared to head (6aa5d54) 97.87%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #96      +/-   ##
==========================================
+ Coverage   97.47%   97.87%   +0.39%     
==========================================
  Files          38       39       +1     
  Lines        1663     1693      +30     
==========================================
+ Hits         1621     1657      +36     
+ Misses         29       25       -4     
+ Partials       13       11       -2     
Files Coverage Δ
builder/default.go 100.00% <100.00%> (ø)
builder/pointer.go 100.00% <100.00%> (ø)
builder/struct.go 100.00% <100.00%> (ø)
config/method.go 100.00% <100.00%> (ø)
generator/generator.go 100.00% <100.00%> (+3.37%) ⬆️
config/extend.go 98.14% <66.66%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@jmattheis jmattheis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for the contribution! I'll probably have a look at this tomorrow.

jmattheis and others added 2 commits November 4, 2023 13:05
Co-authored-by: Andrea Lamparelli <a.lamparelli95@gmail.com>
Copy link
Owner

@jmattheis jmattheis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've fixed my remarks already. It was basically trial and error to a solution I was satisfied with.

builder/struct.go Outdated Show resolved Hide resolved
builder/struct.go Outdated Show resolved Hide resolved
config/extend.go Outdated Show resolved Hide resolved
config/method.go Outdated Show resolved Hide resolved
method/parse.go Outdated Show resolved Hide resolved
builder/struct.go Show resolved Hide resolved
@jmattheis
Copy link
Owner

@lampajr could you review the adjusted version?

docs/config/default.md Outdated Show resolved Hide resolved
builder/struct.go Show resolved Hide resolved
config/method.go Outdated Show resolved Hide resolved
@lampajr
Copy link
Contributor Author

lampajr commented Nov 4, 2023

@lampajr could you review the adjusted version?

I just fixed a leftover in the doc (init --> default), it looks really great!!

I tried it also locally in my use case, and it worked well 💪

Co-authored-by: Andrea Lamparelli <a.lamparelli95@gmail.com>
@jmattheis
Copy link
Owner

Good catch, thanks!

@jmattheis jmattheis merged commit e80558e into jmattheis:main Nov 4, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is it possible to provide default values to converters?
2 participants