LINUX.ORG.RU

Раскритикуйте код

 , , ,


0

1

Сап, ЛОРовцы, я уже создавал тему по этому приложению, но тогда цель другая была. Сейчас мне нужна обоснованная критика моего поделия. Ибо учусь на джуниора в ветке Android разработки и это приложение - вершина моего творчестча, но я не уверен что это можно показывать людям, вдруг что не так сделал.
Сорцы: https://github.com/Hochland/Luna
И нытье в довесок: Хорошо ли нынче андроид разрабам живётся? Я вот сейчас пытаюсь работу найти, но на том-же hh полторы вакансии и тем опыт нужен, как-то не густо, знаете-ли.

Перемещено mono из talks

★★★★

// Singleton instance
private static LocationWorker instance = null;

1) не пиши коменты в стиле КО

2) если синглтон юзается в многопоточном окружении то жёпа

Deleted ()

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

    private void updateUi() {
        runOnUiThread(new Runnable() {
            @Override
            public void run() {
                toggleRefreshAnimationOff();
                placeTv.setText(mCurrentWeather.getPlace());
                if (mCurrentWeather.getTemperature() < 0) {
                    int reversedTemperature = mCurrentWeather.getTemperature() * -1;
                    minusSymbolTv.setVisibility(View.VISIBLE);
                    temperatureTv.setText(String.valueOf(reversedTemperature));
                } else {
                    minusSymbolTv.setVisibility(View.INVISIBLE);
                    temperatureTv.setText(String.valueOf(mCurrentWeather.getTemperature()));
                }
                String formattedHumidity = String.format("%s", mCurrentWeather.getHumidity() + "%");
                humidityValueTv.setText(formattedHumidity);
                pressureValueTv.setText(String.valueOf(mCurrentWeather.getPressure() * 0.75));
                String uppercaseSummary = mCurrentWeather.getSummary()
                        .substring(0, 1)
                        .toUpperCase()
                        + mCurrentWeather.getSummary()
                        .substring(1);
                weatherSummaryTv.setText(uppercaseSummary);
            }
        });
    }
Deleted ()
Ответ на: комментарий от Deleted

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

Jefail ★★★★ ()
Ответ на: комментарий от Deleted

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

Jefail ★★★★ ()
Ответ на: комментарий от Jefail

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

Ты возвел MVC в статус библии, в то время как речь хотяб о разделении на методы по выполняемым функциям - тогда читать эту спагеттину будет проще.

Deleted ()
Ответ на: комментарий от Kaschenko

скорее всего это начальный этап после обработки мозга фразами «пиши коменты сцуко»

Deleted ()
Ответ на: комментарий от Deleted

Как раз таки не фейл, у класса переопределен дефолтный конструктор и если он вызывается без параметров - то создается заглушка.

Jefail ★★★★ ()
Ответ на: комментарий от Jefail

Логику и форматирование в модель сувать не лучшая идея

Это почему же? Как раз в модели и содержится логика, а контроллер только готовит данные модели для вьюшки. Даже паттерн есть такой «fat model skinny controller»

comp00 ★★★★ ()
Последнее исправление: comp00 (всего исправлений: 1)
Ответ на: комментарий от comp00

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

Jefail ★★★★ ()
Ответ на: комментарий от Jefail

Привет дружок, это типичный доступ к полю из разных потоков, а ты рассказываешь что знаешь про это.

Deleted ()
Ответ на: комментарий от TheAnonymous

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

Deleted ()
Ответ на: комментарий от Deleted

Джава-нубасы могу копаться. Какие-нибудь джуниоры, прочитывшие джава за 21 день, могут копаться. Да много кто может. Если код понятен и таком человеку, это лучше, чем если непонятен.

hlebushek ★★ ()
Ответ на: комментарий от Deleted

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

hlebushek ★★ ()
Ответ на: комментарий от Jefail

все дергают одинаковые данные
одинаковые данные

в чем проблема то? Даже на уровне логики продумай - гораздо проще описать операции с моделью в 1м классе, а в 5 активити, используя методы модели, получить различные реализации интерфейсов для представления этих самых одинаковых данных.
А вообще, на лицо непонимание MVC (если, конечно, мы говорим о MVC).

comp00 ★★★★ ()

Самое смешное, что 90% всех приложений для Android написано в стиле, подобном этому говно-коду, и ничего - пользуемся!

rupert ★★★★★ ()
Ответ на: комментарий от Deleted

Как только отдашь код на саппорт жуниарами — так обязательно залезут в твои ботинки на тему «а хуле тут нипанятна? тут надо все переписать!»

slackwarrior ★★★★★ ()
Ответ на: комментарий от slackwarrior

Значит ты их хреново воспитываешь, я парочку раз таких умников приколол и стали шелковыми.

Deleted ()
Ответ на: комментарий от Deleted

Значит ты их хреново воспитываешь, я парочку раз таких умников приколол и стали шелковыми.

Экий ты простой :) Ты вообще не сможешь повлиять, кому и куда отдадут :)

slackwarrior ★★★★★ ()
Ответ на: комментарий от slackwarrior

Бгг, ну тогда беги из той конторы

Deleted ()
Ответ на: комментарий от Jefail

Зачем? Я даже времени не буду тратить на твой говнокод.

rupert ★★★★★ ()
Ответ на: комментарий от ertgblasd

Смотря что вы имели ввиду, выводы или God Object? Если GO, я стараюсь делить функциональность на классы, чтобы каждый из них выполнял только одну функцию, настолько, насколько это вообще возможно.

Jefail ★★★★ ()
Ответ на: комментарий от Jefail

Предпоследний я хотел сказать, но написал не то. Извиняюсь. Бездумного комментирования у тебя более чем дофига, оно будет резать глаза, работодателям в том числе.

ertgblasd ★★ ()
Ответ на: комментарий от ertgblasd

Хорошо, комментарии подчищу, мне лично они не нужны, я свой код понимаю, но вдруг другим пригодятся.

Jefail ★★★★ ()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.