Как отрефакторить с методами?

Question

[Mister_krid]

Программа рисует неправильный треугольник, нужно отрефакторить код я изменил название переменных на понятные и т.д. определенная часть кодa рисует стороны. Я пытался эту чать выделить в методы void но тогда все начиналось рисоваться неправильно. Как это красиво сделать? Честно не понимаю в чем проблема (скину код где эта часть мною не тронута)

class Risovatel
    {
        static float x, y;
        static Graphics graphics;

        public static void Initialization(Graphics newGraphics)
        {
            graphics = newGraphics;
            graphics.SmoothingMode = SmoothingMode.None;
            graphics.Clear(Color.Black);
        }

        public static void SetPosition(float x0, float y0)
        {
            x = x0;
            y = y0;
        }

        public static void MakeIt(Pen pen, double isLong, double angle)
        {
            //Делает шаг длиной dlina в направлении ugol и рисует пройденную траекторию
            var x1 = (float)(x + isLong * Math.Cos(angle));
            var y1 = (float)(y + isLong * Math.Sin(angle));
            graphics.DrawLine(pen, x, y, x1, y1);
            x = x1;
            y = y1;
        }

        public static void Change(double isLong, double angle)
        {
            x = (float)(x + isLong * Math.Cos(angle));
            y = (float)(y + isLong * Math.Sin(angle));
        }
    }

    public class ImpossibleSquare
    {

        public static void Draw(int width, int height, double angleOfRotation, Graphics graphics)
        {

            Risovatel.Initialization(graphics);

            var min = Math.Min(width, height);
            var diagonalLength = Math.Sqrt(2) * (min * 0.375f + min * 0.04f) / 2;
            var x0 = (float)(diagonalLength * Math.Cos(Math.PI / 4 + Math.PI)) + width / 2f;
            var y0 = (float)(diagonalLength * Math.Sin(Math.PI / 4 + Math.PI)) + height / 2f;

            Risovatel.SetPosition(x0, y0);

            //Рисуем 1-ую сторону
            Risovatel.MakeIt(Pens.Yellow, min * 0.375f, 0);
            Risovatel.MakeIt(Pens.Yellow, min * 0.04f * Math.Sqrt(2), Math.PI / 4);
            Risovatel.MakeIt(Pens.Yellow, min * 0.375f, Math.PI);
            Risovatel.MakeIt(Pens.Yellow, min * 0.375f - min * 0.04f, Math.PI / 2);

            Risovatel.Change(min * 0.04f, -Math.PI);
            Risovatel.Change(min * 0.04f * Math.Sqrt(2), 3 * Math.PI / 4);

            //Рисуем 2-ую сторону
            Risovatel.MakeIt(Pens.Yellow, min * 0.375f, -Math.PI / 2);
            Risovatel.MakeIt(Pens.Yellow, min * 0.04f * Math.Sqrt(2), -Math.PI / 2 + Math.PI / 4);
            Risovatel.MakeIt(Pens.Yellow, min * 0.375f, -Math.PI / 2 + Math.PI);
            Risovatel.MakeIt(Pens.Yellow, min * 0.375f - min * 0.04f, -Math.PI / 2 + Math.PI / 2);

            Risovatel.Change(min * 0.04f, -Math.PI / 2 - Math.PI);
            Risovatel.Change(min * 0.04f * Math.Sqrt(2), -Math.PI / 2 + 3 * Math.PI / 4);

            //Рисуем 3-ю сторону
            Risovatel.MakeIt(Pens.Yellow, min * 0.375f, Math.PI);
            Risovatel.MakeIt(Pens.Yellow, min * 0.04f * Math.Sqrt(2), Math.PI + Math.PI / 4);
            Risovatel.MakeIt(Pens.Yellow, min * 0.375f, Math.PI + Math.PI);
            Risovatel.MakeIt(Pens.Yellow, min * 0.375f - min * 0.04f, Math.PI + Math.PI / 2);

            Risovatel.Change(min * 0.04f, Math.PI - Math.PI);
            Risovatel.Change(min * 0.04f * Math.Sqrt(2), Math.PI + 3 * Math.PI / 4);

            //Рисуем 4-ую сторону
            Risovatel.MakeIt(Pens.Yellow, min * 0.375f, Math.PI / 2);
            Risovatel.MakeIt(Pens.Yellow, min * 0.04f * Math.Sqrt(2), Math.PI / 2 + Math.PI / 4);
            Risovatel.MakeIt(Pens.Yellow, min * 0.375f, Math.PI / 2 + Math.PI);
            Risovatel.MakeIt(Pens.Yellow, min * 0.375f - min * 0.04f, Math.PI / 2 + Math.PI / 2);

            Risovatel.Change(min * 0.04f, Math.PI / 2 - Math.PI);
            Risovatel.Change(min * 0.04f * Math.Sqrt(2), Math.PI / 2 + 3 * Math.PI / 4);
        }
}

