New Guideline for Contributing - Duplicate code is BAD

[tig]

I'm posting this to help coach/advise, and to criticize existing CODE in the project. I am NOT criticizing any people who may have contributed to the code (I've made the same errors myself).

• https://www.techrepublic.com/article/in-praise-of-developers-who-delete-code/

In fixing #2358, I had to modify SetRelativeLayout. The code of that function, including private functions it calls, had a bunch of duplicate code (exact copy of code with the only difference being whether it was working on horiz vs vert coordinates). This made fixing this bug more challenging, but I viewed it as a fun challenge.

Once the bug was fixed, I then had to spend many hours and crazy effort updating Scenarios and Unit Tests to deal with the new "correct" behavior. The primary example is the three "Borders on xxx" Scenarios. Because these three Scenarios are copy/paste copies of each other, I not only had to find and fix the issues in one of them, but I had to do it 3 times.

I'm 100% confident the time that was saved simply creating copies of this code far exceeded the amount of time I spent doing the fix.

Duplicate code also contributes to fragility of the library. The Border code is full of literally hundreds of lines of code that look like this:

// Draw the upper Effect3D
				for (int r = frame.Y - drawMarginFrame - sumThickness.Top + effect3DOffset.Y;
					r >= 0 && r < frame.Y - drawMarginFrame - sumThickness.Top; r++) {
					for (int c = frame.X - drawMarginFrame - sumThickness.Left + effect3DOffset.X;
						c >= 0 && c < Math.Min (frame.Right + drawMarginFrame + sumThickness.Right + effect3DOffset.X, driver.Cols); c++) {

						AddRuneAt (driver, c, r, (Rune)driver.Contents [r, c, 0]);
					}
				}

				// Draw the left Effect3D
				for (int r = frame.Y - drawMarginFrame - sumThickness.Top + effect3DOffset.Y;
					r >= 0 && r < Math.Min (frame.Bottom + drawMarginFrame + sumThickness.Bottom + effect3DOffset.Y, driver.Rows); r++) {
					for (int c = frame.X - drawMarginFrame - sumThickness.Left + effect3DOffset.X;
						c >= 0 && c < frame.X - drawMarginFrame - sumThickness.Left; c++) {

						AddRuneAt (driver, c, r, (Rune)driver.Contents [r, c, 0]);
					}
				}

Please look at #2379 to see how I simplified SetRelativeLayout and, as a result, removed a significant amount of code from the project.

From now on, I will be much more hard-core in not merging PRs where I see obvious duplicated code. I will be updating https://github.com/gui-cs/Terminal.Gui/blob/9cfa78a0330f6ff05c9460d7064fd9167d90d60d/CONTRIBUTING.md regarding this.

Pro-tips:

• Blur your eyes when looking at a body of code. If there's an obvious repeating pattern, you can probably refactor to simplify.

• If two functions only differ based on a single aspect like horizontal/vertical, engineer a way to create one function.

• Any time you're tempted to copy/paste big chunks of code stop and think through what it would take to create an abstraction that would result in little or no duplicate code.

This guidance is relevant for:

• The library itself
• Scenarios
• Unit tests

Thanks for listening.