Skip to content

Conversation

@F462
Copy link

@F462 F462 commented Jun 1, 2025

First of all, thank you for the great project!

I use this app to manage my entrance card to go swimming. Thereby, when buying the card, it allows 12 entries, while obviously each entry decreases the balance by 1.
So, at the moment, after each entry I have to open the balance update dialog, enter a "1" and then press the positive button.
To avoid entering the, for the card well-known, value "1" each time, I added in this PR the possibility to provide the card with a default value to change the balance.

Screenshot_1748766692
Screenshot_1748766765

By the way: it seems as if the contribution guidelines are outdated, since the tasks testReleaseUnitTest and lintRelease don't exist anymore.

balance = new BigDecimal(String.valueOf(jsonCard.getDouble("balance")));
}

BigDecimal defaultBalanceChange = null;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I only figured out how to test the Catima importer, so this might be a mistake.

String unparsedDefaultBalanceChange = CSVHelpers.extractString(DBHelper.LoyaltyCardDbIds.DEFAULT_BALANCE_CHANGE, record, null);
if (unparsedDefaultBalanceChange != null) {
try {
defaultBalanceChange = new BigDecimal(unparsedDefaultBalanceChange);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In contrast to the balance, I didn't do the extractString again here when creating the BigDecimal. Is there a reason why this is done for the balance?

@TheLastProject
Copy link
Member

Hi, thanks for contributing.

This seems quite a big change, even needing a database migration, for a fairly niche use case. I'm not sure if the extra UI complexity and setting fits in Catima's philosophy of being simple to use and understand (if anything, I argue we already have too many settings and the UI is overly complex at some points).

Am I correct in that this only saves 1 single tap in a flow that's already only 3 taps? As I understand it:

  1. Press "Spend" icon, a dialog opens and the keyboard automatically pops up
  2. Press 1 <- This is now automated
  3. Press spend

Given how little difference this makes in daily usage, yet it adds a fair bit of extra code complexity, I feel tempted to say I'd rather not accept this contribution. The fact that it adds extra Java and Android XML code while we already need to migrate everything to Kotlin Compose (#2483) which is a huge rewrite (and thus: more to rewrite after this) adds to that decision.

I hope this is not too discouraging, I definitely appreciate seeing someone contribute, it's a fairly rare but very welcome sight :)

@F462
Copy link
Author

F462 commented Jun 2, 2025

A fairly niche use case, but also a fairly nice one ;)

I agree that the added complexity is certainly not worth a merge. It was still worth digging into the app and developing it for myself, so that I can use a slightly different app version. That's the power of Open Source, I guess.

However, the Kotlin Compose rewrite also sounds interesting. I'm not that familiar with it yet, but if you have a good entry point where I could contribute, just let me know @TheLastProject ! :)

@TheLastProject
Copy link
Member

@F462 I'm messing around with Jetpack Compose a bit in #2489, still trying to learn it.

However, we first need to use Kotlin in activities. So the first step is basically rewriting every single activity in Kotlin, so the UI can become Compose after. If you feel like it, help in converting any of the non-claimed activities (basically every single one) to Kotlin would be quite welcome: #2483 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants