RefactorMCP catalog

Consolidate Duplicate Conditional Fragments

Moves statements that every branch of a conditional starts or ends with out of the conditional, so they are written once.

This is a primitive rather than a recipe: moving a statement out of every branch at once is the smallest step that preserves behaviour, since taking it out of one branch alone changes what that branch does.

Target

The first if of the chain, by caret: "target": { "file": "Deal.cs", "caret": "marker" }.

Precondition

  • The caret is on an if statement in a block or switch section, not on an else if.
  • The chain ends with an else, so that some branch always runs. Without one, a moved statement would also run when no branch does.
  • Every branch starts or ends with the same statement, compared as code regardless of comments and layout.
  • A statement moved after the conditional uses no local that its branch declares.
  • A statement moved before the conditional runs before the conditions rather than after them, so the conditions have no side effects, read no local the statement assigns, and, when the statement may change other state (it calls a method, creates an object, or assigns a field or property), read no field, property or method.
  • The result compiles.

Transformation

  • The longest run of statements every branch ends with is removed from each branch and written once after the conditional, set apart from it by a blank line.
  • The longest run of the remaining statements every branch starts with is removed from each branch and written once before the conditional, which is then set apart from them by a blank line.
  • A branch left with nothing keeps its empty braces.

Preserved

  • The behaviour of every path: each moved statement runs exactly when it ran in whichever branch was taken.
  • The comments of the first branch's statements move with them; the other branches' copies go with their comments.

Limitations

  • switch statements are not covered, only if / else chains.
  • A branch left empty is not removed, and the condition is not inverted to drop an empty then branch.
  • A call in a moved-before statement is assumed to change state, so a condition that reads a field or property keeps it in the branches even when the call would not affect it.

Error codes

CodeMeaning
not-an-ifthe caret is not on an if statement
not-in-blockthe if is not a statement of a block or switch section, such as an else if
no-final-elsethe chain does not end with an else
no-common-fragmentsthe branches neither start nor end with the same statement
uses-branch-locala statement every branch ends with uses a local its branch declares
condition-depends-on-fragmentthe conditions depend on what a statement every branch starts with does

Cases

·

Diffs show each file from before to after. /*[*/ … /*]*/ marks a selection and /*^*/ a caret; the runner removes them before the refactoring runs.

#condition-depends-on-fragment-rejected

A common first statement that changes what a condition reads cannot move before the condition

refusal
TargetCounter.cs, the /*^*/ caret
Refusescondition-depends-on-fragment; every file is left unchanged
Counter.csinput
1 using System;
2
3 namespace Metrics
4 {
5 public class Counter
6 {
7 private int _count;
8
9 public void Record()
10 {
11 /*^*/if (_count > 10)
12 {
13 _count++;
14 Console.WriteLine("many");
15 }
16 else
17 {
18 _count++;
19 Console.WriteLine("few");
20 }
21 }
22 }
23 }

View fixture on GitHub

#else-if-rejected

An else if is not a statement of a block, so nothing can move before or after it; start from the first if of the chain

refusal
TargetDeal.cs, the /*^*/ caret
Refusesnot-in-block; every file is left unchanged
Deal.csinput
1 using System;
2
3 namespace Shop
4 {
5 public class Deal
6 {
7 public void Price(decimal price, bool special, bool member)
8 {
9 if (special)
10 {
11 Console.WriteLine(price * 0.95m);
12 }
13 else /*^*/if (member)
14 {
15 Console.WriteLine(price * 0.98m);
16 Console.WriteLine("sent");
17 }
18 else
19 {
20 Console.WriteLine(price);
21 Console.WriteLine("sent");
22 }
23 }
24 }
25 }

View fixture on GitHub

#leading-fragment

A statement that starts every branch of an else if chain moves before it, since the conditions do not depend on it

success
TargetTicket.cs, the /*^*/ caret
Ticket.csmodified
11 using System.Collections.Generic;
22
33 namespace Support
44 {
55 public class Ticket
66 {
77 public string Describe(int code, List<string> log)
88 {
99 string label;
10− /*^*/if (code == 1)
10+ log.Add("describing");
11+
12+ if (code == 1)
1113 {
12− log.Add("describing");
1314 label = "open";
1415 }
1516 else if (code == 2)
1617 {
17− log.Add("describing");
1818 label = "pending";
1919 }
2020 else
2121 {
22− log.Add("describing");
2322 label = "closed";
2423 }
2524
2625 return label;
2726 }
2827 }
2928 }

