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

Allow to use custom types with source gen. #73

Closed
Ashymonth opened this issue Sep 9, 2024 · 2 comments
Closed

Allow to use custom types with source gen. #73

Ashymonth opened this issue Sep 9, 2024 · 2 comments
Labels
enhancement New feature or request
Milestone

Comments

@Ashymonth
Copy link

Ashymonth commented Sep 9, 2024

Currently, it is not possible to use custom types as values for a cell without mapping model properties. I'd like to ask for the option to change this behavior. As an example:

public class ExampleExcelRow
{
    /// <summary>
    /// To type can be applied the style NumberFormat.TwoDecimalPlaces and the provider function can be used to round.
    /// </summary>
    [ColumnHeader("Money value")]
    [CellStyle("MoneyStyleName")]
    [CellValueConverter(typeof(NullToDashDataCellConverter<Money>))]
    public Money MoneyType { get; set; }
    
    /// <summary>
    /// To type can be applied the style NumberFormat.TwoDecimalPlaces and the provider function can be used to round.
    /// </summary>
    [ColumnHeader("Percent value")]
    [CellStyle("Percent style")]
    [CellValueConverter(typeof(NullToDashDataCellConverter<Percent>))]
    public Percent PercentType { get; set; }
    
    /// <summary>
    /// Just general case
    /// </summary>
    [ColumnHeader("A regular value")]
    public decimal Regular { get; set; }
}

This can help to avoid manual mapping of object value to simple value, especially if the custom object has a ToString() overload. For me personally, it's more readable to have types for different numbers, like money, percent, numbers with dots, and maybe for someone else it's more readable too.

So it's can be smth like AllowCustomTypesAttribute to instruct src gen to use ToString(). As an example we can look at Mapperly library, they have similar functional for mappers. Or at least allow to use it with CellValueConverter

[WorksheetRow(typeof(SummaryReportExcelRow))]
[AllowCustomTypes]
internal partial class ExcelRowContext : WorksheetRowContext;

and in source gen:

 cells[0] = new DataCell(obj.MoneyType?.ToString());
 cells[1] = new StyledCell(new DataCell(obj.PercentType?.ToString()), styleIds[0]);
@sveinungf
Copy link
Owner

Using a custom type for which you have declared a CellValueConverter will be a natural extension of what we have now. So that sounds like a good idea.

I don't like the approach of using ToString() though for the following reasons:

  • Custom types would only be converted to string cells.
  • Implementations of ToString() will most likely allocate new strings. So potentially not great performance-wise.
  • It won't be obvious from reading the code that ToString() is being called on these types.
  • The code or IDE won't inform you if you have forgotten to implement ToString().
  • You can only have one ToString() implementation for a type. Some might want to use it for e.g. debugging purposes instead.

@sveinungf
Copy link
Owner

This is now available in version 1.18.0.

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

2 participants