From ba304f004783c14853eba2d5f86493096f945993 Mon Sep 17 00:00:00 2001
From: Heiko Joerg Schick <info@schihei.de>
Date: Sat, 22 Mar 2025 15:57:53 +0100
Subject: [PATCH] Refactor SwapPositions component for better maintainability
 and performance

---
 src/taskpane/components/SwapPositions.tsx | 151 +++++++++++++++++-----
 1 file changed, 118 insertions(+), 33 deletions(-)

diff --git a/src/taskpane/components/SwapPositions.tsx b/src/taskpane/components/SwapPositions.tsx
index d249fc80..d5c5f52e 100644
--- a/src/taskpane/components/SwapPositions.tsx
+++ b/src/taskpane/components/SwapPositions.tsx
@@ -3,49 +3,134 @@ import { ArrowSwapRegular } from "@fluentui/react-icons";
 import { useStatusContext } from "./App";
 import { useCommonStyles } from "./commonStyles";
 import ActionButton from "./ActionButton";
+import { getErrorMessage } from "../types/office-types";
+
+// Position properties to load and swap
+const POSITION_PROPERTIES = ["left", "top"];
+
+// Shape position data interface
+interface ShapePosition {
+  left: number;
+  top: number;
+}
 
 export const SwapPositions: React.FC = () => {
   const styles = useCommonStyles();
   const { setStatusMessage, setStatusType } = useStatusContext();
 
-  const swapPositionsOfTwoSelectedObjects = async () => {
-    await PowerPoint.run(async (context) => {
-      // Get the selected shapes
-      const shapes = context.presentation.getSelectedShapes();
-      shapes.load("items/count");
-      await context.sync();
+  /**
+   * Validates that exactly two shapes are selected
+   * @param shapes The collection of selected shapes
+   * @returns True if validation passes, false otherwise
+   */
+  const validateShapeSelection = (shapes: PowerPoint.ShapeScopedCollection): boolean => {
+    if (shapes.items.length !== 2) {
+      setStatusMessage("Please select exactly two shapes to swap their positions.");
+      setStatusType("warning");
+      return false;
+    }
+    return true;
+  };
 
-      // Check if exactly two shapes are selected
-      if (shapes.items.length !== 2) {
-        setStatusMessage("Please select exactly two shapes to swap their positions.");
-        setStatusType("warning");
-        return;
-      }
+  /**
+   * Loads position properties from the shapes
+   * @param shape1 The first shape
+   * @param shape2 The second shape
+   * @param context The PowerPoint request context
+   * @returns The loaded shapes
+   */
+  const loadShapePositions = async (
+    shape1: PowerPoint.Shape,
+    shape2: PowerPoint.Shape,
+    context: PowerPoint.RequestContext
+  ): Promise<[PowerPoint.Shape, PowerPoint.Shape]> => {
+    // Load position properties for both shapes in a single batch
+    shape1.load(POSITION_PROPERTIES);
+    shape2.load(POSITION_PROPERTIES);
+    await context.sync();
+    return [shape1, shape2];
+  };
 
-      // Get the two shapes
-      const shapeObj1 = shapes.items[0];
-      const shapeObj2 = shapes.items[1];
-      
-      // Load position properties
-      shapeObj1.load("left,top");
-      shapeObj2.load("left,top");
-      await context.sync();
-      
+  /**
+   * Swaps the positions of two shapes
+   * @param shape1 The first shape
+   * @param shape2 The second shape
+   * @returns True if the swap was successful
+   */
+  const swapPositions = (
+    shape1: PowerPoint.Shape,
+    shape2: PowerPoint.Shape
+  ): boolean => {
+    try {
       // Store the position of the first shape
-      const tempLeft = shapeObj1.left;
-      const tempTop = shapeObj1.top;
+      const tempPosition: ShapePosition = {
+        left: shape1.left,
+        top: shape1.top
+      };
       
       // Swap positions
-      shapeObj1.left = shapeObj2.left;
-      shapeObj1.top = shapeObj2.top;
-      shapeObj2.left = tempLeft;
-      shapeObj2.top = tempTop;
+      shape1.left = shape2.left;
+      shape1.top = shape2.top;
+      shape2.left = tempPosition.left;
+      shape2.top = tempPosition.top;
       
-      await context.sync();
-      
-      setStatusMessage("Positions of the two shapes have been swapped successfully.");
-      setStatusType("success");
-    });
+      return true;
+    } catch (err) {
+      console.error("Error swapping positions:", getErrorMessage(err));
+      return false;
+    }
+  };
+
+  /**
+   * Generates an appropriate status message based on the results
+   * @param success Whether the swap was successful
+   * @returns The formatted status message
+   */
+  const generateStatusMessage = (success: boolean): string => {
+    return success
+      ? "Positions of the two shapes have been swapped successfully."
+      : "Failed to swap positions. Please try again with different shapes.";
+  };
+
+  /**
+   * Main function to swap positions of two selected shapes
+   */
+  const swapPositionsOfTwoSelectedObjects = async (): Promise<void> => {
+    try {
+      await PowerPoint.run(async (context) => {
+        // Get the selected shapes
+        const shapes = context.presentation.getSelectedShapes();
+        shapes.load("items");
+        await context.sync();
+
+        // Validate shape selection
+        if (!validateShapeSelection(shapes)) {
+          return;
+        }
+
+        // Get the two shapes
+        const [shape1, shape2] = await loadShapePositions(
+          shapes.items[0],
+          shapes.items[1],
+          context
+        );
+        
+        // Swap positions
+        const swapSuccess = swapPositions(shape1, shape2);
+        
+        // Sync changes to PowerPoint
+        await context.sync();
+        
+        // Update status message
+        const statusMessage = generateStatusMessage(swapSuccess);
+        setStatusMessage(statusMessage);
+        setStatusType(swapSuccess ? "success" : "error");
+      });
+    } catch (error) {
+      console.error("Error in swapPositionsOfTwoSelectedObjects:", getErrorMessage(error));
+      setStatusMessage(`Error: ${getErrorMessage(error)}`);
+      setStatusType("error");
+    }
   };
 
   return (
@@ -61,4 +146,4 @@ export const SwapPositions: React.FC = () => {
   );
 };
 
-export default SwapPositions;
\ No newline at end of file
+export default SwapPositions;