bunk icon indicating copy to clipboard operation
bunk copied to clipboard

Add a Cache.deleteStudent(username) method instead of using Cache.setStudent(username, null)

Open abhijitparida opened this issue 6 years ago • 13 comments

File location: app/src/main/java/app/abhijit/iter/data/Cache.java

~(update/add unit tests accordingly)~

abhijitparida avatar Oct 23 '18 10:10 abhijitparida

I'd like to take this up!

jashasweejena avatar Oct 23 '18 10:10 jashasweejena

Alright! 🎉

abhijitparida avatar Oct 23 '18 10:10 abhijitparida

Do I need to modify only the implementation in data/Cache.java or do I need to modify the method uses too?

jashasweejena avatar Oct 23 '18 12:10 jashasweejena

You also need to change all the method calls. Bonus: add unit tests for the new method.

abhijitparida avatar Oct 23 '18 12:10 abhijitparida

You also need to change all the method calls. Bonus: add unit tests for the new method.

Never added a unit test myself. Could you walk me through briefly, if you don't mind? 😃

jashasweejena avatar Oct 23 '18 12:10 jashasweejena

Could I work on it at the night? if it's all good with you!

jashasweejena avatar Oct 23 '18 12:10 jashasweejena

I fixed the build config issue. Update your fork accordingly. I also recommend using different branches for each pull request. See: https://guides.github.com/introduction/flow/

Never added a unit test myself. Could you walk me through briefly, if you don't mind? 😃

Actually, I haven't set up tests for Cache yet. So you don't have to worry about that.

Here is an example unit test, for Subject: SubjectTest.java

Could I work on it at the night? if it's all good with you!

Take your time!

abhijitparida avatar Oct 23 '18 12:10 abhijitparida

Actually, I haven't set up tests for Cache yet. So you don't have to worry about that.

So I'd have to add tests for the entire 'Cache.java' if I'm not mistaken?

Here is an example unit test, for Subject: SubjectTest.java

Thank you!

Take your time!

Sure 😁

jashasweejena avatar Oct 23 '18 12:10 jashasweejena

No, you don't have to add tests for Cache.

abhijitparida avatar Oct 23 '18 12:10 abhijitparida

Okay. Thank you!

jashasweejena avatar Oct 23 '18 12:10 jashasweejena

No, you don't have to add tests for Cache.

So, all in all, I don't need to work with tests?

jashasweejena avatar Oct 23 '18 12:10 jashasweejena

No. Just ensure that the app builds successfully without any warnings/errors.

abhijitparida avatar Oct 23 '18 13:10 abhijitparida

Hi!

Is this issue taken by anyone?

Thanks! :)

pedroqueiroz avatar Oct 06 '19 03:10 pedroqueiroz