> Or maybe I'm still not getting it, my read is there are posts, they can be flagged, flagged posts can be reviewed, and this is a "basic" serializer for flagged posts that have yet to be reviewed.
I think you are right. The name is pretty clear to me, but that may be because I have worked on similar code bases where this naming is used by convention. Reading code requires knowing the domain and I'm not sure if a shorter name is more clear. You need domain knowledge to know what a post is, and what it means that it has been flagged.
You probably made a very accurate guess based on your knowledge of forums and moderator systems. This may not be apparent to all, and shorter names will probably not help much. In addition, if they shortened the name to "Serializer", "PostSerializer" or even "FlaggedPostSerializer" it could conflict with other serializers in the project.
> But why you'd need such a specialized serializer is beyond me,
I totally agree with your point. They may have their reasons, but it seems to me that a "ReviewableFlaggedPost", "FlaggedPost" and "Post" should have very similar needs and could be solved by structuring them differently (perhaps by using composable classes that can each take care of their own serialization)
Regarding the use of "Basic", it also triggers a "code smell" reaction from me. It may make sense to them, and it's hard for me to make any definitive comments without knowing the rationality behind it. My guess is that they have different types of responses based on the same "post" object. "Basic" may include a subset of the "Full" response, such as id and title only.
In those cases I tend to prefer separate DTOs, like "PostSummaryDTO" and "PostDTO" that can be re-used by composability for different responses (flagged for review etc.). This may of course not be the best choice for all usages, so I would need to know more to say something conclusive about this particular case
Yep, I searched github for it and it looks like exactly that for Discourse. I don't have an problems with the code now that I've seen it (though still don't know why it's Basic except that it's a subclass of BasicReviewableSerializer, which my question would extend to.)
I think you are right. The name is pretty clear to me, but that may be because I have worked on similar code bases where this naming is used by convention. Reading code requires knowing the domain and I'm not sure if a shorter name is more clear. You need domain knowledge to know what a post is, and what it means that it has been flagged.
You probably made a very accurate guess based on your knowledge of forums and moderator systems. This may not be apparent to all, and shorter names will probably not help much. In addition, if they shortened the name to "Serializer", "PostSerializer" or even "FlaggedPostSerializer" it could conflict with other serializers in the project.
> But why you'd need such a specialized serializer is beyond me,
I totally agree with your point. They may have their reasons, but it seems to me that a "ReviewableFlaggedPost", "FlaggedPost" and "Post" should have very similar needs and could be solved by structuring them differently (perhaps by using composable classes that can each take care of their own serialization)
Regarding the use of "Basic", it also triggers a "code smell" reaction from me. It may make sense to them, and it's hard for me to make any definitive comments without knowing the rationality behind it. My guess is that they have different types of responses based on the same "post" object. "Basic" may include a subset of the "Full" response, such as id and title only.
In those cases I tend to prefer separate DTOs, like "PostSummaryDTO" and "PostDTO" that can be re-used by composability for different responses (flagged for review etc.). This may of course not be the best choice for all usages, so I would need to know more to say something conclusive about this particular case