Если нельзя, но очень хочется, то нужно обязательно и ничего в мире не стоит того, чтобы делать из этого проблему!


Интересна Java? Кликай по ссылке и изучай!
Если тебе полезно что-то из того, чем я делюсь в своем блоге - можешь поделиться своими деньгами со мной.
с пожеланием
столько времени читатели провели на блоге - 
сейчас онлайн - 

вторник, 22 января 2013 г.

Сегодня суперкод - завтра говнокод

Сегодня знакомились с новой группой Java тренинга в КПИ. На этот раз был приглашен в качестве гостя, чем бесконечно рад. Но не об этом сейчас. А о том, что случилось на этой встрече. Цель собрания была познакомиться, сделать коллективно code review, после чего ребята самостоятельно могли бы общаться и решать поставленные задачи. В ходе code review мы зацепили тему Don't Repeat Yourself, а так же магических констант. Не сдержался - показал свой код 10 летней давности. Код все того же фрактального проводника, который писал во времена студенческие. Писал я его на Delphi7. В общем, я его просто тут оставлю и все станет понятно...
type
  TPal = array [0..511] of TColor;
  TSavePalRec = array [1..50] of record
    Color:TColor;
    X:Integer;
  end;
//---------------------------------------------------------------------------------------------
var
    PalProp:record
        mT, mW, mH, mW05:integer;
    end;
    ClkX, ClkY:integer;
    bMovePan, bDouble:boolean;
    SavePalRec:TSavePalRec;
    pan:array [1..100] of record
        x:integer;
        col:TColor;
    end;
    CountPan:integer;
    bDown:boolean;
    bIncDec:boolean;
    RedPos, GreenPos, BluePos:integer;
    RedCount, GreenCount, BlueCount:integer;
    ColArrR, ColArrG, ColArrB:array [0..1024] of byte;
    bMoveR, bMoveG, bMoveB:boolean;
    bmp:TBitMap;
//--------------------------------------------------------------------------------------------- 
 const NumbColor=19;
      ColorArray:array [0..NumbColor - 1] of TColor=($FFFFFF, $00FFFF, $FF00FF, $FFFF00, $0000FF, $FF0000, $00FF00, $C0FFFF, $FFC0FF, $FFFFC0, $C0C0FF, $FFC0C0, $C0FFC0, $C000FF, $00C0FF, $00FFC0, $C0FF00, $FFC000, $FF00C0);
//---------------------------------------------------------------------------------------------
function TForm2.CreateAndSortPanel(X:integer; bRepaint:boolean; cl:TColor):integer;
var i, j, tle, le:integer;
    col, tcol:TColor;
begin
    for i:=1 to CountPan-1 do // определяем после которого маркера будет создаваемый
        if (pan[i].x < X) and (X < pan[i+1].x) then break;

    if i = CountPan then Exit; // если после последнего то выходим

    Result:=i;

    if (pan[i+1].x - pan[i].x) < PalProp.mW+2 then Exit;  // если маркер некуда втиснуть между двумя ближайшими то выходим
    if ((pan[i].x + PalProp.mW+1) > X) or ((pan[i+1].x - PalProp.mW-1) < X) then Exit; // очень близко ставить маркер возле соседнего нельзя

    le:=pan[i+1].x;
    col:=pan[i+1].Col;
    pan[i+1].x:=X;
    pan[i+1].Col:=cl;

    for j:=i+2 to CountPan do begin
        tle:=pan[j].x;
        tcol:=pan[j].Col;
        pan[j].x:=le;
        pan[j].Col:=col;
        le:=tle;
        col:=tcol;
    end;

    CreatePanel(pb.Width);

    CurrentPan:=i+1;

    pan[CountPan].Col:=col;
    if bRepaint then begin
        RepaintPan(CurrentPan);
        RepaintPalitra(i, i+2);
    end;
end;
//---------------------------------------------------------------------------------------------
procedure TForm2.RepaintPan(Num: Integer);
var x1, x2, y1, y2:integer;
begin
    if Num = 1
        then x1:=0
        else x1:=pan[Num-1].x + PalProp.mW05+1;
    if Num = CountPan
        then x2:=pb.Width
        else x2:=pan[Num+1].x - PalProp.mW05-1;
    y1:=PalProp.mT + 1;
    y2:=PalProp.mT + PalProp.mH + 1;

    bmp.Canvas.Pen.Color:=clBtnFace;
    bmp.Canvas.Brush.Color:=clBtnFace;
    bmp.Canvas.Rectangle(x1, y1, x2, y2);

