Make CrashManagerListener an Interface

[shortstuffsushi]

Sorry if this was discussed elsewhere, a brief glance through the issues didn't make it appear to be the case.

CrashManagerListener is passed as a third parameter callback param in the initialize function. In some cases where I want to implement that sort of item, I'll use the current this context to do so. However, because I'm in an Activity, and the CrashManagerListener is not an interface, I can't do that. I understand the idea of it being abstract to allow for optional methods, but it makes it impossible for you to do anything besides subclass. Ultimately you end up making a throwaway class for the sake of that one method.

Not sure what your opinions on this will be, but I think it's worth putting out there as a discussion item.

[matthiaswenz]

The main idea was to support easy in-line overrides since all methods are optional to override.
What we could do is have an interface for CrashManagerListener and then have an abstract AbstractCrashManagerListener which implements this for the optional override.
However, this will be a breaking change, not to be implemented before the next major version.
@elektrojunge thoughts?

[ElektrojungeAtWork]

This is a very good idea.
We're going to release 4.1.4 later this month and haven't talked about 5.0.0, let's start talking about it, soon. :)

[shortstuffsushi]

I like both of these comments. I agree it would be a breaking change, and would therefore need to wait til the next major revision, but I think it could be a good change.

One side note -- you mention having 4.1.4 (or perhaps 4.1.3) out, but currently Android Studio isn't picking up a new version as available. Have you updated your listings on Maven / JCenter?

[shortstuffsushi]

Hmm, never mind. I do see it listed, it's just not being suggested by AS for some reason.

[jaredsburrows]

@shortstuffsushi Should we close this? or will you accept a PR @TroubleMakerBen?

[shortstuffsushi]

I'm not using this currently, so it's unlikely that I'll do the work for it, but it still seems that it could be valuable to others. Up to you guys. I see "wontfix" was already added, so closing seems appropriate since the intention is to not do this.

[ElektrojungeAtWork]

Hey folks,

I added the "wontfix" tag for us as we won't provide a PR for this ourselves. That said, we're happily accepting PRs. =)

[jaredsburrows]

@TroubleMakerBen I feel like we should be consistent. If want to change a single listener from abstract class to interface, we can should make them all interfaces. What do you think?

[ElektrojungeAtWork]

@jaredsburrows I agree. If we gonna make this change, we need to make the listeners interface. We need to make sure that we are not introducing a breaking change, but a) have interfaces b) have implementations of them and also make sure we deprecate before removing/refactoring.

[jaredsburrows]

@TroubleMakerBen After reviewing this, I would vote for not making this change for the following reason:

• I believe that the current architecture with abstract classes is better because we do not need users to implement all methods, especially if they are not being used.
• Also it would introduce breaking changes like you said, because it would force users to go back and implement the remaining methods.

[ElektrojungeAtWork]

@jaredsburrows I completely agree with you – which is why we didn't make the change in the past. And as a user, it'd be somewhat irritating to have an interface for Crashes, and none for the other functionality. What do you think?

[jaredsburrows]

I completely agree. Maybe it safe to close this now?

[ElektrojungeAtWork]

Yeah. Thx for the discussion!
Have a great weekend!