Не нашёл здесь такой темы, но считаю, что она будет полезна многим. Возможно, даже многим из тех, кто уже давно считает себя не новичком в Java.
В общем, идея проста - все, кому есть, что сказать, предлагают свои советы по тому, как писать код в хорошем стиле.
Чтобы было понятно, приведу пару советов из тех, что в последние дни вычитал на форуме:

1. Если в метод были переданы некорректные данные, выкидываем исключение вместо тихого-мирного притягивания данных за уши к корректному виду.
Пример.
плохо:

лучше:


2. При переопределении метода equals() надо переопределять ещё и hashCode(). Причём хэшкод должен быть равен для объектов, если они эквивалентны (equals() == true).

P.S. Понятно, что многим подобные вещи известны и они пользуются ими не задумываясь. С другой стороны, всё знать нельзя и скорее всего каждый найдёт для себя что-то интересное в этой теме. Ну, или вспомнит хорошо забытое старое.  

Сюда же: имя параметра в setter-методе желательно иметь соответствующим имени метода, также как имя поля.

И пара придирок к вашему коду: на мой взгляд, охватывающие скобки "{}" в if следует ставить всегда, когда условие тривиально и не входит в единственную строчку. При глубине вложенности больше 2-х это сильно упрощает чтение.
Также я бы печатал значение параметра weight, чтобы сразу было понятно что с ним не так. Программу это не усложнит, а при отсутствиии доступа к исходников упростит локализацию ошибки (в том числе в этой проверке). 

И кстати: такое "исправление внутри" может быть удобным и вполне имеет право быть применимым если :
1) Это private метод (наши грабли)
2) Факт описан в javadoc (документированные грабли)
Иначе придётся писать как "культурный" метод, так и враппер к нему. Я не сильно разделяю такую точку зрения, просто привожу её как контрпример.
Прошу не принимать близко к сердцу ;)


Второй пунк описан в каждом учебнике по Java. А IDEA так парой и предлагает их генерить...

P.S. Хорошая тема, надо развивать. Только нужно структурирование.
Люди пишут программу, отмечают "тру-решение" или наоборот , постят сюда - только кто возьмётся за сортировку?

Не думаю, что это всегда так... Такой подход, конечно, весьма практичен, но засоряет код лишними try/catch блоками... Кроме того, exception зачастую подразумевает исключительную ситуацию, которая связана с принципиально непреодалимой ошибкой, ну а в примере ошибка более чем преодолима и может встречаться довольно-таки часто.

Исключения как раз придуманы для того, чтобы преодолевать некритические ошибки и незапланированное поведение системы. Я согласен с тем, как в своем примере этот механизм применил nornad.

Дополню. Если создаётся наследник класса, в котором переопределёны equals() и hashCode(), в классе наследнике их следует переопределить обязательно.

На всякий случай поясню: я согласен с приведённым мною примером, но авторство не моё - в одной из тем именно этот пример дал LSD.
Мне, конечно, приятно собирать лавры, но в данном случае они не мои.
Первый пример тоже не мой, но автора я уже не помню (пример взят из темы "Маленький тест").

По поводу засорения кода блоками try-catch.
Имхо, лучше засорить код такими блоками и получить взамен более простой и удобный способ обнаружения и устранения ошибок.


К своему стыду должен признать, что не читал таких учебников. Даже TIJ до сих пор только издалека видел.


Если я правильно понял, то это звучит так:
Если мы в классе А переопределили equals+hashCode, то и в классе Б (наследнике от А) их тоже надо переопределить.

Думмаю, так чуток яснее.


Предпочитаю не захламлять код лишними блоками и пользуюсь следующим правилом:
1. если условие динное (из нескольких частей, либо просто больше 20-30 символов), переносим оператор (то, что надо выполнить) на следующую строку. Замечу, что это верно только для if с одним оператором - блоки всегда структурирую в отдельных строках, не считая самих скобок.
2. если в коде идёт сложная структура из операторов if, то в каждом обязательно обрамляем оператор скобками блока (даже если оператор один). Это позволяет избежать неверной работы, когда мы используем if-else. Пример:

Это плохо, т.к. выглядит и работает по-разному. Чтобы работа соответствовала виду, надо сделать хотя бы так:

Но я в таких случаях предпочитаю везде поставить блоки (иногда через пару дней лезешь в код и видишь, что надо добавить ещё один if - тогда-то блоки и спасают от вероятности приделать новый if не к месту).


Если честно, не понял. Опиши шире, что ты имеешь в виду.

Если у вас в коде есть константа и переменная типа String, которые нужно сравнить, то всегда вызывайте метод equails на константе:

плохо:


хорошо:

Небольшое дополнение.
Иногда нет необходимости выделять константу в собственное поле (по-хорошему, всё же лучше выделить), но совет всё равно стоит использовать:


Добавлено через 4 минуты и 35 секунд
Кстати, а вот и ещё один совет. Просто, обозначу его явно:

Все строковые константы желательно выделять в поля. Это относится к дефолтовым значениям строковых полей, названиям кнопок, страниц и панелей. Единственное исключение - пустая строка.

И немного расширим:

Все константы (любого типа) желательно выделять в поля. Исключения - пустая строка, 0 и т.п.

missorted modifiers, private string constant...

я бы сделал так

Я думаю что подобные константы всегда нужно выносить в поля, хотя с по поводу пустой строки согласен.  Это так называемые "magic numbers" (это обычно к числам относится, поэтому и "numbers"). Их тяжело искать в коде, да и если константа используется в нескольких местах, что чаще всего и бывает, то корректировать их во всех участках кода не есть благородное занятие.


А еще лучше учитывать интернационализацию и все названия GUI контролов читать из bundle-файла, тогда вопрос выделения их в поля просто не будет иметь место.

Добавлено через 41 секунду

очень может быть 

nornad

А чем 0 лучше других чисел?

Думаю, эти вещи нужно не выделять в поля, а хранить в properties файле и доставать через ResourceBundle.

Тем же, чем и пустая строка "лучше" остальных строк.
Кстати, к "исключительным" магическим числам, имхо, стоит отнести и -1 - очень часто это означает "неверный индекс/вариант".
То есть, если 0 используется для проверки на пустоту списка (по количеству элементов), то не стоит его выносить в отдельное поле.
Также и -1, если она используется, скажем, для определения, есть ли выделение в таблице (JTable.getSelectedRow(), например).

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

Это я и имел в виду. Ноль в группе других числовых констант ничем не выделяется. В то же время, он как количество элементов списка -- очевиден. А как результат выполнения операции -- почти очевиден..

Создать что-то типа справочника "как надо" и "как не надо" - набор практических советов. Люди постят идеи в форуме, определённый человек их отфильтровывает и подкладывает в какой-нибудь FAQ. Строго по пунктам, со всеми "за" и "против", но лаконично. Для начала можно просто в начале темы. Можно выделить такие тематики как "косметическое оформление", "часто используемые конструкции" и т. д. в ширь и вглубь - по мере накопления материала.
И вообще я согласен с тезисом, что даже на глубоко тривиальные иногда нужно указать - не всегда они приходят легко и сразу.


Я как-то для себя решил не усложнять такие вещи. Либо if в одну строчку (обычно с break, return или throw), либо if полноценный - потенциально многострочный. Вколотить туда отладку какую-нибудь или логи опять же удобней...


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

На первый взгляд громоздко, зато ошибиться потом просто невозможно. 
Строки, используемые в GUI однократно, имеет смысл загружать на ходу (всё равно они там где-то кэшируются). Незачем засорять класс лишними константами, если они нужны только в одном методе и один раз. Только если уверен, что завтра они не понадобятся ;)

И на счёт ResourceBundle. Даже если не думать об интернализации, редактировать строки в ресурсном файле всё равно в разы удобнее, чем в исходнике, где они раскиданы по всем тексту.

К слову о константах. Читая книжки, натыкаешься чуть ли не на холивары на тему - должны быть они public или private. Хотя на самом деле всё просто - как и остальные члены класса, они должны иметь тот модификатор, который соответствует их необходимости для наследников или внешних классов.

Это сообщение отредактировал(а) RebornCrusader - 15.6.2007, 08:03

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


Согласен. Я понимаю, что equals+hashCode - тривиально, но лично я этого не читал раньше (ну, не люблю ходить к сану на сайт, а в книжках нормального описания не было).


Даже при такой уверенности они могут понядобиться завтра или послезавтра. Но тут уже можно либо сразу всё подозрительное выделять в поля, либо выделять в поле как только потребуется во втором месте.


Предпочитаю делать либо public (если может пригодиться снаружи), либо protected (уже пару раз столкнулся с тем, что бывает необходимость в потомке изменить константу, а если она приватная, то приходится либо менять доступ, либо привешивать сеттер - в любом случае маразм).