В помощь новичкам и любителям.

Страницы: «1» «2» «3» «4» «5» «6» «7» «8» «9» «10»

Предлагаю собирать кусочки "протухшего" кода, как в соседней теме: http://www.delphi.int.ru/forum/topic/39/

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

Плюс, думаю даже очень опытные программисты найдут для себя, хотя бы один хороший пример. Желательно если вы будете выкладывать собственный код, специально написанный для примера, но можно и исправлять существующий (свой старый или чужой).

Запутанные идентификаторы.

Вроде бы все понимают проблему, и тем не менее, всё рано продолжают использовать названия, которые совершенно ничего не объясняют, либо используют короткие названия.

Вот несколько примеров того, как некоторые из нас пишут, и как им надо было писать:

I.

var 
  del, Val, MAX: longint;

Кажется что проблемы только с переменной del, но не тут то было!

var
  Divider, Value, MaxValue: LongInt;

Выводы:
1. Сокращения позволяются с общепринятыми названиями (max(imal), val(ue), i(ndex)). Но лучше их не использовать.
2. Переменные с транслитерованными русскими словами - плохая идея. Моя переменная del - транслитерация от delitel, я заменил её на английское слово Divider.
3. Используйте одинаковый регистр для всех переменных. В примере сначала del - в нижнем, MAX в верхнем регистрах, а Val - с большой буквы.
4. Идентификаторы должны иметь хорошее отношение понятность/длина. Max - короткий идентификатор, но какой максимум он отображает можно не понять, поэтому я дописал - MaxValue, и теперь точно понятно, что MaxValue - самое большое значение Value в коде.

II.

  unsigned char ts,kod,ks;
  int adr,i;

10 минут у меня ушло, только чтобы понять, что значат переменные ts и ks, а суть одной переменной (sk) я так и не понял, две (pr_z, chet) вообще не используются в коде!

  uchar NowSum, CheckSum, Code;
  int Adress, Index;

Оказалось, что ts - текущая сумма, ks - контрольная сумма, adr и i - очевидные сокращения, но как же красиво выглядит код с нормальными Adress и Index.

Прим. На языке си++ принято соглашение о именовании имён переменных с маленькой буквы. Тогда код станет такой:

  uchar nowSum, checkSum, code;
  int adress, index;

На мой взгляд читаемость немного ухудшилась, но разобраться что за что отвечает всё равно легко.

III.

...
const
  n = 15;
type
  mas = array[0..n - 1] of integer;
 
  procedure ra( var x: mas );
  var
    ind: integer;
  //...
 
  procedure printmas( const x: mas );
  var
    ind: integer;
  //...
 
  procedure ex( var x: mas; var ch: integer );
  var
    ind: integer;
  //...
 
var
  x:  mas;
  ch: integer;
begin
  Ra( x );
  printmas( x );
  ex( x, ch );
  printmas( x );
end;

Это код одного из участников нашего портала, надеюсь он не обидится, что я укажу на его ошибки.

...
const
  ArrayLength = 15;
type
  TArray = Array[0..ArrayLength - 1] of Integer;
 
  procedure FillArrayRandom( var ArrayToFill: TArray );
  var
    Index: Integer;
  //...
 
  procedure PrintArray( const ArrayToPrint: TArray );
  var
    Index: Integer;
  //...
 
  function MakeArrayPositive( var ArrayToChange: TArray ): Integer;
  var
    Index: Integer;
  //...
 
var
  WorkArray:  TArray;
  NumberOfOperations: integer;
begin
  FillArrayRandom( WorkArray );
  PrintArray( WorkArray );
  NumberOfOperations := MakeArrayPositive( WorkArray );
  PrintArray( WorkArray );
end;

Выводы:
1. Не используйте сокращения для нескольких слов даже если они кажутся вам понятными. Человеку писавшему этот код казались понятными: ch, ex, ra, ind.
2. Начинайте названия типов с буквы T. Это общее соглашение для всех дельфистов. Сишники, начинайте названия классов с большой буквы.
3. Имена параметров должны прояснять, что делает функция. Например если вы не понимаете смысл фразы Fill array random то, посмотрев на её параметр ArrayToFill, всё сразу прояснится.
4. *Оффтоп* Если процедура возвращает значение, лучше оформить её как функцию.
MakeArrayPositive( x, ch ) превратилась в ch:=MakeArrayPositive( x ).
Теперь вызывающий точно знает что в ch он ничего не передаёт процедуре, и что она точно что то ему вернёт.

Список тем, на которые ещё можно что-либо написать:
фрагмент кода

Магические числа

Магические числа это любые числа в программе, суть которых не ясна.

I.

BlockWidth := Image.Width div 10;

Вроде бы всё красиво и понятно, но почему именно 10? Просто Image делится по ширине на 10 блоков. Разве это понятно?

const BlockCount = 10;
BlockWidth := Image.Width div BlockCount;

Другое дело!

Вывод:
1. Магические числа нужно делать константами, для лучшего понимания происходящего.

II.

  if MiniMode then
  begin
    Width  := 240;
    Height := 260;
  end
  else
  begin
    Width  := Screen.Width * 7 div 10;
    Height := Screen.Height * 7 div 10;
  end;

Как видите, в этом коде 2 проблемных места.

const 
  MiniModeWidth           = 240;
  MiniModeHeight          = 260;
  NormalModeScreenPercent = 0.70;      
 
  ...
 
  if MiniMode then
  begin
    Width  := MidiModeWidth;
    Height := MiniModeHeight;
  end
  else
  begin
    Width  := Round( Screen.Width  * NormalModeScreenPercent );
    Height := Round( Screen.Height * NormalModeScreenPercent );
  end;

Вывод:
1. Округление лучше чем деление+умножение. В данном случае я заменил последовательность x*p/q на z=p/q, Round(x*z);
2. Магические числа нужно делать константами, на случай редактирования. Если мне приспичит сделать так, чтобы программа занимала 60 или 80 процентов экрана, я могу поменять это значение в одном месте и буду уверен, что всё продолжит работать как надо.

III.

case key of
  37: ...
  38: ...
  39: ...
  40: ...
end;

const 
  LeftArrow  = 37;
  UpArrow    = 38;
  RightArrow = 39;
  DownArrow  = 40;
...
 
case PressedKey of
  LeftArrow:  ...
  UpArrow:    ...
  RightArrow: ...
  DownArrow:  ...
end;

Вывод:
1. Магические числа могут помешать осмыслению кода.

Gooddy: очень дельные советы! Дай бог, чтобы как можно большее число людей это прочитало! Возможно, есть смысл оформить советы в виде статьи (статей?) и добавить их в соответствующий раздел — материал отличный.

Я призывал всех в начале статьи публиковать хотя бы маленькие кусочки кода с редактированием и комментариями, сформировать всё в одну статью будет уже делом десятым.

Цитата (Ерёмин А.А.):

Я прочитал. Всё правда.

min@y™: Ну то что ты прочитал, это лишь страховка, что я не допустил ошибок)

Цитата (Gooddy):

Да я по диагонали...

Жаль