Open rfuerst87 opened 1 year ago
Hey @rfuerst87, Thanks for sharing your concerns regarding the architecture design. The Rollbar SDK is a relatively new addition to our portfolio hence the unorthodox solutions. We highly appreciate every user feedback, so I'll bring this back to the devs. I'll get back to you whenever I have an update.
Hey @rfuerst87, do you think you could elaborate a bit on what you're seeing, maybe offer an example, and how would you approach it?
Are you referring to
Sorry for my late reply here. Rollbar is not really in my focus these days. However you might wan't to tackle this anyway.
The frist two examples you linked use extensions as intended. They extend functionality of classes Rollbar doesn't control. Such as Iterable
and String
. They are defined gobally, so that they can be used throughout your codebase. Nothing to "complain" there. However the third example is IMHO questionable. I'm not sure wheter private extensions are useful. Maybe it's a matter of preference, to me it's a private method with extra steps that hurts readability. Also I'm not sure what methods like JsonMap get trace => this['trace'];
are good for. Seems like syntax bending.
When I brought up this issue I had cases in mind such as:
FlutterError
seems to be pointless. Just us a plain old class instead._Methods on MethodChannel
this could just be ordinary private functions. There is many more occasions where private Extensions is declared.Body
. This feels like a code smell... Maybe there is a design flaw in how the Marshaller
is built.Please take this "complaint" with a grain of salt. There might be good reason you defined your classes and extensions like you did and this is fine. I don't have the knowlegde you have when you built this thing, and a lot of design desicions reflect personal preferences. To boil down my issue to one sentence: I have the impressions this library uses extensions for the sake of using extensions, which hurts readability and long-term maintainability.
@rfuerst87 No worries. Thanks for taking the time and sharing your thoughts, I appreciate your help :)
While debugging #91 I noticed the extensive use of dart extensions all over the code base. May I ask what the intention behind this design decision is?
As far as I can see I noticed the following pattern:
In my opinion all this diminishes readability of the code. Why not use proper interface definitions and plain old methods instead?
Thanks for a quick feedback!