delet-dis / neumorphic-calculator

🧮 Simple neumorphic Android calculator

Hi there, I'm Igor - aka delet-dis 🖖🏼

I'm a student and middle Android and iOS developer!

• 🇷🇺 I'm from Russia, Tomsk
• 👨🏻‍💻 I’m currently working as a freelancer, Samsung IT-School Teacher 🧑🏻‍🏫 and MIPT Phystech 🏫 mobile developer and tester
• 👨🏻‍🎓 I’m currently teaching schoolchildren Android development
• ✅ 2024 Goals: Keep working hard 💪🏻 and successfully study in the third year of the university

☎️ Connect with me:

💪🏻 Tech stack:

neumorphic-calculator's Issues

Ревью кода

1. Не должно быть никаких глобальных переменных. Переменные должны быть только внутри того класса, в котором они используются.

2. Для каждой кнопки ставится отдельный лисенер, хотя логика в них одна и та же: записать значение текста самой кнопки в отдельное поле. Можно создать группу в xml разметке и поместить в нее все кнопки с одинаковой логикой (пока что только кнопки цифр). После, в классе активности обращаясь к группе написать один единственный onClickListener с логикой
   calculator_display_non_mock.text = calculator_display_non_mock.text.toString() + (it as? Button).text.toString()

3. Примерно тоже самое, что я описал выше, можно сделать и с кнопками операций

4. Обработка ошибки есть только в некоторых лисенерах. Так, к примеру, если я жму несколько раз на кнопку плюс, то приложение просто вылетает, а вылет приложения -- смерть для мобильного разработчика

5. Название операций определяется по входной строке. Если ты где-нибудь сделаешь опечатку, то долго будешь отлавливать ошибку. Все строковые константы должны быть в отдельных константных переменных. В твоем же случае лучше сделать отдельный enum class Operation, в котором будут описаны возможные операции. И уже в функцию onClickOperation передавать нужный тебе enum элемент.

6. В коде xml находятся элементы, которые предназначены для телефонов с версией api выше, чем минимальная в проекте. Так лучше не делать, потому что для телефонов с api 26 не будет работать то, что работает на api 28.

7. Не понимаю логику того, что в mock_text'е куча запятых, которые ты потом еще и меняешь на нужные тебе значения. Это вообще не понятно для стороннего программиста и так лучше не делать. Не лучше ли тогда просто оставить его пустым и записывать нужные значения?

8. Так как ты пишешь на котлине, то и наименование переменных в xml тоже нужно придерживаться котлиновских, а именно использовать верблюжью нотацию (вместо calculator_display_non_mock писать calculatorDisplayNonMock)

Ревью кода #2

1. Один файл — один класс (исключением могут быть, если есть множество дата классов или функций объединенных одной логикой). А это означает, что enum Operation должен находится либо внутри активности, либо в отдельном файле в студии

2. Лучше инициализацию лисенеров переместить в отдельную функцию initListeners(). В onCreate должны находится базовые функции, которые можно будет прочитать и примерно понять что происходит в активности.

К примеру очень часто бывает: "initView() initListeners() initAdapters() observeViewModel()" в onCreate и тогда человеку сразу ясно что проиходит в основном в активности и где. "Ага, тут инициализирует вью, тут устанавливает лисенеры, тут устанавливает адаптеры, а потом делает какую-то логику"

3. В onClickOperation ты все равно принимаешь строку, а это означает, что другой программист вызовет onClickOperation("хочу арбуз") и словит ошибку сказав, что это ты неправильно написал функцию. Поэтому функция должна выглядеть вот так onClickOperation(processingOperation: Operation) и уже внутри разруливать с этим атрибутом так, как тебе нужно

4. if (isAvailableToOperate()) { onClickOperation(Любая_переменная) }
   Этот кусок кода часто повторяется, хотя тут меняется только одна переменная. Нужно сделать отдельную функцию, куда ты будешь просто передавать нужный тебе параметр (для деления DIVIDE, для умножения MULTIPLY и тд)

5. Переименуй переменные val1 и val2, т.к. не понятно за что они отвечают. Это две разные цифры, или предыдущее и текущее значение? Чтобы таких вопросов не возникало нужно сразу именовать переменные таким образом, чтобы было понятно что в ней лежит

6. В логике описания нажатия кнопки минуса 3 раза повторяется calculatorDisplayNonMock.text.toString() в одном ифе. Лучше вынеси это в отдельную переменную, и тогда тебе не придется постоянно писать огромный текст, так еще и работать будет быстрее, потому что сделал преобразование только один раз, а не несколько

7. Магическая константа 2 в обработке логики кнопки минус. Почему 2? Что за 2? Почему она там? Я, честно говоря, до сих пор и не понял)

8. В котлине есть очень удобный способ конкатинации строк. Вместо того, чтобы писать "str1" + "-" + "str2" можно писать "$str1 - $str2". Если str1 будет не просто переменной, а какой-нибудь str1.toSth().toString(), то можно использовать фигурные скобки "${str1.toSth().toString()} - $str2"

9. Все еще не понятно почему у тебя один из textView напичкан запятыми и ты их реплейсишь. Оставь коммент в этом случае, это сильно упростит работу другим людям