Fixing a possible null pointer exception and removing unnecessary nullability for settings (#99)

Author: MichaelGHSeg

Analytics-CSharp/Segment/Analytics/Analytics.cs

@@ -176,9 +176,9 @@ public virtual void Reset()
         /// it's not recommended to be used in async method.
         /// </summary>
         /// <returns>Instance of <see cref="Settings"/></returns>
-        public virtual Settings? Settings()
+        public virtual Settings Settings()
         {
-            Task<Settings?> task = SettingsAsync();
+            Task<Settings> task = SettingsAsync();
             task.Wait();
             return task.Result;
         }
@@ -187,20 +187,10 @@ public virtual void Reset()
         /// Retrieve the settings.
         /// </summary>
         /// <returns>Instance of <see cref="Settings"/></returns>
-        public virtual async Task<Settings?> SettingsAsync()
+        public virtual async Task<Settings> SettingsAsync()
         {
-            Settings? returnSettings = null;
-            IState system = await Store.CurrentState<System>();
-
-            // I don't understand this as Store.CurrentState will at worst return a default(TState) which will be a default(System) in this case, hence this cast will always go through?
-            // System is a struct, so the default is an empty instance, not null
-            // might as well write /* if (true) */
-            if (system is System convertedSystem)
-            {
-                returnSettings = convertedSystem._settings;
-            }
-
-            return returnSettings;
+            System system = await Store.CurrentState<System>();
+            return system._settings;
         }
 
         #endregion

Analytics-CSharp/Segment/Analytics/Timeline.cs

@@ -160,26 +160,22 @@ internal void Add(Plugin plugin)
             Analytics analytics = plugin.Analytics;
             analytics.AnalyticsScope.Launch(analytics.AnalyticsDispatcher, async () =>
             {
-                Settings? settings = await plugin.Analytics.SettingsAsync();
-
-                // This is basically never null, look at the comment in SettingsAsync
-                if (settings == null)
-                {
-                    return;
-                }
-
+                Settings settings = await plugin.Analytics.SettingsAsync();
                 // Fetch system afterwards for a minuscule but cool performance gain
                 System system = await analytics.Store.CurrentState<System>();
 
+                // Don't initialize unless we have updated settings from the web.
+                // CheckSettings will initialize everything added before then, so wait until other inits have happened.
                 // Check for nullability because CurrentState returns default(IState) which could make the .Count throw a NullReferenceException
                 if (system._initializedPlugins != null && system._initializedPlugins.Count > 0)
                 {
                     await analytics.Store.Dispatch<System.AddInitializedPluginAction, System>(new System.AddInitializedPluginAction(new HashSet<int>{plugin.GetHashCode()}));
-                    plugin.Update(settings.Value, UpdateType.Initial);
+                    plugin.Update(settings, UpdateType.Initial);
                 }
             });
         }
 
+
         internal void Remove(Plugin plugin) => _plugins.RemoveAll(tempPlugin => tempPlugin == plugin);
 
         internal RawEvent Execute(RawEvent incomingEvent)