//    if Num = CurrentPan
//        then bmp.Canvas.Pen.Color:=clRed
//        else bmp.Canvas.Pen.Color:=clBlack;
    bmp.Canvas.Pen.Color:=clBlack;
    bmp.Canvas.Brush.Color:=pan[Num].Col;
    bmp.Canvas.Rectangle(pan[Num].x - PalProp.mW05, y1, pan[Num].x + PalProp.mW05, y2);

    x2:=x2 - x1;
    y2:=PalProp.mH+2;
    BitBlt(pb.Canvas.Handle, x1, y1, x2, y2, bmp.Canvas.Handle, x1, y1, SRCCOPY);
end;
//---------------------------------------------------------------------------------------------
procedure TForm2.RepaintAllPan;
var i:integer;
begin
    bmp.Canvas.Pen.Color:=clBtnFace;
    bmp.Canvas.Brush.Color:=clBtnFace;
    bmp.Canvas.Rectangle(0,
                         PalProp.mT + 1,
                         pb.Width,
                         PalProp.mT + PalProp.mW + 1);
    for i:=1 to CountPan do begin
//        if i = CurrentPan
//            then bmp.Canvas.Pen.Color:=clRed
//            else bmp.Canvas.Pen.Color:=clBlack;
        bmp.Canvas.Pen.Color:=clBlack;
        bmp.Canvas.Brush.Color:=pan[i].Col;
        bmp.Canvas.Rectangle(pan[i].x - PalProp.mW05,
                             PalProp.mT + 1,
                             pan[i].x + PalProp.mW05,
                             PalProp.mT + PalProp.mW + 1);
    end;
    BitBlt(pb.Canvas.Handle,
           0,PalProp.mT + 1,
           pb.Width, PalProp.mH,
           bmp.Canvas.Handle,
           0, PalProp.mT + 1, SRCCOPY);
end;
//----------------------------------------------------------------------------------------------------------------------------------------------------------
procedure TForm2.RepaintPalitra(Num1, Num2:integer); // перерисовка
var i, j, a, b:integer;
begin
    if Num1 < 1 then Num1:=1;  // проверка выхода за пределы (они есть в Panels_onDblClick)
    if Num2 > CountPan then Num2:=CountPan;

    b:=pan[Num1].x;
    for i:=Num1 to Num2-1 do begin
        a:=pan[i+1].x - pan[i].x;
        for j:=0 to a do begin
            bmp.Canvas.Pen.Color:=ColorChange(pan[i].Col, pan[i+1].Col, a, j);
            bmp.Canvas.MoveTo(b+j, 0);
            bmp.Canvas.LineTo(b+j, PalProp.mT);
        end;
        b:=b+a;
    end;
    BitBlt(pb.Canvas.Handle,
           pan[Num1].x, 0,
           pan[Num2].x - pan[Num1].x, PalProp.mT,
           bmp.Canvas.Handle,
           pan[Num1].x, 0, SRCCOPY);
    if not Form1.bFirstLoad then SaveChangeToMainForm;
end;
//---------------------------------------------------------------------------------------------
procedure TForm2.SaveChangeToMainForm;
var i:integer;
begin
    for i:=0 to 511 do
        Form1.pal[i]:=bmp.Canvas.Pixels[i, 1]; // заганяем новую палитру
    Form1.DrawFromArray(Form1.FractArr, Rect(0, 0, Form1.pb.Width - 1, Form1.pb.Height - 1), Form1.Bmp);
    Form1.pbPaint(self);
end;
//---------------------------------------------------------------------------------------------
procedure TForm2.RandomPalitra;
var i, j, k:integer;
    r:real;
begin
    if cb1.Checked
        then begin
            Randomize;
            CountPan:=2;
            pan[1].x:=0;
            pan[1].col:=RGB(Random(256), Random(256), Random(256));
            pan[2].x:=pb.Width;
            pan[2].col:=pan[1].col;

            k:=Random(10) + 3;
            r:=pb.Width / (k + 1);
            CreateAndSortPanel(9, false);
            for i:=1 to k do begin
                j:=Round(i*r);
                if Random(2) = 1 then CreateAndSortPanel(j - PalProp.mW - 1, false);
                CreateAndSortPanel(j, false, ColorArray[Random(NumbColor - 1)]);
                if Random(2) = 1 then CreateAndSortPanel(j + PalProp.mW + 1, false);
            end;
            CreateAndSortPanel(pb.Width - PalProp.mW - 1, false);
            RepaintAllPan;
            RepaintPalitra(1, CountPan);
    end
    else begin
        if Form1.smnPerelyv.Checked then Form1.smnPerelyvClick(Self);
        RedCount:=Random(9)+1;
        GreenCount:=Random(9)+1;
        BlueCount:=Random(9)+1;
        RedPos:=Random(511)+1;
        GreenPos:=Random(511)+1;
        BluePos:=Random(511)+1;
        bMoveR:=Random(2)=1;
        bMoveG:=Random(2)=1;
        bMoveB:=Random(2)=1;
        ChangeParam;
    end;
