search

SecUSo / privacy-friendly-sudoku

Privacy Friendly App to play Sudoku on Android.
https://secuso.org/pfa-sudoku
GNU General Public License v3.0
144 stars 55 forks source link

Getting correct value of "gameSolved" key (Partial fix for Issue #37) #67

Closed rdatta95 closed 3 years ago

rdatta95 commented 3 years ago

Observation:

  1. savedInstanceState.getInt("gameSolved") ALWAYS returned 0 (default).
  2. The key "gameSolved" is set with putBoolean, which is correct. A ClassCastException was thrown in getInt and the catch block returned the default 0-value.

Issues before change:

  1. Multiple statements that are supposed to be executed based on gameSolved (like disabling the special keys and the sudoku grid), were skipped.
  2. Because of that, after finishing a game, if someone changed the screen orientation, the keys and the grid became active again and the grid could be modified.

Changes done:

  1. Changed it to getBoolean("gameSolved")
  2. Removed the " == 1" part because that is redundant for a boolean.
rdatta95 commented 3 years ago

@Kamuno There are 2 additional things that I wanted to mention.

  1. I am trying to work on Issue #37 . The approach I am thinking is adding a private boolean member to store if the the Win Screen has been closed by the player, or if it is still open. If still open, then I'll redisplay it in the new orientation. Let me know if I should go ahead with this approach.

  2. I really like this project and want to keep contributing to it, but the issue that I am facing is I don't know how to run tests on this project. I am not sure if there is a test suite. The changes that I am doing till now are pretty trivial and should not introduce any errors. But not being able to run a test suite will prevent me from making bigger changes in the future if required. Is there a test suite for this project? If yes, how do I run it? This might be a silly question, but I am new the android environment. So any kind of guidance would be useful.

Kamuno commented 3 years ago

Thank you for the fix. I completely missed this.

  1. I am trying to work on Issue #37 . The approach I am thinking is adding a private boolean member to store if the the Win Screen has been closed by the player, or if it is still open. If still open, then I'll redisplay it in the new orientation. Let me know if I should go ahead with this approach.

I think the main problem is, that the context is leaked with the current approach. While writing this answer and thinking about the problem I fired up AndroidStudio and fixed the issue myself...... sooooo.... you are free to look for anything fun you might want to implement 😄, sorry.

  1. I really like this project and want to keep contributing to it, but the issue that I am facing is I don't know how to run tests on this project. I am not sure if there is a test suite. The changes that I am doing till now are pretty trivial and should not introduce any errors. But not being able to run a test suite will prevent me from making bigger changes in the future if required. Is there a test suite for this project? If yes, how do I run it?

Glad you like it 😃. There are a few tests in \app\src\test\java\org\secuso\privacyfriendlysudoku\. For this project there are no instrumentation tests currently. You can easily run the existing tests with ./gradlew test or just run them from within AndroidStudio. To do that you just open the test class and click the "run" button next to the class name. I will add a CI script that will run the tests automatically on push. The tests are far from complete or even useful tbh. I added them when initially starting the project to test individual components when no UI was present. If you want to add new components or make huge changes, you can just add a new test file - make sure it works on the old version - add your changes and then test again. Or just say what you want to work on and I might add tests.

If you need tests that need a context, you will have to use instrumentation tests.

This might be a silly question, but I am new the android environment. So any kind of guidance would be useful.

There are a lot of good guides out there. I always found that android development is very easy to get into because google is actually doing a pretty good job with providing resources. They have a lot of codelabs, that teach the fundamentals of android development and when you are currently implementing something, the documentation is actually good enough to let you understand what is going on (in AndroidStudio pressing CTRL+Q on a selected class or method will bring up the documentation - or just CTRL+Click to read the entire class 😉).

This project in particular is rather old and was first written for API 16 - 26 or something. Android development is constantly changing. So a lot of the newer best practices are not yet integrated into this project. E.g. the fix you just added should be completely unnecessary because ideally the app should use the MVVM pattern (See https://developer.android.com/jetpack/guide) which mostly gets rid of any configuration change management (atleast stuff like this). This is rather basic but to get started, maybe have a look at this page https://developer.android.com/guide. Apart from that, the only advice I can give is - read code(!) and documentation.