Skip to content

Commit 12d8d1b

Browse files
[FSSDK-11544] review update
1 parent f77205d commit 12d8d1b

File tree

5 files changed

+18
-21
lines changed

5 files changed

+18
-21
lines changed

OptimizelySDK.Tests/ProjectConfigTest.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1425,8 +1425,7 @@ public void TestGetHoldout_Integration()
14251425
Assert.AreEqual("global_holdout", globalHoldout.Key);
14261426

14271427
var invalidHoldout = datafileProjectConfig.GetHoldout("invalid_id");
1428-
Assert.IsNotNull(invalidHoldout);
1429-
Assert.AreEqual("", invalidHoldout.Id); // Dummy holdout has empty ID
1428+
Assert.IsNull(invalidHoldout);
14301429
}
14311430

14321431
[Test]
@@ -1461,8 +1460,7 @@ public void TestMissingHoldoutsField_BackwardCompatibility()
14611460
Assert.AreEqual(0, holdouts.Length);
14621461

14631462
var holdout = datafileProjectConfig.GetHoldout("any_id");
1464-
Assert.IsNotNull(holdout);
1465-
Assert.AreEqual("", holdout.Id); // Dummy holdout has empty ID
1463+
Assert.IsNull(holdout);
14661464
}
14671465

14681466
#endregion

OptimizelySDK/Config/DatafileProjectConfig.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,7 @@ public Rollout GetRolloutFromId(string rolloutId)
795795
/// Get the holdout from the ID
796796
/// </summary>
797797
/// <param name="holdoutId">ID for holdout</param>
798-
/// <returns>Holdout Entity corresponding to the holdout ID or a dummy entity if ID is invalid</returns>
798+
/// <returns>Holdout Entity corresponding to the holdout ID or null if ID is invalid</returns>
799799
public Holdout GetHoldout(string holdoutId)
800800
{
801801
#if NET35 || NET40
@@ -804,7 +804,7 @@ public Holdout GetHoldout(string holdoutId)
804804
if (string.IsNullOrWhiteSpace(holdoutId))
805805
#endif
806806
{
807-
return new Holdout();
807+
return null;
808808
}
809809

810810
if (_HoldoutIdMap.ContainsKey(holdoutId))
@@ -816,7 +816,7 @@ public Holdout GetHoldout(string holdoutId)
816816
Logger.Log(LogLevel.ERROR, message);
817817
ErrorHandler.HandleError(
818818
new InvalidExperimentException("Provided holdout is not in datafile."));
819-
return new Holdout();
819+
return null;
820820
}
821821

822822
/// <summary>

OptimizelySDK/Entity/Holdout.cs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,6 @@ namespace OptimizelySDK.Entity
2828
/// </summary>
2929
public class Holdout : IdKeyEntity, IExperimentCore
3030
{
31-
/// <summary>
32-
/// Constructor that initializes properties to avoid null values
33-
/// </summary>
34-
public Holdout()
35-
{
36-
Id = "";
37-
Key = "";
38-
}
39-
4031
/// <summary>
4132
/// Holdout status enumeration
4233
/// </summary>

OptimizelySDK/ProjectConfig.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ public interface ProjectConfig
322322
/// Get the holdout from the ID
323323
/// </summary>
324324
/// <param name="holdoutId">ID for holdout</param>
325-
/// <returns>Holdout Entity corresponding to the holdout ID or a dummy entity if ID is invalid</returns>
325+
/// <returns>Holdout Entity corresponding to the holdout ID or null if ID is invalid</returns>
326326
Holdout GetHoldout(string holdoutId);
327327

328328
/// <summary>

OptimizelySDK/Utils/HoldoutConfig.cs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,17 +120,25 @@ public List<Holdout> GetHoldoutsForFlag(string flagId)
120120
return _flagHoldoutCache[flagId];
121121

122122
var activeHoldouts = new List<Holdout>();
123-
124123
// Start with global holdouts, excluding any that are specifically excluded for this flag
125124
var excludedForFlag = _excludedHoldouts.ContainsKey(flagId) ? _excludedHoldouts[flagId] : new List<Holdout>();
126125

127-
foreach (var globalHoldout in _globalHoldouts)
126+
if (excludedForFlag.Count > 0)
128127
{
129-
if (!excludedForFlag.Contains(globalHoldout))
128+
// Only iterate if we have exclusions to check
129+
foreach (var globalHoldout in _globalHoldouts)
130130
{
131-
activeHoldouts.Add(globalHoldout);
131+
if (!excludedForFlag.Contains(globalHoldout))
132+
{
133+
activeHoldouts.Add(globalHoldout);
134+
}
132135
}
133136
}
137+
else
138+
{
139+
// No exclusions, add all global holdouts directly
140+
activeHoldouts.AddRange(_globalHoldouts);
141+
}
134142

135143
// Add included holdouts for this flag
136144
if (_includedHoldouts.ContainsKey(flagId))

0 commit comments

Comments
 (0)