Make ACRAConfiguration final

[F43nd1r]

perform #421 (comment)

ACRAConfiguration now only contains the logic of checkCrashResources. Maybe this could move somewhere else, so that it is only a configuration object and nothing else?

[F43nd1r]

Note: I used my own very simple Implementation for Immutable Collections. Guava contains better optimized versions, but I din't want to add such a huge dependency to the project just for this.

[william-ferguson-au]

{edit} removed comment that should have been on another PR

[F43nd1r]

ACRAConfiguration now only contains the logic of checkCrashResources. Maybe this could move somewhere else, so that it is only a configuration object and nothing else

@william-ferguson-au what do you think?

also should we keep ACRA.getConfig and ACRA.getNewDefaultConfig?

[william-ferguson-au]

I agree that ACRAConfiguration.checkCrashResources() belongs somewhere else.
It looks to me like the ConfigurationBuilder.build() should own that logic. But that means build() either needs to

1. throw the ACRAConfigurationException and all clients need to catch log and then not call ACRA.init()
2. return a null ACRAConfiguration and we need to relax the @NotNull on build() returns and ACRA.init(config)

Neither is pretty from a backwards compat POV but the first is more correct.
WDYT?

I'd like to get rid of ACRA.getNewDefaultConfig(). But if we are going to do that then we need to expunge it from the Wiki.

ACRA.getConfig() is not such a big issue. It can stay for a while as deprecated. Now that ACRAConfiguration is final accessing it can do far less damage.

[william-ferguson-au]

This is what I have in mind #445

[F43nd1r]

@william-ferguson-au Maybe now that ACRAConfiguration is final, subsequent init calls should be allowed. I don't think removing runtime configuration changes completely is a good idea.

[william-ferguson-au]

What's the use case?

On Fri, 29 Apr 2016, 6:47 AM F43nd1r notifications@github.com wrote:

@william-ferguson-au https://github.com/william-ferguson-au Maybe now
that ACRAConfiguration is final, subsequent init calls should be allowed.
I don't think removing runtime configuration changes completely is a good
idea.

—
You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#439 (comment)

[F43nd1r]

E.g. http://stackoverflow.com/questions/36909342/acra-change-formuri-programatically

[william-ferguson-au]

I consider that to be an extreme edge case that can almost certainly be handled more simply by thinking about the problem differently. Instead of reiterating here why I don't think allowing multiple #inits is a good idea, see my response on SO.

[jngr]

For handled exceptions, showing case-specific text would still be useful. While it can be possible to recover from an exception the app may not do what's expected, and informing the user would be polite. Particularly important in case some user input could not be saved.

getConfig() would probably let me do that but if it's deprecated I hesitate using it in my new app. The way I do it now is with a second dialog or a toast but that's pretty heavy.

Ideal for that use case would be

• an additional argument in handleException() passing the text to show instead of the default text from the initial configuration
• an option 'end activity' in addition to 'end application' (preventing an endless loop e.g. in case of an exception thrown in onResume())

[william-ferguson-au]

@jngr I don't understand your comment.

If you are handling the Exception within your app then ACRA will not be invoked.

[jngr]

Yes, true, but unless I use Exceptions as means of program control (which I try to avoid) there is still something wrong, and I want to know about it even if the app recovers to some degree. I believe that's why you included ACRA.getErrorReporter().handleException(e).

For instance, you can recover from a failed database update but you definitely want to let the user know about it. Or I could imagine other cases where I handle failure of a less important feature to let the user save his data instead of directly exiting the app.

[F43nd1r]

If you want to have exception specific text in the crash dialog, write your own CrashReportDialog. The base class provides access to the exception.

[jngr]

OK, but since you're not directly invoking that dialog how would you transport the custom message? That same exception could happen in different places, and the dialog won't know unless it dissects the stack trace. Pushing the text would in any case be cleaner. There is the exception message - using that, localized - seems a bit messy. I probably overlooked something. What did you have in mind?

[F43nd1r]

As long as it is serializable you can add anything to custom exceptions, including e.g. resource ids.

[jngr]

That sounds interesting. I'll keep it in mind. Thank you. If you happen to know where I could find an example for such a class, a pointer would be great.

[F43nd1r]

import android.support.annotation.StringRes;

/**
 * @author F43nd1r
 * @since 27.09.2016
 */
public class BaseException extends Exception {
    @StringRes
    private final int msgId;

    public BaseException(@StringRes int msgId) {
        this.msgId = msgId;
    }

    public BaseException(String message, @StringRes int msgId) {
        super(message);
        this.msgId = msgId;
    }

    public BaseException(String message, Throwable cause, @StringRes int msgId) {
        super(message, cause);
        this.msgId = msgId;
    }

    public BaseException(Throwable cause, @StringRes int msgId) {
        super(cause);
        this.msgId = msgId;
    }

    @StringRes
    public int getMsgId() {
        return msgId;
    }
}

[jngr]

OK, seems straightforward. :) Thank you very much!