Refactoring to use model classes instead of database entitites by Futsch1 · Pull Request #1347 · Futsch1/medTimer

Futsch1 · 2026-04-06T07:24:23Z

No description provided.

…ry for overview

# Conflicts: # app/src/main/java/com/futsch1/medtimer/reminders/ReminderNotificationProcessor.kt # app/src/test/java/com/futsch1/medtimer/processortests/NotificationProcessorTest.kt # app/src/test/java/com/futsch1/medtimer/processortests/RefillProcessorTest.kt # app/src/test/java/com/futsch1/medtimer/processortests/RepeatProcessorTest.kt # app/src/test/java/com/futsch1/medtimer/processortests/ScheduleNextReminderNotificationProcessorTest.kt

Resolved conflicts by keeping entity naming convention (*Entity suffix) and adding mapper layer in ReminderEventRepository to bridge between database entities and domain model ReminderEvent.

…s not compiling)

# Conflicts: # app/src/main/java/com/futsch1/medtimer/database/MedicineRepository.kt # app/src/main/java/com/futsch1/medtimer/reminders/NotificationProcessor.kt # app/src/main/java/com/futsch1/medtimer/statistics/CalendarEventsViewModel.kt

qltysh · 2026-04-06T07:25:53Z

2 new issues

Tool	Category	Rule	Count
qlty	Structure	Function with many parameters (count = 6): Factory	1
qlty	Structure	Complex binary expression	1

app/src/main/java/com/futsch1/medtimer/database/TagRepository.kt

app/src/main/java/com/futsch1/medtimer/overview/model/PastReminderEvent.kt

app/src/main/java/com/futsch1/medtimer/overview/model/ScheduledReminderEvent.kt

app/src/main/java/com/futsch1/medtimer/database/dao/ReminderDao.kt

app/src/main/java/com/futsch1/medtimer/database/dao/ReminderEventDao.kt

app/src/main/java/com/futsch1/medtimer/database/dao/TagDao.kt

Futsch1 · 2026-04-06T07:38:05Z

@ThomasKiljanczykDev
I have drafted the PR to use only model classes in the application code. It is also a big one, but the scope of changes is limited to this (and a few minor things left and right). Please have a look.

Android Tests still fail, so there's a few bugs left which I will fix.

ThomasKiljanczykDev

I haven't reviewed everything, but some of these comment are disruptive enough that the code might change a lot.

I will continue the review once you address my current comments.

ThomasKiljanczykDev · 2026-04-06T08:21:58Z

app/src/main/java/com/futsch1/medtimer/database/backup/BackupManager.kt

-    private val reminderRepository: ReminderRepository,
-    private val reminderEventRepository: ReminderEventRepository,
-    private val tagRepository: TagRepository,
+    private val medicineDao: MedicineDao,


Why have you replaced repositories with dao?
The repositories were meant to abstract ALL the code from the db implementation.

With this mixed approach you're breaking this layer of abstraction

I have moved this class into the database package for a reason: I consider the backup tightly coupled to the database implementation, so it is a database backup, not a model backup (also for the sake of compatibility). Since repositories interface is now based on the model classes, the backup requires a direct database access.

So I do not consider this breaking the abstraction.

ThomasKiljanczykDev · 2026-04-06T08:22:56Z

