search

amis92 / RecordGenerator

C# immutable records generator
https://amis92.github.io/RecordGenerator/
MIT License
72 stars 10 forks source link

Reorganize and refactor internals #71

Closed amis92 closed 5 years ago

amis92 commented 5 years ago

I still have my doubts about the overall approach of bit-per-feature, but I'm putting that aside for now. Apart from the other comments I've left, I think RecordDescriptor and generators need some refactoring because it seems that Features doesn't belong on RecordDescriptor. Unfortunately, that means some sweeping changes, and as a consequence, would take pages of comments to explain, justify & demonstrate the alternative. It would be much easier and quickly done as a PR and then use that as a basis for discussion. However, since all of those concerns are about the internal design, I've refrained from pointing them out as I am not sure what all you were looking for from this review. What I've done instead is focus on the public surface because the internals can always re-worked (let me know if you're interested in that PR).

I don't think this belongs here because it feels like this is mixing description of a record with code generation options.

_Originally posted by @atifaziz in https://github.com/_render_node/MDIzOlB1bGxSZXF1ZXN0UmV2aWV3VGhyZWFkMTg4OTc3NDE2OnYy/pull_request_review_threads/discussion_

amis92 commented 5 years ago

I'm very open for suggestions. I know this project needs a lot of refactoring and rethinking it.

For one, I'm heavily considering rewriting the whole generation to be string-based with the final step being parsing the generated syntax back to SyntaxNodes as required by CG.R framework.

It'd definitely improve readability. No idea about performance impact, but a wild guess is it wouldn't be much worse, maybe even better since we'd drop a lot of allocations being done when fluent-building syntax by SyntaxFactory.

amis92 commented 5 years ago

Regarding RecordDescriptor getting overused, I agree. We probably ought to create a record generation context that'd contain necessary information, and keep descriptor focused on syntax nodes.

amis92 commented 5 years ago

Random thought: One of the things I think could help readability is refactoring GeneratedCodeAttributeGenerator to use SyntaxRewriter approach instead of the custom dictionary with delegates.

edit: extracted to #80

atifaziz commented 5 years ago

This was addressed by 60277823d04b3725096911eb564ad79654f502ca thus close?