View fixture on GitHub

#no-common-fragments-rejected

Branches that neither start nor end with the same statement have nothing to move

refusal
TargetDeal.cs, the /*^*/ caret
Refusesno-common-fragments; every file is left unchanged
Deal.csinput
1 using System;
2
3 namespace Shop
4 {
5 public class Deal
6 {
7 public void Price(decimal price, bool special)
8 {
9 /*^*/if (special)
10 {
11 Console.WriteLine("special");
12 Console.WriteLine(price * 0.95m);
13 }
14 else
15 {
16 Console.WriteLine(price * 0.98m);
17 Console.WriteLine("standard");
18 }
19 }
20 }
21 }

View fixture on GitHub

#no-final-else-rejected

Without a final else, a statement moved out of the branches would also run when no branch does

refusal
TargetDeal.cs, the /*^*/ caret
Refusesno-final-else; every file is left unchanged
Deal.csinput
1 using System;
2
3 namespace Shop
4 {
5 public class Deal
6 {
7 public void Price(decimal price, bool special, bool member)
8 {
9 /*^*/if (special)
10 {
11 Console.WriteLine(price * 0.95m);
12 Console.WriteLine("sent");
13 }
14 else if (member)
15 {
16 Console.WriteLine(price * 0.98m);
17 Console.WriteLine("sent");
18 }
19 }
20 }
21 }

View fixture on GitHub

#not-an-if-rejected

The caret must be on an if statement

refusal
TargetDeal.cs, the /*^*/ caret
Refusesnot-an-if; every file is left unchanged
Deal.csinput
1 using System;
2
3 namespace Shop
4 {
5 public class Deal
6 {
7 public void Price(decimal price, bool special)
8 {
9 /*^*/Console.WriteLine(price);
10 if (special)
11 {
12 Console.WriteLine("special");
13 }
14 else
15 {
16 Console.WriteLine("special");
17 }
18 }
19 }
20 }

View fixture on GitHub

#several-statements-and-comments

Several common statements move at each end, keeping the first branch's comments, and a return common to every branch moves too

success
TargetDeal.cs, the /*^*/ caret
Deal.csmodified
11 using System;
22
33 namespace Shop
44 {
55 public class Deal
66 {
77 public decimal Price(decimal price, bool special)
88 {
99 decimal total;
10− /*^*/if (special)
10+ // Every deal is logged.
11+ Console.WriteLine("pricing");
12+ Console.WriteLine(price);
13+
14+ if (special)
1115 {
12− // Every deal is logged.
13− Console.WriteLine("pricing");
14− Console.WriteLine(price);
1516 total = price * 0.95m;
16− // Tell the customer.
17− Console.WriteLine(total);
18− return total;
1917 }
2018 else
2119 {
22− Console.WriteLine("pricing");
23− Console.WriteLine(price);
2420 total = price * 0.98m;
25− Console.WriteLine(total);
26− return total;
2721 }
22+
23+ // Tell the customer.
24+ Console.WriteLine(total);
25+ return total;
2826 }
2927 }
3028 }

View fixture on GitHub

#trailing-fragment

A statement that ends every branch moves after the conditional

success
TargetDeal.cs, the /*^*/ caret
Deal.csmodified
11 namespace Shop
22 {
33 public class Deal
44 {
55 public decimal Total { get; private set; }
66
77 public void Price(decimal price, bool special)
88 {
9− /*^*/if (special)
9+ if (special)
1010 {
1111 Total = price * 0.95m;
12− Send();
1312 }
1413 else
1514 {
1615 Total = price * 0.98m;
17− Send();
1816 }
17+
18+ Send();
1919 }
2020
2121 private void Send()
2222 {
2323 }
2424 }
2525 }

View fixture on GitHub

#uses-branch-local-rejected

A common last statement that uses a local its branch declares cannot move out of the branch

refusal
TargetDeal.cs, the /*^*/ caret
Refusesuses-branch-local; every file is left unchanged
Deal.csinput
1 using System;
2
3 namespace Shop
4 {
5 public class Deal
6 {
7 public void Price(decimal price, bool special)
8 {
9 /*^*/if (special)
10 {
11 var total = price * 0.95m;
12 Console.WriteLine(total);
13 }
14 else
15 {
16 var total = price * 0.98m;
17 Console.WriteLine(total);
18 }
19 }
20 }
21 }

View fixture on GitHub