app/src/main/java/com/futsch1/medtimer/database/backup/BackupManager.kt


    private fun createAndSave(fileContent: String?) {
-        val file = File(context.cacheDir, backupFilename)
+        val file = File(context.cacheDir, PathHelper.backupFilename)


Helper (and Util) is a very vague name which often leads to dumping a lot of unrelated code into a single class.
It would probably be better to give it some more sensible name

ThomasKiljanczykDev · 2026-04-06T08:28:57Z

app/src/main/java/com/futsch1/medtimer/model/Medicine.kt

+    val id: Int,
+    val color: Int,
+    val useColor: Boolean,
+    val notificationImportance: ReminderNotificationChannelManager.Importance,


You should probably extract the importance into NotificationImportance to make it independent from the channel manager. Channels themself are more part of the application layer

ThomasKiljanczykDev · 2026-04-06T08:32:33Z

app/src/main/java/com/futsch1/medtimer/database/backup/JSONMedicineBackup.kt

+) : JSONBackup<FullMedicineEntity>(FullMedicineEntity::class.java) {
+    override fun createBackup(databaseVersion: Int, list: List<FullMedicineEntity>): JsonElement {
+        // Correct the medicines where the reminder's instructions are null in this loop
+        for (fullMedicine in list) {


You could use flatMap here instead of this for

ThomasKiljanczykDev · 2026-04-06T08:36:22Z

app/src/main/java/com/futsch1/medtimer/database/backup/JSONMedicineBackup.kt

+
+    private suspend fun processTags(fullMedicine: FullMedicineEntity, medicineId: Int) {
+        for (tag in fullMedicine.tags) {
+            val tagId = tagDao.create(tag).toInt()


You should probably sort this ID casting nonsense.

It should be either int everywhere or long everywhere.
Long is the safer option since the performance impact should not be that noticeable and you get practically infinite possible values

I do not see how: Room always returns long from @insert operations. I chose to use int as ID type some time ago and do not want to do a migration just to change this to long.

The repositories now always return int IDs, so towards the application it's int only. Internally, we have to do a cast, but I do not see how this hurts.

ThomasKiljanczykDev · 2026-04-06T08:37:55Z

app/src/main/java/com/futsch1/medtimer/database/backup/JSONMedicineBackup.kt

+        }
+    }
+
+    private suspend fun processReminders(fullMedicine: FullMedicineEntity, medicineId: Int) {


You should accept the domain model and copy the reminders (with id reset)

See my comment above: I'd like to keep the backup logic close to the database structure.

ThomasKiljanczykDev · 2026-04-06T09:06:22Z

app/src/main/java/com/futsch1/medtimer/database/MedicineEntity.kt

-@Entity
-class Medicine() {
+@Entity(tableName = "Medicine")
+class MedicineEntity() {


You could move all the properties into the constructor so you create new entities more easily without for default values

Rename PathHelper.kt to ExportBackupPath.kt

Unified database id datatype handling Improve JSONBackup::createBackup

Futsch1 · 2026-04-07T20:25:45Z

@ThomasKiljanczykDev
I have addressed all your review comments

Futsch1 · 2026-04-11T18:11:49Z

@ThomasKiljanczykDev
Can you let me know when you'll be able to review? No hurries, but still would be good to know...

Futsch1 added 24 commits April 2, 2026 17:31

Rename database objects to "Entity"

ed87a31

Move ScheduledReminder.kt to model directory and create model directo…

b44fa7d

…ry for overview

Add reminder event model classes and the mapper in the database

7b277ac

Use new model types for OverviewViewModel.kt

571d104

Add preliminary AGENTS.md

f7fd55c

Use ReminderEvent in several MedicineRepository functions

5554579

Post merge fixes

6c43037

Merge ReminderEvent model into single class again

10c98b3

Merge origin/main: split repositories and DAOs per entity type

6f9c06d

Resolved conflicts by keeping entity naming convention (*Entity suffix) and adding mapper layer in ReminderEventRepository to bridge between database entities and domain model ReminderEvent.

Fix remaining merge issues

522b252

Eliminate more entity usages

0f69303

Fix issue with copied ReminderEvent in variable amount

8f44d3e

Eliminate remaining uses of ReminderEventEntity in the application

45a6cdf

Further cleanup of ReminderEvent

ab88b4d

Prepare Reminder model class and mapper

bb0207d

Intermediate state having Reminder and ReminderEntity separated (test…

42afb09

…s not compiling)

Intermediate state having Medicine and FullMedicineEntity separated

ef283b3

Fixes in unit tests

f87a17b

Fixes in unit tests

59f8441

Finish migrating TagEntity to Tag

7b52c8c

Fix handling interval times in Reminder model

252233e

Merge branch 'main' into databaseEntityToModel

c3ddd9e

# Conflicts: # app/src/main/java/com/futsch1/medtimer/database/MedicineRepository.kt # app/src/main/java/com/futsch1/medtimer/reminders/NotificationProcessor.kt # app/src/main/java/com/futsch1/medtimer/statistics/CalendarEventsViewModel.kt

Post merge fixes

d0364ea

Futsch1 marked this pull request as draft April 6, 2026 07:24

qltysh bot reviewed Apr 6, 2026

View reviewed changes

Cleanup in database package

0b68b1a

qltysh bot reviewed Apr 6, 2026

View reviewed changes

Fix qlty issues

b69dcb5

More qlty issues

67f8514

ThomasKiljanczykDev suggested changes Apr 6, 2026

View reviewed changes

Futsch1 added 5 commits April 6, 2026 23:36

Fix Android tests

aa88ad3

More test fixes

7e8aa09

Move notification importance to model

31e8a75

Rename PathHelper.kt to ExportBackupPath.kt

Change entities to contain all parameters in their constructors

c34df28

Renamed MedicineDao functions

371e7f1

Unified database id datatype handling Improve JSONBackup::createBackup

Always use Europe/Berlin time zone for unit tests

abaa829

Futsch1 marked this pull request as ready for review April 8, 2026 17:11

Uh oh!

Conversation

Futsch1 commented Apr 6, 2026

Uh oh!

qltysh bot commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Futsch1 commented Apr 6, 2026

Uh oh!

ThomasKiljanczykDev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Futsch1 commented Apr 7, 2026

Uh oh!

Futsch1 commented Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

qltysh bot commented Apr 6, 2026 •

edited

Loading