Сегодня знакомились с новой группой 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? Это всего лишь озвучивания той дельты, которая существует у двух специалистов в их опыте. Если мне больше нечего сказать напарнику по поводу его кода, это вовсе не значит что его код идеален. Я уже завтра могу прочитать новую статью и пережив ее понять, что код не такой уж и совершенный, как казалось вчера. Кроме того ревью может сделать более сеньорный специалист и рассказать о том, на какие грабли он наступал, на какие еще не наступали я с моим напарником. И то, что этому сеньорному специалисту в какой-то момент больше нечего будет сказать нам - значит лишь одно - мы написали такой код, который хотел бы написать он сам. А завтра все поменяется.
По этой причине код стоит пересматривать регулярно. Если в какой-то момент про код забыли - он устаревает со скоростью света. Но если в код вносились изменения сегодня - велика вероятность, что в него будут вноситься изменения завтра. Такой код после внесения серии изменений стоит пересматривать его коллективно.
Ну вот как бы и все, что хотел запечатлеть в памяти. Надеюсь кому-то пригодится...
Сообщения
