Extensive use of dart extensions

[rfuerst87]

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:

• Extensions (with only a few exceptions) are applied to self-owned classes
• Extensions are all private to their respective classes
• Some extensions define the same method signatures on classes their applied on

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!

[ghost]

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.

[matux]

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

• extensions done to the language (eg. the ones found in https://github.com/rollbar/rollbar-flutter/blob/main/rollbar_common/lib/src/extension/object.dart or https://github.com/rollbar/rollbar-flutter/blob/main/rollbar_common/lib/src/extension/function.dart)
• Extensions on Dart's stdlib (eg. https://github.com/rollbar/rollbar-flutter/blob/main/rollbar_common/lib/src/extension/collection.dart, https://github.com/rollbar/rollbar-flutter/blob/main/rollbar_common/lib/src/zipped.dart or https://github.com/rollbar/rollbar-flutter/blob/main/rollbar_common/lib/src/tuple.dart)
• Or other extensions like in https://github.com/rollbar/rollbar-flutter/blob/main/rollbar_dart/lib/src/data/payload/body.dart#L129?

[rfuerst87]

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:

• https://github.com/rollbar/rollbar-flutter/blob/main/rollbar_flutter/lib/src/flutter_error.dart
  Defining this as extension on FlutterError seems to be pointless. Just us a plain old class instead.
• https://github.com/rollbar/rollbar-flutter/blob/main/rollbar_flutter/lib/src/rollbar.dart#L13
  Instead of defining _Methods on MethodChannel this could just be ordinary private functions. There is many more occasions where private Extensions is declared.
• https://github.com/rollbar/rollbar-flutter/blob/main/rollbar_dart/lib/src/marshaller/data_marshaller.dart
  Extensions defined on classes you control yourself, such as 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.

[ghost]

@rfuerst87 No worries. Thanks for taking the time and sharing your thoughts, I appreciate your help :)