Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Предложения по улучшению #10

Open
indrih17 opened this issue May 30, 2018 · 2 comments
Open

Предложения по улучшению #10

indrih17 opened this issue May 30, 2018 · 2 comments

Comments

@indrih17
Copy link
Contributor

indrih17 commented May 30, 2018

"menu" переименовал бы на "menuId"
"click" на "clickListener"
fun processTitle(textTitle: TextView) на fun TextView.changeTitile()

var itemLayoutId = R.layout.item_linear`
if (layoutManager is GridLayoutManager) {
    itemLayoutId = R.layout.item_grid
}

Я бы поменял на

val itemLayoutId = when (layoutManager) {
    is GridLayoutManager -> R.layout.item_grid
    else -> R.layout.item_linear
}

Ещё бы из функции fun processRecycler(recycler: RecyclerView, dialog: BottomSheetDialog) вынес
if (menu > 0){}, и саму функцию вообще убрал бы. Например, вот так:

fun show(context: Context) {
    ...
    val recycler = root.findViewById(R.id.recycler_view) as RecyclerView
    if (menuId > 0) {
        if (layoutManager == null) {
            layoutManager = createVerticalLinearLayoutManager()
        }
        val itemLayoutId = when (layoutManager) {
            is GridLayoutManager -> R.layout.item_grid
            else -> R.layout.item_linear
        }
        if (adapter == null) {
            adapter = createAdapter(dialog, itemLayoutId)
        }
        recyclerView.adapter = adapter
        recyclerView.layoutManager = layoutManager
    }
    ...
}
MenuItem.OnMenuItemClickListener({
    click.onMenuItemClick(it)
    dialog.cancel()
    true
})

поменял бы на

MenuItem.OnMenuItemClickListener {
    click.onMenuItemClick(it)
    dialog.cancel()
    true
}
protected open fun processGrid(root: View) {
    if (root.findViewById<View>(R.id.text_title).visibility != View.VISIBLE) {
        if (layoutManager is GridLayoutManager) {
            root.marginTop(24)
        }
    }
}

изменил на

protected open fun GridLayoutManager.setTopMarginIfTitleIsVisible(root: View) {
    if (root.findViewById<View>(R.id.text_title).visibility != View.VISIBLE) {
        root.marginTop(24)
    }
}
@indrih17
Copy link
Contributor Author

А ещё из fun show(context: Context) вынес контекст и убрал бы его в конструктор.
И кстати, почему fun processClick(dialog: BottomSheetDialog) нигде не используется? Это нормально?

@indrih17
Copy link
Contributor Author

indrih17 commented May 30, 2018

И ещё как в анко можно сделать:
fun Context.sheetMenu(init: SheetMenu.() -> Unit) = SheetMenu(this).apply { init() }

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

No branches or pull requests

1 participant