Please revert the isEnabled(...) back to just being a boolean

[JamesAiken]

A safe way to manage including a third party package into any project, is to write a wrapper class around it.
For example, with Adjust V4 I have an AdjustWrapper class with this method:

public static bool IsEnabled() { return Adjust.isEnabled(); }

But in Adjust V5 the migration guide shows this replacement.

Adjust.IsEnabled(isEnabled => { // use isEnabled });

I'll be honest, I just think this replacement isn't good. I'm just looking for a simple boolean. Not to have to write some weird logic the isEnabled function itself. When Adjust.Enable() or Adjust.Disable() is called, I would expect something within the Adjust code is caching whether or not Adjust is enabled or not. So it should be straight forward to just return a boolean instead. Can this be reverted back? It'd be much more convenient.

[uerceg]

Hey @JamesAiken,

Let me try to explain the reasoning and motivation behind this change first.

SDK v4 indeed contained synchronous Adjust.isEnabled() getter that was returning the bool about what was currently read from the place where we keep the information about SDK being enabled or disabled (in-memory value of the value in some file on file system). In the super early days of v4, setEnabled method was a method that used to flip the enable/disable switch in the SDK as soon as that method was invoked. As the years went on, we were getting more and more complains because of this behavior - all of the API that we have made was in theory being built with having FIFO order of execution, exactly in the way how calls to the Adjust methods were made. However, enabling/disabling (together with its getter) was an exception due to the behavior explained above. For some clients it was really important to preserve the order of execution and if they would perform the following in their app code line after line:

Adjust.trackEvent(event1);
Adjust.trackEvent(event2);
Adjust.trackEvent(event3);
Adjust.setEnabled(false);

expectation was that all three events should be tracked before SDK gets disabled (which initial implementation was effectively not allowing because in-memory setting of the enabled/disabled flag is way faster than creation of package events / their persistence to the package queue in the files, making HTTP request, etc). Behavior we initially had was deemed as not desirable since it was preventing events from being tracked in situations like this (and these could be some big revenue events or anything important one can think of in worse case scenarios where it's super important to properly display this information in our dashboard).

So back then we have changed the behavior of the setEnabled method to be synchronous with other calls to Adjust API. That fixed the issue which we had complaints about. However, there was the isEnabled getter that has remained synchronous and effectively not accurate in certain edge cases (it remained synchronous because we were reluctant to introduce identical async getter next to the sync one and potentially confuse clients which one to use && removal of the synchronous one to be replaced by the asynchronous was requiring major version upgrade which back in the days we didn't have in our plans). If we were to continue with the example from above, now with the new behavior of the setEnabled method, something like this would make isEnabled to return the wrong value:

// SDK is enabled
Adjust.trackEvent(event1);
Adjust.trackEvent(event2);
Adjust.trackEvent(event3);
Adjust.setEnabled(false);
bool isEnabled = Adjust.isEnabled();

isEnabled value would say true. Because now synchronization of the Adjust.setEnabled call is making it too slow in this example and simple reading of the in-memory flag that Adjust.isEnabled() is looking into is way faster to execute. However, this particular behavior was something that seemed to be quite an edge case and isEnabled getter remained synchronous during the SDK v4 lifetime, even though it was possible for it to return wrong value under certain conditions.

With the v5 we wanted to patch this finally and make isEnabled getter to always return the proper value back, regardless of when it was invoked. Important to state in here that the correct value in this context refers to enabled/disabled state after a call to Adjust.setEnabled has actually executed by the SDK, not when the call to that API was made.

Those are the reasons why isEnabled getter is now asynchronous in SDK v5. Disregarding the pattern itself (sync vs. async), with this implementation we have achieved that this getter now should return the correct value all the time (vs. having it not return the correct value in some edge cases in v4). We are aware that the tradeoffs we made might not make everyone happy. But I hope that at least I managed to explain to you why are things like this and that hopefully they make a bit more sense now.

I understand that some of the concerns we were trying to address with this change (and maybe even the v4 change in way how setEnabled works) might be nothing that applies to your particular case and that in theory even leaving methods to disable/enable SDK as sync in first place would do just fine. I also understand that some people prefer more sync over async paradigm, but async paradigm in this case was chosen due to SDK's internal behavior.

I am also curious to hear more about your particular use case. Is the async pattern preventing you now to do something which you were able to do with synchronous one or is it just a matter of style?

[JamesAiken]

I am also curious to hear more about your particular use case. Is the async pattern preventing you now to do something which you were able to do with synchronous one or is it just a matter of style?

The use case is we just want to check if Adjust is enabled or not so we can determine if we need to set up an additional script we wrote internally for handling analytics with adjust.

The quick and relevant snippets of our code:
AnalyticsManager.cs - Check if we can include Adjust as one of the analytics providers we use.

if (AdjustWrapper.isEnabled())
{
    _providers.Add(new AdjustAnalyticsProvider());
}

AdjustWrapper.cs - A static class that makes it so any calls to Adjust go through this. Making future updates easier since it's all in one place.

public static bool IsEnabled()
{
    return Adjust.IsEnabled();
}

This is how it would look when using V4. We encapsulate anytime we would need to use the Adjust class purposefully. As with any SDK, whenever there's a major update a lot can change. If we can keep having to make changes to one or two files, then it's a much smoother upgrade. If we for example called Adjust.trackEvent(...) across our code base, and that function had changed significantly, then that means we might have to find all locations where we're calling that and make any necessary changes.

In this case a synchronous approach is better. We want to know before the code moves on. This is what we'd consider to be part of the app start up process. Everything in such a process needs to be up and ready before moving on.

For v5, we're basically having to store a boolean inside AdjustWrapper that says if Adjust is enabled or not. If we call Adjust.Enable() or Adjust.Disable() then we'd swap the boolean. Instead of calling Adjust.IsEnabled(...) we would just return the boolean that's a part of AdjustWrapper. It's the closest we have to getting around it, and it's a simple approach.

[uerceg]

Thank you for detailed explanation and I see how this plays into your use case.

Based on the workaround that you have explained that you have implemented, it seems that for you SDK being enabled or disabled equals the fact whether API was invoked or not (regardless of what happens under the hood). Which is not what the semantics of enabled/disabled from Adjust SDK API POV is.

We will revisit our stance on this behavior on our end and try to see if there's some non-confusing way to make peace between the things I mentioned above and your particular use case. If synchronicity is important to you during the scripts loading process, I would say that what you did in your wrapper as a workaround sounds like a right way to go for the time being.

[JamesAiken]

In short, being able to check if the SDK is enabled or disabled determines if we'll be able to track any adjust events in the session. So that's why we want to check it early into app start up and don't need an async way of handling it.

[uerceg]

Thank you for the additional explanation. I understand what you are saying, but like said - due to SDK nature and way how it processes requests, synchronous way of invoking this getter can't guarantee that the return value will be correct all the time.

Also, if the SDK is disabled, it will discard any attempt to track an event. I would assume that there is probably some external logic that is driving your decision to enable or disable SDK (user consent, some privacy-related things or something else). Of course, it depends on your app logic whether this makes sense to you or not, but in theory, those two concerns can be completely separated - SDK being enabled or disabled can be one thing and tracking events in your app can take place where it's supposed to happen, regardless of the fact whether SDK is enabled or not; if disabled, events just won't be tracked for that user. In theory, you don't really need to check whether SDK is enabled in order to perform tracking event (unless that matters to you for some reason) - you can just track it and in case SDK is disabled, that will just be ignored.

I understand how synchronous way of doing things is more valuable in case where scripts are being loaded vs. potentially events being tracked, for the time being, I'd say that the workaround that you implemented in your wrapper is the way to go for your use case.