end;
//---------------------------------------------------------------------------------------------
function ColorChange(col1, col2: TColor; R, i: real): TColor;
var cr, cg, cb:real;
    cr1, cg1, cb1:byte;
    cr2, cg2, cb2:byte;
    dcr, dcg, dcb:byte;
begin
    cr1:=GetRvalue(col1);   cr2:=GetRvalue(col2);
    cg1:=GetGvalue(col1);   cg2:=GetGvalue(col2);
    cb1:=GetBvalue(col1);   cb2:=GetBvalue(col2);
    dcr:=abs(cr1 - cr2);
    dcg:=abs(cg1 - cg2);
    dcb:=abs(cb1 - cb2);
    if cr1 <> cr2
        then begin
            if cr1 < cr2 then cr:=cr1 + dcr*i/R;
            if cr1 > cr2 then cr:=cr1 - dcr*i/R;
        end
        else cr:=cr1;
    if cg1 <> cg2
        then begin
            if cg1 < cg2 then cg:=cg1 + dcg*i/R;
            if cg1 > cg2 then cg:=cg1 - dcg*i/R;
        end
        else cg:=cg1;
    if cb1 <> cb2
        then begin
            if cb1 < cb2 then cb:=cb1 + dcb*i/R;
            if cb1 > cb2 then cb:=cb1 - dcb*i/R;
        end
        else cb:=cb1;
    Result:=RGB(Round(cr), Round(cg), Round(cb));
end;
Выложил я только 1/10 часть кода, которая отвечает за формирование рендомной палитры, по которой потом отрисуется фарктал в красивых, пестрых красках. Вчера он мне понадобился. Я хотел сделать то же но на Java. Я хотел приделать эту же палитру к своему недавнему минипроектику "Рисуем фракталы на Java".

Что я хотел донести ребятам - так это те эмоции, которые я получал когда вчера портировал этот код на Java, а потом отлаживал его. На все про все 2 часа времени. Хотя это могло занять 10 минут, если бы код был поддерживаем. Я глядя в код, не мог сразу понять что он делает. Не мог вспомнить что я хранил в переменных с хитромудрным названием. Я знал лишь только, что он работает и работает так как надо. Так же я знал, что написать такой же с нуля у меня займет существенно больше времени, чем портирование существующего. И я взялся за рефакторинг.

Первым делом я написал некоторое подобие тестов и сделал механическое превращение (портирование) кода Pascal в Java. Работа не сложная, но требует внимания, ведь чем больше я ошибок тут сделаю, тем больше потом времени потрачу на отладку. 30 минут и код компилировался в Idea. Первая зеленая полоса! Я даже на радостях закоммитился, хотя понимал, что ничего оно не работает.

Потом был небольшой тестовый класс, который генерит палитру, рисует ее в bmp и сохраняет в BMP файл. А я уже чекаю результат. Конечно же он рисовал не то, что требовалось. Иногда случается, что код работает сразу, но я рад что это было не так. Пришлось его отлаживать.

Еще пол часа внимательного рефакторинга, и постепенного разтуманивания прдназначения методов и переменных. Потом полировка и тюнинг под новые нужны. Тоже пол часа. Итого я получил вот это
public class RandomPalette implements Palette {

    class Marker {
        int x;
        int color;

        public Marker(int x, int color) {
            this.x = x;
            this.color = color;
        }
    }

    private static final int MW = 8; // ширина маркера
    private static final int[] colorArray = new int[]{
            0xFFFFFF, 0x00FFFF, 0xFF00FF, 0xFFFF00, 0x0000FF, 0xFF0000, 0x00FF00,
            0xC0FFFF, 0xFFC0FF, 0xFFFFC0, 0xC0C0FF, 0xFFC0C0, 0xC0FFC0, 0xC000FF,
            0x00C0FF, 0x00FFC0, 0xC0FF00, 0xFFC000, 0xFF00C0};

    private List<Marker> markers = new LinkedList<Marker>();
    private int[] palette;

