-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature_MC-1970 #88
base: develop
Are you sure you want to change the base?
Feature_MC-1970 #88
Conversation
using CleverTapSDK.Utilities; | ||
|
||
#if (!UNITY_IOS && !UNITY_ANDROID) || UNITY_EDITOR | ||
namespace Native.UnityNativeWrapper |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix namespace to be CleverTapSDK.Native and remove it from the usings.
#if (!UNITY_IOS && !UNITY_ANDROID) || UNITY_EDITOR | ||
namespace Native.UnityNativeWrapper | ||
{ | ||
internal class UnityNativeMetadataResponseInterceptor : IUnityNativeResponseInterceptor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The response interceptor is unused, it is not added to the engine interceptors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it is better for the interceptor to not have a reference to the network engine. This creates a circular reference since the engine adds and holds the interceptors.
The options on top of my head are:
- The interceptor to set the
_i
and_j
in the preferences and the engine to use them from there. The interceptor can get the preference manager using the account id. - Do this logic directly in the network engine.
I believe it might be cleaner to go with 1. since you also already started with that approach.
_networkEngine.SetI(i); | ||
} | ||
} catch (Exception t) { | ||
// Ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not ignore the exception.
_networkEngine.SetJ(j); | ||
} | ||
} catch (Exception t) { | ||
// Ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not ignore the exception.
Implemented _i and _j functionality