Remove unnecessary string.IsNullOrEmpty checks in Plan.cs

Summary: This commit simplifies the logic for setting step variables in the Plan class by removing redundant checks for empty strings. The variables.Get and State.Get methods already handle empty strings internally, so there is no need to check them again. This makes the code more readable and consistent.
microsoft · May 6, 2023 · 47009e3 · 47009e3
1 parent a1f436d
commit 47009e3
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 6 deletions.
diff --git a/dotnet/src/SemanticKernel.UnitTests/Planning/PlanTests.cs b/dotnet/src/SemanticKernel.UnitTests/Planning/PlanTests.cs
@@ -577,6 +577,47 @@ public async Task CanExecutePlanWithOneStepAndStateAsync()
         mockFunction.Verify(x => x.InvokeAsync(It.IsAny<SKContext>(), null, null, default), Times.Once);
     }
 
+    [Fact]
+    public async Task CanExecutePlanWithStateAsync()
+    {
+        // Arrange
+        var kernel = new Mock<IKernel>();
+        var log = new Mock<ILogger>();
+        var memory = new Mock<ISemanticTextMemory>();
+        var skills = new Mock<ISkillCollection>();
+
+        var returnContext = new SKContext(
+            new ContextVariables(),
+            memory.Object,
+            skills.Object,
+            log.Object
+        );
+
+        var mockFunction = new Mock<ISKFunction>();
+        mockFunction.Setup(x => x.InvokeAsync(It.IsAny<SKContext>(), null, null, default))
+            .Callback<SKContext, CompleteRequestSettings, ILogger, CancellationToken?>((c, s, l, ct) =>
+            {
+                c.Variables.Get("type", out var t);
+                returnContext.Variables.Update($"Here is a {t} about " + c.Variables.Input);
+            })
+            .Returns(() => Task.FromResult(returnContext));
+
+        var planStep = new Plan(mockFunction.Object);
+        planStep.Parameters.Set("type", string.Empty);
+        var plan = new Plan(string.Empty);
+        plan.AddSteps(planStep);
+        plan.State.Set("input", "Cleopatra");
+        plan.State.Set("type", "poem");
+
+        // Act
+        var result = await plan.InvokeAsync();
+
+        // Assert
+        Assert.NotNull(result);
+        Assert.Equal($"Here is a poem about Cleopatra", result.Result);
+        mockFunction.Verify(x => x.InvokeAsync(It.IsAny<SKContext>(), null, null, default), Times.Once);
+    }
+
     [Fact]
     public async Task CanExecutePlanWithCustomContextAsync()
     {

diff --git a/dotnet/src/SemanticKernel/Planning/Plan.cs b/dotnet/src/SemanticKernel/Planning/Plan.cs
@@ -512,11 +512,11 @@ private ContextVariables GetNextStepVariables(ContextVariables variables, Plan s
                 continue;
             }
 
-            if (variables.Get(param.Name, out var value) && !string.IsNullOrEmpty(value))
+            if (variables.Get(param.Name, out var value))
             {
                 stepVariables.Set(param.Name, value);
             }
-            else if (this.State.Get(param.Name, out value) && !string.IsNullOrEmpty(value))
+            else if (this.State.Get(param.Name, out value))
             {
                 stepVariables.Set(param.Name, value);
             }
@@ -531,19 +531,19 @@ private ContextVariables GetNextStepVariables(ContextVariables variables, Plan s
             }
 
             var expandedValue = this.ExpandFromVariables(variables, item.Value);
-            if (!string.IsNullOrEmpty(item.Value) && !expandedValue.Equals(item.Value, StringComparison.OrdinalIgnoreCase))
+            if (!expandedValue.Equals(item.Value, StringComparison.OrdinalIgnoreCase))
             {
                 stepVariables.Set(item.Key, expandedValue);
             }
-            else if (variables.Get(item.Key, out var value) && !string.IsNullOrEmpty(value))
+            else if (variables.Get(item.Key, out var value))
             {
                 stepVariables.Set(item.Key, value);
             }
-            else if (this.State.Get(item.Key, out value) && !string.IsNullOrEmpty(value))
+            else if (this.State.Get(item.Key, out value))
             {
                 stepVariables.Set(item.Key, value);
             }
-            else if (!string.IsNullOrEmpty(expandedValue) && !stepVariables.Get(item.Key, out var _))
+            else
             {
                 stepVariables.Set(item.Key, expandedValue);
             }