Kotlin migration permission #55
Conversation
| class PermissionManager @Inject | ||
| constructor(private val context: Context, private val requestListeners: SparseArray<PermissionListener>) { |
There was a problem hiding this comment.
Put constructor word on the first line and the parameters on many lines.
class PermissionManager @Inject constructor(
private val context: Context,
private val requestListeners: SparseArray<PermissionListener>
) {
| /** Filters a list of [permissions] and returns only the ones which have not been granted. */ | ||
| private fun filterUngranted(vararg permissions: String): Array<String> { | ||
| val ungranted = ArrayList<String>() | ||
| permissions.filter { permission -> ContextCompat.checkSelfPermission(context, permission) != PackageManager.PERMISSION_GRANTED }.forEach { ungranted.add(it) } |
There was a problem hiding this comment.
Separate this on more lines.
| private fun filterUngranted(vararg permissions: String): Array<String> { | ||
| val ungranted = ArrayList<String>() | ||
| permissions.filter { permission -> ContextCompat.checkSelfPermission(context, permission) != PackageManager.PERMISSION_GRANTED }.forEach { ungranted.add(it) } | ||
| return ungranted.toTypedArray() | ||
| } |
There was a problem hiding this comment.
Use .toTypedArray() after the filter instead of the foreach and the new list.
If you want: 👀
fun filterUngranted(vararg permissions: String) = permissions.filter { permission ->
ContextCompat.checkSelfPermission(context, permission) != PackageManager.PERMISSION_GRANTED
}.toTypedArray()
| private fun allGranted(results: IntArray): Boolean { | ||
| if (results.isEmpty()) return false | ||
| return results.all { it == PackageManager.PERMISSION_GRANTED } | ||
| } |
There was a problem hiding this comment.
You should almost never check a boolean to return another boolean.
fun allGranted(results: IntArray) = results.isNotEmpty() && results.all { it == PackageManager.PERMISSION_GRANTED }
There was a problem hiding this comment.
(I'm loving the usage of the all
| fun requestPermission(activity: Activity, listener: PermissionListener?, vararg permissions: String): Boolean { | ||
| val ungrantedPermissions = filterUngranted(*permissions) | ||
| if (ungrantedPermissions.isNotEmpty()) { | ||
| ActivityCompat.requestPermissions(activity, ungrantedPermissions, requestCount) |
There was a problem hiding this comment.
The only line changed from the previous method is this. We should check how to save this code repetition.
| */ | ||
| public void onPermissionsDenied(@NonNull String[] deniedPermissions) {} | ||
| /** Called when all or some of the [deniedPermissions] rejected by the user */ | ||
| open fun onPermissionsDenied(deniedPermissions: Array<String>) {} |
There was a problem hiding this comment.
Should we return an Array or a List? IMO, a List.
There was a problem hiding this comment.
If I use a list, I have to change a lot of things, in particular in the tests of another classes. Is this OK?
All test passed ok