Where is the redundancy? 

(Everyday Code – instead of keeping our knowledge in a README.md let’s share it with the internet)

I think calling a method thrее times is redundant. But then again, you have to balance. Today’s article is about a code review that at the end took like a few hours in total of different discussions and I believe it is important. This kind of things take time. Failed builds, difficult specs.

The IRL example – Where is the redundancy? Is this DRY?

 @dataclass(frozen=True)
 class LdrawLine(abc.ABC):
+    default_x: ClassVar[float] = 23
+    default_y: ClassVar[float] = 45
+    default_z: ClassVar[float] = 0
     """
         This is the abstract class which every LdrawLine should implement.
     """
@@ -85,11 +88,19 @@ class LdrawLine(abc.ABC):
         elif line_args[1] == "STEP":
             line_param = _Step()
         elif line_args[1] == "ROTSTEP":
-            rostep_params = {
-                "rot_x": float(line_args[2]),
-                "rot_y": float(line_args[3]),
-                "rot_z": float(line_args[4])
-            }
+            if line_args[2] == "END":
+                rostep_params = {
+                    "rot_x": LdrawLine.default_x,
+                    "rot_y": LdrawLine.default_y,
+                    "rot_z": LdrawLine.default_z,
+                    "type": "ABS"
+                }
+            else:
+                rostep_params = {
+                    "rot_x": float(line_args[2]),
+                    "rot_y": float(line_args[3]),
+                    "rot_z": float(line_args[4])
+                }

This piece of code (along with a few other changes in the commit) were the root of a 2 hours discussion in the team. A spec failed because some things were int while we were expecting them to be float.

Calling ‘float’ three times like this is redundant

The reason I think like this is that if you have to change and would like to have a double value or an int value you would have to change the code in three places.

Probably e better solution would be:

+            if line_args[2] == "END":
+                rostep_params = {
+                    "rot_x": LdrawLine.default_x,
+                    "rot_y": LdrawLine.default_y,
+                    "rot_z": LdrawLine.default_z,
+                    "type": "ABS"
+                }
+            else:
+                rostep_params = {
+                    "rot_x": line_args[2],
+                    "rot_y": line_args[3],
+                    "rot_z": line_args[4]
+                }
                  # We are adding a loop to call the float
+                for key in ["rot_x", "rot_y", "rot_z"]:
+                     rotstep_params[key] = float(rotstep_params[key])

We call float only once at a single place. Now we have to deal with the fact that we have “rot_x” as a variable in two places, yes, that is true, but this could easily be extracted and we can iterate over the rostep_params values. But now we have consistency.

Is it harder to read? Probably it is a little harder. Instead of simple statements you now have a loop. So you lose something, but you gain something. A float function that is called at a single place.

What are we doing with these rotsteps?

ROTSTEP is a command in the LDR format. We support LDR for 3D building instructions. Here is one example with a FabBrix Monster that uses the LDR ROTSTEP as a command:

FabBRIX Monsters, Cthulhu in 3D building instructions