Там еще отдельный класс есть вроде как для сборки всего этого, но думаю его кидать не нужно

Answer 1

[Василий Банников]

У тебя тут четыре куска кода, которые рисуют стороны этого четырёхугольника.
1. Вынеси отрисовку каждой стороны в отдельный метод, чтобы не писать эти комменты.
2. Обобщи эти методы, чтобы у тебя только в одном месте было

var pen = Pens.Yellow;
Risovatel.MakeIt(pen, ???, ???);
Risovatel.MakeIt(pen, ???, ???);
Risovatel.MakeIt(pen, ???, ???);
Risovatel.MakeIt(pen, ???, ???);

Risovatel.Change(???, ???);
Risovatel.Change(???, ???);

Я хз, что за Risovatel, что за MakeIt, что за Change, так что параметры я заменил на вопросительные знаки.
(а блин, тут же дан его код) - тогда надо переименовать эти методы и их параметры. Делать это г статическим нет никакого смысла.

Ну и названия переменных всё ещё ничего не говорят. min - это видимо длина стороны квадрата, если я правильно понял.

Answer 2

[Mister_krid]

Категорически извиняюсь, но почему то сейчас, когда я распределил по методам все сработало, хотя вчера делая тоже самое (я собственно вчерашний код и запустил) получалась фигня. И кст я попробовал сделать не статическим, ничего не вышло т.к. пришлось делать ВСЕ не статическим, а в первом файле метод Drow началась запрашивать ссылку на объект (я про это почитал возможно класс ImpossibleSquare статический)Если что, вот код второго файла, что бы не путать

using System;
using System.Drawing;
using NUnit.Framework;
using RefactorMe.Properties;

namespace RefactorMe
{
    [TestFixture]
    public class DrawingProgram_Tests
    {
        [Test]
        public void DrawExpectedImage()
        {
            const int width = 800;
            const int height = 600;
            var actual = new Bitmap(width, height);
            ImpossibleSquare.Draw(width, height, 0, Graphics.FromImage(actual));
            //actual.Save(TestContext.CurrentContext.TestDirectory + "/expected-image.bmp");
            AssertImageEquals(Resources.expected_image, actual);
        }

        public void AssertImageEquals(Bitmap expected, Bitmap actual)
        {
            var diffCount = 0.0;

            for (var x = 0; x < expected.Width; x++)
            {
                for (var y = 0; y < expected.Height; y++)
                {
                    Color expectedPixel = expected.GetPixel(x, y);
                    Color actualPixel = actual.GetPixel(x, y);
                    if (Math.Abs(expectedPixel.GetBrightness() - actualPixel.GetBrightness()) > 0.1)
                        diffCount++;
                }
            }

            // Изображение, которое рисует метод Draw, должно в точности совпадать с изображением expected-image
            Assert.Less(diffCount, 100);
        }
    }
}

что бы не путать.