    public RandomPalette(int size) {
        palette = new int[size];

        int color = getRandomColor();
        markers.add(new Marker(0, color));
        markers.add(new Marker(size, color));

        int count = random(size / (MW * 3)) + 3;
        double r = size / (count + 1);

        addBlackMarker(MW + 1);
        for (int i = 1; i <= count; i++) {
            int j = (int) (i * r);
            if (yesOrNo()) {
                addBlackMarker(j - MW - 1);
            }
            addMarker(j, getRandomColor());
            if (yesOrNo()) {
                addBlackMarker(j + MW + 1);
            }
        }
        addBlackMarker(size - MW - 1);

        calculatePalette();
    }

    private int getRandomColor() {
        return colorArray[random(colorArray.length)];
    }

    private boolean yesOrNo() {
        return random(2) == 1;
    }

    private void addBlackMarker(int x) {
        addMarker(x, 0);
    }

    private void calculatePalette() {
        int x = markers.get(0).x;
        for (int i = 0; i < markers.size() - 1; i++) {
            int length = markers.get(i + 1).x - markers.get(i).x;
            for (int dx = 0; dx < length; dx++) {
                palette[x + dx] = colorChange(markers.get(i).color, markers.get(i + 1).color, length, dx);
            }
            x = x + length;
        }
        palette[0] = 0;
    }

    private int colorChange(int from, int to, double len, double x) {
        double red = change(getR(from), getR(to), len, x);
        double green = change(getG(from), getG(to), len, x);
        double blue = change(getB(from), getB(to), len, x);

        return rgb((int) red, (int) green, (int) blue);
    }

    private double change(double from, double to, double len, double x) {
        if (from == to) {
            return from;
        }

        double delta = Math.abs(from - to) * x / len;

        if (from < to) {
            return from + delta;
        } else {
            return from - delta;
        }
    }

    private int getR(int col) {
        return (col & 0x0000FF);
    }

    private int getG(int col) {
        return (col & 0x00FF00) >>> 8;
    }

    private int getB(int col) {
        return (col & 0xFF0000) >>> 16;
    }

    private int rgb(int r, int g, int b) {
        return (r) | (g << 8) | (b << 16);
    }

    private void addMarker(int x, int color) {
        // определяем после которого маркера будет создаваемый
        int index;
        for (index = 0; index < markers.size(); index++) {
            if ((markers.get(index).x < x) && (x < markers.get(index + 1).x)) {
                break;
            }
        }

        // если после последнего то выходим
        if (index == markers.size()) {
            return;
        }

        // если маркер некуда втиснуть между двумя ближайшими то выходим
        if (markers.get(index + 1).x - markers.get(index).x < MW + 2) {
            return;
        }

        // очень близко ставить маркер возле соседнего нельзя
        if ((markers.get(index).x + MW + 1 > x) || (markers.get(index + 1).x - MW - 1 < x)) {
            return;
        }

        markers.add(index + 1, new Marker(x, color));
    }

    private int random(int n) {
        return new Random().nextInt(n);
    }

    @Override
    public int getColor(int r) {
        return palette[r];
    }

    @Override
    public int getSize() {
        return palette.length;
    }
}
И я более чем уверен, что глядя на этот код спустя некоторое время я буду считать его говнокодом. Если этого не случится – значит. Не даром, если заметил, я этот код назвал «это». «Это» только оно сейчас работает… Вообще считаю, стоит относиться к своему коду не как к произведению искусства, а как к продукту жизнедеятельности, тому что могло быть чуточку лучше, раз уж появилось на этот свет.

Что есть code review? Это всего лишь озвучивания той дельты, которая существует у двух специалистов в их опыте. Если мне больше нечего сказать напарнику по поводу его кода, это вовсе не значит что его код идеален. Я уже завтра могу прочитать новую статью и пережив ее понять, что код не такой уж и совершенный, как казалось вчера. Кроме того ревью может сделать более сеньорный специалист и рассказать о том, на какие грабли он наступал, на какие еще не наступали я с моим напарником. И то, что этому сеньорному специалисту в какой-то момент больше нечего будет сказать нам - значит лишь одно - мы написали такой код, который хотел бы написать он сам. А завтра все поменяется.

По этой причине код стоит пересматривать регулярно. Если в какой-то момент про код забыли - он устаревает со скоростью света. Но если в код вносились изменения сегодня - велика вероятность, что в него будут вноситься изменения завтра. Такой код после внесения серии изменений стоит пересматривать его коллективно.

Ну вот как бы и все, что хотел запечатлеть в памяти. Надеюсь кому-то пригодится...

Комментариев нет:

Отправить комментарий