From 4b9fcc1b3df7f85785fec2efd1a3bad52ffc7a8f Mon Sep 17 00:00:00 2001 From: shahraj Date: Thu, 6 Apr 2023 21:18:31 -0300 Subject: [PATCH 1/2] Refactoring changes in doLayout, doLayoutOnlyVariables, doLayoutNormal --- .../builder/circuit/FragmentExpression.java | 73 ++++++++++++------- 1 file changed, 48 insertions(+), 25 deletions(-) diff --git a/src/main/java/de/neemann/digital/builder/circuit/FragmentExpression.java b/src/main/java/de/neemann/digital/builder/circuit/FragmentExpression.java index ce3335866..f375a1b9a 100644 --- a/src/main/java/de/neemann/digital/builder/circuit/FragmentExpression.java +++ b/src/main/java/de/neemann/digital/builder/circuit/FragmentExpression.java @@ -33,6 +33,9 @@ private static ArrayList createList(Fragment fragment) { return f; } + + + /** * Creates a new instance * @@ -56,10 +59,24 @@ public FragmentExpression(List frags, Fragment merger) { fragments.add(new FragmentHolder(fr)); } + //decomposed conditional in doLayoutNormal to new calculateMergerPositionY for improved readability + private int calculateMergerPositionY(int centerIndex) { + int y; + boolean isEvenNumberOfInputs = (fragments.size() & 1) == 0; + if (isEvenNumberOfInputs) { + int y1 = fragments.get(centerIndex - 1).fragment.getOutputs().get(0).y; + int y2 = fragments.get(centerIndex).fragment.getOutputs().get(0).y; + y = raster((y1 + y2) / 2) - centerIndex * SIZE; + } else { + y = fragments.get(centerIndex).fragment.getOutputs().get(0).y - centerIndex * SIZE; + } + return y; + } private Box doLayoutNormal() { ArrayList constExpr = new ArrayList<>(); int height = 0; - int width = 0; + //Renamed variable width to maxWidth for better readability //rename variable + int maxWidth = 0; for (FragmentHolder fr : fragments) { fr.fragment.setPos(new Vector(0, height)); if (isConst(fr.fragment)) @@ -68,8 +85,8 @@ private Box doLayoutNormal() { height += fr.box.getHeight(); int w = fr.box.getWidth(); - if (w > width) - width = w; + if (w > maxWidth) + maxWidth = w; height += SIZE * 2; } @@ -77,31 +94,23 @@ private Box doLayoutNormal() { Box mergerBox = merger.doLayout(); - width += (fragments.size() / 2 + 1) * SIZE; + maxWidth += (fragments.size() / 2 + 1) * SIZE; if (alignedWidth > 0) { - width = alignedWidth - mergerBox.getWidth(); + maxWidth = alignedWidth - mergerBox.getWidth(); } for (ConstExpression ce : constExpr) - ce.setXPos(width); + ce.setXPos(maxWidth); int centerIndex = fragments.size() / 2; - int y; - if ((fragments.size() & 1) == 0) { - // even number of inputs - int y1 = fragments.get(centerIndex - 1).fragment.getOutputs().get(0).y; - int y2 = fragments.get(centerIndex).fragment.getOutputs().get(0).y; - y = raster((y1 + y2) / 2) - centerIndex * SIZE; - } else { - // odd number of inputs - y = fragments.get(centerIndex).fragment.getOutputs().get(0).y - centerIndex * SIZE; - } - merger.setPos(new Vector(width, y)); + //decomposed conditional + int mergerPosY = calculateMergerPositionY(centerIndex); + merger.setPos(new Vector(maxWidth, mergerPosY)); - width += mergerBox.getWidth(); + maxWidth += mergerBox.getWidth(); - return new Box(width, Math.max(height, y + mergerBox.getHeight())); + return new Box(maxWidth, Math.max(height, mergerPosY + mergerBox.getHeight())); } private boolean isConst(Fragment fragment) { @@ -119,7 +128,11 @@ private Box doLayoutOnlyVariables() { Box mergerBox = merger.doLayout(); - merger.setPos(new Vector(xPos, 0)); + //introduced explaining variable + int mergerPosX = xPos; + Vector mergerPos = new Vector(mergerPosX, 0); + merger.setPos(mergerPos); + // merger.setPos(new Vector(xPos, 0)); Iterator in = merger.getInputs().iterator(); for (FragmentHolder fr : fragments) { @@ -130,13 +143,21 @@ private Box doLayoutOnlyVariables() { return new Box(mergerBox.getWidth() + SIZE, mergerBox.getHeight()); } - @Override - public Box doLayout() { - for (FragmentHolder fr : fragments) + //Extract Method + private boolean containsNonVariableFragments() { + for (FragmentHolder fr : fragments) { if (!(fr.fragment instanceof FragmentVariable)) { - normalLayout = true; - return doLayoutNormal(); + return true; } + } + return false; + } + @Override + public Box doLayout() { + if (containsNonVariableFragments()) { + normalLayout = true; + return doLayoutNormal(); + } normalLayout = false; return doLayoutOnlyVariables(); @@ -246,4 +267,6 @@ public V traverse(V v) { merger.traverse(v); return v; } + + } From 9f646d5ca3cc67378bf68a24fdfd7f1e522e4d54 Mon Sep 17 00:00:00 2001 From: shahraj Date: Thu, 6 Apr 2023 21:20:04 -0300 Subject: [PATCH 2/2] removed refactoring changes comments --- .../digital/builder/circuit/FragmentExpression.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/main/java/de/neemann/digital/builder/circuit/FragmentExpression.java b/src/main/java/de/neemann/digital/builder/circuit/FragmentExpression.java index f375a1b9a..38d7e4b9a 100644 --- a/src/main/java/de/neemann/digital/builder/circuit/FragmentExpression.java +++ b/src/main/java/de/neemann/digital/builder/circuit/FragmentExpression.java @@ -59,7 +59,6 @@ public FragmentExpression(List frags, Fragment merger) { fragments.add(new FragmentHolder(fr)); } - //decomposed conditional in doLayoutNormal to new calculateMergerPositionY for improved readability private int calculateMergerPositionY(int centerIndex) { int y; boolean isEvenNumberOfInputs = (fragments.size() & 1) == 0; @@ -75,7 +74,6 @@ private int calculateMergerPositionY(int centerIndex) { private Box doLayoutNormal() { ArrayList constExpr = new ArrayList<>(); int height = 0; - //Renamed variable width to maxWidth for better readability //rename variable int maxWidth = 0; for (FragmentHolder fr : fragments) { fr.fragment.setPos(new Vector(0, height)); @@ -104,7 +102,6 @@ private Box doLayoutNormal() { ce.setXPos(maxWidth); int centerIndex = fragments.size() / 2; - //decomposed conditional int mergerPosY = calculateMergerPositionY(centerIndex); merger.setPos(new Vector(maxWidth, mergerPosY)); @@ -128,11 +125,10 @@ private Box doLayoutOnlyVariables() { Box mergerBox = merger.doLayout(); - //introduced explaining variable + int mergerPosX = xPos; Vector mergerPos = new Vector(mergerPosX, 0); merger.setPos(mergerPos); - // merger.setPos(new Vector(xPos, 0)); Iterator in = merger.getInputs().iterator(); for (FragmentHolder fr : fragments) { @@ -143,7 +139,6 @@ private Box doLayoutOnlyVariables() { return new Box(mergerBox.getWidth() + SIZE, mergerBox.getHeight()); } - //Extract Method private boolean containsNonVariableFragments() { for (FragmentHolder fr : fragments) { if (!(fr.fragment instanceof FragmentVariable)) {