Qr Reader #229

MaximeZmt · 2022-05-02T11:01:13Z

Qr Reader

This is related to issue #221

This Pull Requests contains:

UserSearchActivity: Add of a button to go to QR Code Scan Activity
QrCodeActivity: QR code scanner that add friends

In addition the code is tested with a coverage of: [64.8]%

Notes for reviewers

Low coverage because impossible (very difficult) to mock the camera

codeclimate · 2022-05-02T11:21:27Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+    }
+
+
+    /**


Method setupControls has 56 lines of code (exceeds 25 allowed). Consider refactoring.

codeclimate · 2022-05-02T11:21:27Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+
+        binding.cameraSurfaceView.getHolder().addCallback(object : SurfaceHolder.Callback {
+            @SuppressLint("MissingPermission")
+            override fun surfaceCreated(holder: SurfaceHolder) {


Identical blocks of code found in 2 locations. Consider refactoring.

codeclimate · 2022-05-02T11:21:27Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+                format: Int,
+                width: Int,
+                height: Int
+            ) {


Identical blocks of code found in 2 locations. Consider refactoring.

the body of surfaceChanged and surfaceCreated are identical, refactor may also boost the coverage

codeclimate · 2022-05-02T11:21:27Z

app/src/main/java/ch/sdp/vibester/activity/SearchUserActivity.kt

@@ -38,6 +44,14 @@ class SearchUserActivity : AppCompatActivity() {
        searchEditText = findViewById(R.id.searchUserET)
        searchForUsers("")

+        val buttonScan: FloatingActionButton = findViewById(R.id.searchUser_scanning)
+
+        buttonScan.setOnClickListener {


Similar blocks of code found in 2 locations. Consider refactoring.

codeclimate · 2022-05-03T09:26:25Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+    val usersRepo: DataGetter = DataGetter()
+    var isTest: Boolean = false
+
+    override fun onCreate(savedInstanceState: Bundle?) {


Method onCreate has 29 lines of code (exceeds 25 allowed). Consider refactoring.

will be a good practice to break onCreate down into smaller functions.

codeclimate · 2022-05-03T09:26:25Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+    }
+
+
+    /**


Method setupControls has 45 lines of code (exceeds 25 allowed). Consider refactoring.

codeclimate · 2022-05-03T19:44:29Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+     * This section is related to the handler when code is detected
+     */
+
+    private fun solveDetection(detections: Detector.Detections<Barcode>) {


Method solveDetection has a Cognitive Complexity of 23 (exceeds 20 allowed). Consider refactoring.

codeclimate · 2022-05-03T19:44:30Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+     * This section is related to the handler when code is detected
+     */
+
+    private fun solveDetection(detections: Detector.Detections<Barcode>) {


Method solveDetection has 26 lines of code (exceeds 25 allowed). Consider refactoring.

codeclimate · 2022-05-03T22:52:05Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+    var uidList: ArrayList<String> = ArrayList()
+    val usersRepo: DataGetter = DataGetter()
+
+    override fun onCreate(savedInstanceState: Bundle?) {


Method onCreate has 31 lines of code (exceeds 25 allowed). Consider refactoring.

codeclimate · 2022-05-03T22:52:05Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+    }
+
+
+    /**


Method setupControls has 42 lines of code (exceeds 25 allowed). Consider refactoring.

jiabaow

Nice work! I left some comments concerning breaking down large functions into multiple smaller functions and duplications.

jiabaow · 2022-05-04T06:11:30Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+        if (extras != null) {
+            uidList = extras.get("uidList") as ArrayList<String>
+            isTest = extras.getBoolean("isTest", false)
+        }else{


Suggested change

}else{

} else {

jiabaow · 2022-05-04T06:12:10Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+            AnimationUtils.loadAnimation(this@QrScanningActivity, R.anim.qr_anim)
+        binding.barcodeLine.startAnimation(aniSlide)
+
+        if(isTest){


Suggested change

if(isTest){

if (isTest) {

jiabaow · 2022-05-04T06:13:07Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+    val usersRepo: DataGetter = DataGetter()
+    var isTest: Boolean = false
+
+    override fun onCreate(savedInstanceState: Bundle?) {


will be a good practice to break onCreate down into smaller functions.

jiabaow · 2022-05-04T06:16:17Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+                format: Int,
+                width: Int,
+                height: Int
+            ) {


the body of surfaceChanged and surfaceCreated are identical, refactor may also boost the coverage

jiabaow · 2022-05-04T06:17:23Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+                finish()
+            } else {
+                // Camera permission not granted, come back to previous activity
+                Toast.makeText(applicationContext, "Camera Permission should be granted for that feature", Toast.LENGTH_LONG).show()


can use string.xml

jiabaow · 2022-05-04T06:17:32Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+
+        barcodeDetector.setProcessor(object : Detector.Processor<Barcode> {
+            override fun release() {
+                Toast.makeText(applicationContext, "Scanner has been closed", Toast.LENGTH_SHORT).show()


can use string.xml

jiabaow · 2022-05-04T06:17:47Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+    /*
+     * This section is related to the handler when code is detected
+     */
+


Suggested change

jiabaow · 2022-05-04T06:18:14Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+                    }
+                    Toast.makeText(this@QrScanningActivity, "Congratulations, you have a new friends", Toast.LENGTH_SHORT).show()
+                    endInt.putExtra("isSuccess", true)
+                }else{


Suggested change

}else{

} else {

jiabaow · 2022-05-04T06:18:33Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+                    if (!isTest) {
+                        usersRepo.setSubFieldValue(FireBaseAuthenticator.getCurrentUID(),"friends", scannedValue, true)
+                    }
+                    Toast.makeText(this@QrScanningActivity, "Congratulations, you have a new friends", Toast.LENGTH_SHORT).show()


Suggested change

Toast.makeText(this@QrScanningActivity, "Congratulations, you have a new friends", Toast.LENGTH_SHORT).show()

Toast.makeText(this@QrScanningActivity, "Congratulations, you have a new friend", Toast.LENGTH_SHORT).show()

jiabaow · 2022-05-04T06:19:49Z

app/src/main/java/ch/sdp/vibester/database/DataGetter.kt

@@ -136,7 +136,9 @@ class DataGetter @Inject constructor() {
     * @param searchInput search text inputed by user
     * @param callback function to call with found users by username
     */
-    fun searchByField(field: String, searchInput: String, callback:(ArrayList<User>) -> Unit) {
+    fun searchByField(field: String, searchInput: String, callback:(ArrayList<User>) -> Unit, callback2 :(ArrayList<String>) -> Unit) {


don't forget to update the function documentation.

and maybe give callback and callback2 more meaningful names

kamilababayeva

I have did a partial review. I wanna give it a second look later! thank you for the changes! The feature is cool and the moving bar in an old way is the best.

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

app/src/main/java/ch/sdp/vibester/activity/SearchUserActivity.kt

app/src/androidTest/java/ch/sdp/vibester/activity/QrScanningActivityTest.kt

codeclimate · 2022-05-04T18:42:51Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+        }
+    }
+
+    /**


Method setupControls has 35 lines of code (exceeds 25 allowed). Consider refactoring.

codeclimate · 2022-05-04T18:42:51Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+                    if (!isTest) {
+                        usersRepo.setSubFieldValue(FireBaseAuthenticator.getCurrentUID(),"friends", scannedValue, true)
+                    }
+                    Toast.makeText(this@QrScanningActivity, getString(R.string.qrScanning_newFriend), Toast.LENGTH_SHORT).show()


Similar blocks of code found in 2 locations. Consider refactoring.

codeclimate · 2022-05-04T18:42:51Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+                    Toast.makeText(this@QrScanningActivity, getString(R.string.qrScanning_newFriend), Toast.LENGTH_SHORT).show()
+                    startActivityWExtra(Intent(this@QrScanningActivity, SearchUserActivity::class.java), "isSuccess", true)
+                } else {
+                    Toast.makeText(this@QrScanningActivity, getString(R.string.qrScanning_noExistUid), Toast.LENGTH_SHORT).show()


Similar blocks of code found in 2 locations. Consider refactoring.

codeclimate · 2022-05-04T18:42:51Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+
+        barcodeDetector.setProcessor(object : Detector.Processor<Barcode> {
+            override fun release() {
+                Toast.makeText(applicationContext, getString(R.string.qrScanning_scannerClosed), Toast.LENGTH_SHORT).show()


Similar blocks of code found in 2 locations. Consider refactoring.

codeclimate · 2022-05-04T18:42:51Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+                startActivityWExtra(Intent(this@QrScanningActivity, QrScanningActivity::class.java), "uidList", uidList)
+            } else {
+                // Camera permission not granted, come back to previous activity
+                Toast.makeText(applicationContext, getString(R.string.qrScanning_cameraError), Toast.LENGTH_LONG).show()


Similar blocks of code found in 2 locations. Consider refactoring.

will having a function for toast solve the problem?

kamilababayeva

Looks good to me! thanks for a nice feature!

kamilababayeva · 2022-05-04T18:58:57Z

app/src/main/java/ch/sdp/vibester/activity/QrScanningActivity.kt

+                startActivityWExtra(Intent(this@QrScanningActivity, QrScanningActivity::class.java), "uidList", uidList)
+            } else {
+                // Camera permission not granted, come back to previous activity
+                Toast.makeText(applicationContext, getString(R.string.qrScanning_cameraError), Toast.LENGTH_LONG).show()


will having a function for toast solve the problem?

jiabaow

LGTM but don't forget to change the string.xml

jiabaow · 2022-05-04T20:43:02Z

app/src/main/res/values/strings.xml

+    <string name="qrScanning_scannerClosed">Scanner has been closed</string>
+    <string name="qrScanning_cameraError">Camera Permission should be granted for that feature</string>
+    <string name="qrScanning_noExistUid">Error not an existing uid</string>
+    <string name="qrScanning_newFriend">Congratulations, you have a new friends</string>


Suggested change

<string name="qrScanning_newFriend">Congratulations, you have a new friends</string>

<string name="qrScanning_newFriend">Congratulations, you have a new friend</string>

Co-authored-by: jiabaow <[email protected]>

codeclimate · 2022-05-05T08:22:54Z

Code Climate has analyzed commit a766e60 and detected 7 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	3
Duplication	4

The test coverage on the diff in this pull request is 71.7% (80% is the threshold).

This pull request will bring the total coverage in the repository to 84.0% (0.0% change).

View more on Code Climate.

Qr Reader first version

80a90af

codeclimate bot reviewed May 2, 2022

View reviewed changes

MaximeZmt added 3 commits May 2, 2022 22:45

improve test qrcode

d9e5c7d

update weird test infrastructure

f559fd7

attempt removing test

c6d7eaa

codeclimate bot reviewed May 3, 2022

View reviewed changes

MaximeZmt added 7 commits May 3, 2022 11:28

reenable qr test

40b7ddb

merge

5a79b0d

merge 2

fe52656

sync build gradle

5cae698

fix test

9d3fc53

last modif attempt

91bb070

updated with callback

82ffce0

codeclimate bot reviewed May 3, 2022

View reviewed changes

MaximeZmt added 2 commits May 3, 2022 21:52

Merge branch 'main' into maximezmt/qrreader

2da5ed1

update testing

3cb406f

MaximeZmt marked this pull request as ready for review May 3, 2022 22:38

MaximeZmt changed the title ~~Qr Reader first version~~ Qr Reader May 3, 2022

MaximeZmt linked an issue May 3, 2022 that may be closed by this pull request

identify the QR code and add it to the user's friend list.(#220) #221

Closed

MaximeZmt self-assigned this May 3, 2022

MaximeZmt added the actual: 8h label May 3, 2022

codeclimate bot reviewed May 3, 2022

View reviewed changes

jiabaow self-requested a review May 4, 2022 06:08

jiabaow reviewed May 4, 2022

View reviewed changes

kamilababayeva reviewed May 4, 2022

View reviewed changes

app/src/androidTest/java/ch/sdp/vibester/activity/QrScanningActivityTest.kt Outdated Show resolved Hide resolved

address tests

44f4ba3

MaximeZmt added actual: 10h actual time to fix and removed actual: 8h labels May 4, 2022

attempt without jcenter

4bd7e66

codeclimate bot reviewed May 4, 2022

View reviewed changes

kamilababayeva approved these changes May 4, 2022

View reviewed changes

jiabaow approved these changes May 4, 2022

View reviewed changes

MaximeZmt and others added 10 commits May 4, 2022 23:10

Merge branch 'main' into maximezmt/qrreader

ff532c6

Update app/src/main/res/values/strings.xml

5601a74

Co-authored-by: jiabaow <[email protected]>

correct activity merging

ff33575

update xml

2e997af

Merge branch 'main' into maximezmt/qrreader

6f7e1b5

revert update for Jiabao's code

5527d5a

remove some test

e0467ea

remove some test 2

49d1499

reenable some tests

f12f16b

reenable some tests 2

a766e60

MaximeZmt merged commit 99d421e into main May 5, 2022

MaximeZmt deleted the maximezmt/qrreader branch May 5, 2022 08:24

	Toast.makeText(this@QrScanningActivity, "Congratulations, you have a new friends", Toast.LENGTH_SHORT).show()
	Toast.makeText(this@QrScanningActivity, "Congratulations, you have a new friend", Toast.LENGTH_SHORT).show()

+                      }
+                  }
+                  /**

	<string name="qrScanning_newFriend">Congratulations, you have a new friends</string>
	<string name="qrScanning_newFriend">Congratulations, you have a new friend</string>

Qr Reader #229

Qr Reader #229

Conversation

MaximeZmt commented May 2, 2022 • edited Loading

Qr Reader

This is related to issue #221

Notes for reviewers

codeclimate bot May 2, 2022

Choose a reason for hiding this comment

codeclimate bot May 2, 2022

Choose a reason for hiding this comment

codeclimate bot May 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot May 2, 2022

Choose a reason for hiding this comment

codeclimate bot May 3, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot May 3, 2022

Choose a reason for hiding this comment

codeclimate bot May 3, 2022

Choose a reason for hiding this comment

codeclimate bot May 3, 2022

Choose a reason for hiding this comment

codeclimate bot May 3, 2022

Choose a reason for hiding this comment

codeclimate bot May 3, 2022

Choose a reason for hiding this comment

jiabaow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kamilababayeva left a comment

Choose a reason for hiding this comment

codeclimate bot May 4, 2022

Choose a reason for hiding this comment

codeclimate bot May 4, 2022

Choose a reason for hiding this comment

codeclimate bot May 4, 2022

Choose a reason for hiding this comment

codeclimate bot May 4, 2022

Choose a reason for hiding this comment

codeclimate bot May 4, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kamilababayeva left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiabaow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot commented May 5, 2022

MaximeZmt commented May 2, 2022 •

edited

Loading