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

Custom MapType #146

Open
DevNico opened this issue Oct 30, 2023 · 5 comments
Open

Custom MapType #146

DevNico opened this issue Oct 30, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@DevNico
Copy link
Contributor

DevNico commented Oct 30, 2023

I have the use case that I need to define a custom mapping function for an entire object. The problem is that this Object has inner objects which can be "automatically" mapped by other MapType definitions.

My proposed solution would be to dynamically inject an instance of the mapper into the custom convert function. This could be done for both a new MapType.custom but also the existing custom conversion functions in TypeConverters and Field.customs. The idea would be to "detect" that the provided function takes in two parameters instead of one e.g. when looking at Field.custom not only accepting Target Function(Source dto) but also Target Function(MapprInstance mappr, Source dto).

A full example of this imaginary syntax could look like this:

class WrapperDto {
  final SuperStringDto? superStringDto;
  final SuperIntDto? superIntDto;
}

sealed class Wrapper {}
class StringWrapper implements Wrapper {
  final SuperString superString;
}
class IntWrapper implements Wrapper {
  final SuperInt superInt;
}

class SuperStringDto {}
class SuperString {}

class SuperIntDto {}
class SuperInt {}

@AutoMappr([
  MapType<WrapperDto, Wrapper>.custom(mapWrapper),
  MapType<SuperStringDto, SuperString>(),
  MapType<SuperIntDto, SuperInt>(),
])
class ImaginaryMappr extends $ImaginaryWrapper {
  const ImaginaryMappr();
}

Wrapper mapWrapper(ImaginaryMappr mappr, WrapperDto dto) {
  if (dto.superStringDto != null) {
    return StringWrapper(mappr.convert(dto.superStringDto!));
  }

  if (dto.superIntDto != null) {
    return IntWrapper(mappr.convert(dto.superIntDto!));
  }

  throw Exception('Invalid wrapper dto');
}

I'd be open to implement this feature because unless I overlooked some existing feature to handle my use case I am kinda dependent on a solution :D

@DevNico DevNico added the enhancement New feature or request label Oct 30, 2023
@DevNico
Copy link
Contributor Author

DevNico commented Oct 30, 2023

The MapType.custom could potentially be left out of this proposal since TypeConverters with the dynamically injected mapper instance would cover the use case.

Actually it seems like a TypeConverter that isn't used as part of a MapType will not result in a convert implementation for that type. I don't know if that is by design / expected behaviour but if so a MapType.custom would actually be needed.

@petrnymsa
Copy link
Member

In mapWrapper, you can create an instance of Mappr. There could be cases where different mappers would be used to map output, so passing instances does not make sense.

However, allowing the creation of a 'custom' MapType for whole objects is something we did not think about.

We can introduce a special factory version of MapType<A, B>.custom(func). This function will accept instances of A (as is) and return B (as is) (that is, if you declare it as Nullable, you are expected to accept Nullable / return Nullabe).

If inside func is required to have access to "mappr" - create it on its own - we can not be sure, that for example, there is a need to have access to different mappr as well.

What do you think @tenhobi ?

@DevNico
Copy link
Contributor Author

DevNico commented Oct 30, 2023

I already started implementing a small poc because as I said I'm dependant on this feature. I settled on the first parameter just being an AutoMapprInterface if this is passed in the generated code wouldn't the implemented convert method then go through all delegates and therefore work? I think this would be more elegant than creating new instances "all the time".

@DevNico
Copy link
Contributor Author

DevNico commented Oct 30, 2023

Actually creating instances might be fine considering the reduced overhead in the generator. I actually didn't think of that initially probably because it's too easy haha. Guess I'll just implement a MapType.custom for now for my use case and let you give your input on how/if this will be implemented into the package.

@tenhobi
Copy link
Member

tenhobi commented Oct 31, 2023

If inside func is required to have access to "mappr" - create it on its own - we can not be sure, that for example, there is a need to have access to different mappr as well.

Agree. MapType.custom can be added, but access to this/other mapprs is probably easier to do inside that function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants