Теме чистого кода на одном только habrahabr посвящено тысячи статей. Никогда бы не подумал, что захочу написать свою. Но после проведения компанией курсов для желающих связать карьеру с разработкой ПО, в частности разрабатывать под Android, мое мнение поменялось.

За основу статьи взяты советы из классики “Роберт К. Мартин: Чистый код”. Отобрал из них те, которые наиболее часто встречались в виде проблем у студентов. Приведенные советы написаны с учетом моего опыта разработки коммерческих Android приложений. Поэтому не ко всем Android-проектам приведенные ниже советы подойдут, я уже не говорю про другие системы.

Советы в основном приводил с примерами кода как НЕ НУЖНО делать. Как ни странно, у большинства студентов были одни и те же ошибки. Все примеры кода были придуманы, любые совпадения с реально существующим кодом случайны.

Общие советы

1. Код должен быть легко читаемым, понятным и очевидным

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

2. При написании кода нужно придерживаться Java Code Conventions либо других спецификаций, принятых на проекте командой

Спецификацию можно сравнить с правилами оформления текста книги. Думаю, немногие захотят читать книгу, где каждый абзац отличается шрифтом, размером и цветом текста. Так и с кодом, куда легче читать оформленный в едином стиле код.

Наименование

1. Имена классов, функций, переменных и параметров должны передавать закладываемый в них смысл.

Довольно очевидный совет, но не всегда его придерживаются. Тут, в качестве примера, можно привести оглавление книги: если все главы названы просто "Главами", то понять в чем суть нельзя. Наименования классов, функций должны сообщать об их предназначении без погружения в детали реализации.

Наименования классов MyActivity, CustomView, MyAdapter говорит только об одном, что это не часть Android SDK и все. SecondActivity говорит, что это не часть Android SDK и о наличии в проекте еще одного Activity, но не факт =)

Правильно именовать классы: MainActivity, FoodsActivity, FoodCartActivity, PhoneInputView. MainActivity — очевидно, что основной разводящий экран приложения. FoodsActivity — экран с перечнем продуктов. PhoneInputView — компонент ввода номера телефона.

Следующие названия переменных бесполезны и нарушают правила наименования:

private TextView m_textview_1;
private TextView m_textview_2;
private TextView m_textview_3;

как и параметры конструктора:

public Policy(int imgId,int imgId2)

2. Название публичного класса и файла должно совпадать.

Есть следующий класс

public class ProductsActivity extends AppCompatActivity { ... }

Название файла при этом оказывается “products_Activity.java”.
Это не правильно. Название публичного класса и java-файла должны совпадать.

3. В наименованиях нужно использовать только буквы латинского алфавита, никаких цифр, символов подчеркивания и дефисов. Исключения составляют наименования из стандартов (ГОСТ, ISO), символы подчеркивания для разделения слов в наименованиях констант.

Примеры выше: m_textview_1. Часто вместо lastName пишут userName2, что не правильно.

4. Не нужно использовать строчные “L” и “O” в качестве имен локальных переменных, т.к. их трудно отличить от “1” и “0”.

Надуманный пример, но что-то подобное я встречал в своей практике

private void s(int a[]) {
        for (int l = 0; l < a.length; l++) {
            for (int O = a.length - 1; O > l; O--) {
                if (a[O - 1] > a[O]) {
                    int o = a[0 - 1];
                    a[O - 1] = a[O];
                    a[O] = o;
                }
            }
        }
    }

В этой функции пузырьковой сортировки есть одна ошибка, сможете за секунды ее найти=)?
Такой код труден для чтения и при написании легко сделать ошибку, которую можно очень долго искать.

5. Не нужно указывать тип в суффиксе имен.

Вместо accountList нужно писать просто accounts. Это позволит в любое время изменить тип переменной без переименования самой переменной.
А еще ужаснее выглядит nameString, ageFloat.

Исключение составляют наследники классов Android SDK: Activity, Fragment, View, Uri и т.д. По названию NewsSynsService сразу понятно, что класс является "сервисом" и ответственен за синхронизацию новостей. Использование суффикса view в nameView, photoView позволяет легко отличить переменные, относящиеся к верстки, от остальных. Имена view обычно начинают с существительного. Но имена кнопок лучше начинать с глагола: buyButton

6. Имена могут и должны содержать термины из математики, названия алгоритмов, паттернов проектирования и т.д.

Увидев имя BitmapFactory, не автор кода сразу поймет смысл этого класса.

7. Не нужно указывать никакие префиксы при именовании.

Вместо m_user, mUser просто пишется user. Указывать префикс s для статических полей в современных IDE излишне.

Исходники Android SDK не являются здесь показателем в силу давности создания первых версий и наследования кодовой базы до наших дней.

public static final String s_default_name  = "name";

s_ ни к чему в начале названия статического поля. К тому же название констант должно писаться прописными буквами:

public static final String DEFAULT_NAME = "name";

8. В наименование классов нужно использовать существительные.

Классы это как объекты реального мира. Поэтому нужно использовать существительные для их названия: AccountsFragment, User, Car, CarModel.

Не нужно называть классы Manager, Processor, Data, Info, т.к. они имеют слишком общее значение. Лучше название класса длиной в два-четыре слова, чем просто Data.

9. Названия классов должны начинаться с прописной буквы.

Слова НЕ должны отделяться символом подчеркивания. Нужно следовать нотации CamelCase: GoodsFragment, BaseFragment

10. Используйте одно слово для каждой концепции.

Использование fetch, retrieve, get в одном классе сбивает с толку. Если класс назвали Customer, то имена переменных класса и параметров функций этого типа лучше называть customer, а не user.

Функции

1. Функция должна выполнять только одну “операцию”. Она должна выполнять ее хорошо. И ничего другого она делать не должна.

Под “операцией” следует понимать не одну математическую операцию или вызов другой функции, а действия на одном уровне абстракции. Чтобы определить, что функция выполняет более одной операции, следует попробовать извлечь из нее другую функцию. Если извлечение функции не дает ничего кроме простой перестановки кода, то значит разбивать дальше функцию не имеет смысла

К примеру есть функция setProducts класса ProductsAdapter:

public class ProductsAdapter extends RecyclerView.Adapter<ProductHolder> {

public void setProducts(List<Product> newProducts) {
   products.clear();

   for (Product newProduct : newProducts) {
       if (!TextUtils.isEmpty(newProduct.getName())) {
           products.add(newProduct);
       }
   }

   notifyDataSetChanged();
}

Внутри функции происходит три основных операции: 1) фильтрация newProducts, 2) очистка и вставка новых значений в products, 3) обновление адаптера notifyDataSetChanged.

Фильтрация элементов newProducts должна происходить в другом методе и даже в другом в класса (например в presenter-е). В ProductsAdapter должны прийти уже отфильтрованные данные.

Лучше переделать код следующим образом:

public void setProducts(List<Product> newProducts) {
   products.clear();
   products.addAll(newProducts);
   notifyDataSetChanged();
}

Параметр newProducts содержит уже отфильтрованный список продуктов.
Можно еще строчки java products.clear(); products.addAll(newProducts);
вынести в отдельную функцию, но особого смысла я в этом не вижу. Лучше заменить notifyDataSetChanged на DiffUtil, это позволит обновлять ячейки в списке эффективнее

2. Размер функции должен быть 8-10 строк.

Функция размером в 3 экрана это не функция, это *****. У меня тоже не всегда получается ограничить размер функции 8-10 строками кода, но нужно стремится к этому. Пример по понятным причинам приводить не буду.

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

3. В теле функции все должно быть на одном уровне абстракции.

private void showArticelErrorIfNeed(Article article) {
   if (validateArticle(article)) {
       String errorMessage = "Article " + article.getName() + " is incorrect";
       showArticleError(errorMessage);
   } else  {
       hideArticleError();
   }
}

Вычисление значения локальной переменной errorMessage имеет более низкий уровень абстракции, чем остальной код внутри функции. Поэтому код java "Article " + article.getName() + " is incorrect" лучше вынести в отдельную функцию.

4. Наименовании функции и передаваемых параметров должно сообщать о том, что делает функция

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

public Product find(String a) { … }

Непонятно по какому полю будет происходить поиск, что передается на вход функции.

Лучше переделать в следующий вид:

@Nullable
public Product findProductById(@NonNull String id) { … }

Название функции говорит, что происходит поиск Product по полю id. На вход функция принимает не “null” значение. Если Product не найдется, то вернется “null”.

Роберт К. Мартин советует использовать параметры в качестве части названия функции:

public void add(Product product) { … }

Вызов функции может выглядеть так:

add(product);

На проектах я такой способ не встречал. Лучше данный способ не использовать и писать полностью название:

public void addProduct(Product product){ … }

5. Имена функций должны начинаться с глагола. Лучше длинное имя, чем не содержательное короткое.

public static void start(Context context) {...}

Название start не сообщает, что именно стартует функция. Только заглянув в тело становится понятно, что функция открывает Activity.

А должно быть понятно предназначение функции уже по названию.

public static Intent makeIntent(Context context) {...}  // значительно лучше.

6. Вместо передачи в аргументы функции флага (boolean) лучше разбить функцию на две функции

Часто этот флаг является причиной увеличение размера функции при ветвлении логики выполнения в зависимости от значения флага. В таких случаях следует подумать о разбиении данной функции на две. Разное поведение функции в зависимости от переданного флага не всегда очевидно.

7. Дублирующий код следует выносить в отдельную функцию.

lightButton.setOnClickListener(new View.OnClickListener() {
   @Override
   public void onClick(View view) {
  titleView.setTextAppearance(R.style.lightText);
       descriptionView.setTextAppearance(R.style.lightText);
       titleView.setBackgroundResource(R.color.lightBack);
       descriptionView.setBackgroundResource(R.color.lightBack);
   }
});
(... идет аналогичный код для других цветов)
greyButton.setOnClickListener(new View.OnClickListener() {
   @Override
   public void onClick(View view) {
       titleView.setTextAppearance(R.style.greyText);
       descriptionView.setTextAppearance(R.style.greyText);
       titleView.setBackgroundResource(R.color.greyBack);
       descriptionView.setBackgroundResource(R.color.greyBack);
   }
});

Код внутри setOnClickListener отличается только стилем. Этот код стоит вынести в отдельный метод:

private void changeStyle(@StyleRes int textSyleResource, @ColorRes int backgroundColorResource) {
   titleView.setTextAppearance(textSyleResource);
   descriptionView.setTextAppearance(textSyleResource);
   titleView.setBackgroundResource(colorResource);
   descriptionView.setBackgroundResource(RcolorResource);
}

8. Не передавайте и не возвращайте из функции “null” когда можно вернуть пустой объект.

Вместо 
public List<Product> findProductsByType(String type) {
   if (TextUtils.isEmpty(type)) {
       return null;
   } 
   List<Product> result = new ArrayList<>();
   for (Product product : products) {
       if (product.getType().equals(type)) {
           result.add(product);
       }
   }
   return result;
}

Лучше написать:
@NonNull
public List<Product> findProductsByType(@NonNull String type) {
   if (TextUtils.isEmpty(type)) {
       return Collections.emptyList();
   }
   List<Product> result = new ArrayList<>();
   for (Product product : products) {
       if (product.getType().equals(type)) {
           result.add(product);
       }
   }
   return result;
}

Это позволит избежать ошибок NullPointerexception и не нужно будет в местах вызова писать проверки на null.

Kotlin поможет отучиться от вредной привычки передавать и возвращать null.)

Форматирование

1. Код в классе должен читаться сверху-вниз как газетная статья в порядке убывания уровня абстракции. Вначале идут публичные функции, затем приватные.

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

2. Связанные концепции/функции следует размещать рядом, чтобы не было необходимости постоянно перемещаться по файлу вверх-вниз. Если одна функция вызывает другую, то вызываемая функция должна располагаться под вызывающей функцией (если это возможно) и разделяться пустой строкой.

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

3. Каждая последовательная группа строк кода должна представлять одну законченную мысль. Мысли отделяются друг от друга в коде с помощью пустых строк, только слишком злоупотреблять пустыми строчками не стоит

Связанная группа строк это как абзацы в статье. Для разделения абзацев используется красная строка или пустая строка. Так и в коде следует разделять разные по смыслу группы строк, используя пустые строки.

4. Переменные экземпляра класса, статические константы должны быть все в начале класса в одном месте.

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

Правильное объявление:

public class ImagePickerActivity extends AppCompatActivity {

public final static String KEY_CODE_HEX_COLOR = "hex_color";
public final static String KEY_CODE_PICKED_IMAGE = "picked_image";

private final static int REQUEST_CODE_GET_IMAGE = 2;
private final int REQUEST_CODE_HEX_COLOR = 1;

private Uri imageUri;

@Override
protected void onCreate(Bundle savedInstanceState) {

5. Используйте пробелы для повышения читаемости кода

• в математических операциях для отделения символа операции от операндов: int x = a + b;
• отделения параметров функций: private Article createNewArticle(String title, String description)
• передаваемых параметров функции: createNewArticle(title, description)

6. "Магические цифры, строки" должны быть вынесены в константы !!!
Пожалуй, самый популярный совет, но все равно продолжают его нарушать.

StringBuilder builder= new StringBuilder();
for (int i = days; i > 0; i /= 10) {
    builder.insert(0, i % 10);
}

Лишь прочитав постановку задачи я понял, для чего в коде использовалось число “10”. Лучше это число вынести в константу и дать осмысленное имя.

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

Классы

1. Класс должен иметь одну “ответственность”, одну причину для изменения.

К примеру, наследники класса RecyclerView.Adapter должны отвечать за создание и связывание View с данным. В нем не должен находится код сортировки/фильтрации списка элементов.
Часто в файл с Activity добавляют класс RecyclerView.Adapter, что является не правильным.

2. Не нужны пустые методы или просто вызывающий метод из родительского класса

@Override
protected void onStart() {
   super.onStart();
}

3. Если переопределяете какой-то метод без вызова метода родительского, то проверьте, что так можно делать.

Загляните в исходники родительских классов, документации. Переопределяемые методы жизненного цикла Activity, Fragment, View должны обязательно должны вызывать методы родительского класса.

Есть аннотация @CallSuper, предупреждающая о необходимости вызывать родительский метод при переопределении.

Советы о разном

1. Используйте средства Android Studio для улучшение качества Вашего кода.

Если она подчеркивает или выделяет код, значит что-то может быть не так. В Android Studio есть средства Rearrange, Reformat, Extract Method, Inline Method, анализаторы кода, горячие клавиши и т.д.

2. Не нужно комментировать каждый метод, код должен быть самодокументированным.

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

3. Строки, цвета, размеры должны быть в ресурсах (xml)

Не нужно писать:

private static final String GREEN_BLUE = "#33c89b";

или  String.valueOf("Осталось " + days + " дней")

Исключения составляют файлы из папок test, mock. Такие файлы не должны попадать в релизную версию

4. Не нужно передавать context в RecyclerView.Adapter. context можно получить из любой view. Элемент по клику в RecyclerView.Adapter должен меняться через notifyItemChanged, а не непосредственным изменением вьюшек в методе обработки нажатия

 holder.itemView.setOnClickListener(new View.OnClickListener() {
                @Override
                public void onClick(View view) {
                    item.buyNow();
                    bindViewHolder(holder, position);
                }
            });

В методе onClick непосредственно происходит вызов bindViewHolder, что НЕ правильно.

5. В RecyclerView.ViewHolder вместо определения позиции объекта в списке нужно вызывать getAdapterPosition, а не передавать position из метода onBindViewHolder

Правильно писать:

private static class ItemHolder extends RecyclerView.ViewHolder {

   public ItemHolder(View itemView) {
       super(itemView);      
       itemView.setOnClickListener(view -> {
           int position = getAdapterPosition();
           ...
       });
   }
}

Либо можно вместо позиции передавать ссылку на исходный объект с данными

itemView.setOnClickListener((v) -> {
   productClickListener.onProductClick(product);
});

6. Используйте Quantity Strings (Plurals) для множественного числа. Вместо написания в коде логики по подбору правильного окончания слов.

7. Не сохраняйте большие данные Bitmap в bundle. Размер bundle ограничен

8. Если прописать “id” в xml, то Android сам восстановит состояние стандартных View при повороте.