``` companion object { fun <T> fromD...

# codereview

[ivano]

companion object {
        fun <T> fromData(data: T): Result<T> {

            return Result(data, null)
        }

        fun <T> fromError(error: Throwable): Result<T> {

            return when (error) {
                is HttpException -> {
                    if (error.code() in 400..499) {

                        if (error.code() == 401) AccountStorage().signOut()

                        mapErrorBody(error, PAErrorModel::class.java)?.let {
                            return Result(null, it)
                        } ?: return Result(null, PAErrorModel("", error.localizedMessage))
                    }

                    return Result(null, PAErrorModel("", error.localizedMessage))
                }
                else -> Result(null, error)
            }
        }
    }

[ivano]

I solved as this!

[ivano]

class Result<T>(val data: T? = null, val error: Throwable? = null) {

    companion object {
        fun <T> fromData(data: T): Result<T> {

            return Result(data, null)
        }

        fun <T> fromError(error: Throwable): Result<T> {

            return if (error is HttpException) {
                manageCodeHttpException(error)
            } else Result(null, error)
        }

        private fun <T> manageCodeHttpException(error: HttpException): Result<T> {

            return checkRange(error)
                

        }

        private fun <T> checkRange(error: HttpException): Result<T> {
            if (error.code() in 400..499) {
                if (error.code() == 401) AccountStorage().signOut()

                mapErrorBody(error, PAErrorModel::class.java)?.let {
                    return@let Result(null, it)
                } ?: return Result(null, PAErrorModel("", error.localizedMessage))
            }

            return Result(null, PAErrorModel("", error.localizedMessage))
        }
    }

[Mike]

you could also remove all the return statements in the is HttpException, and add an else to the if(error.code() in 400..499) to wrap the Result(null, PAErrorModel("", error.localizedMessage) return value. This will keep it all in one function.

[Mike]

And I might do additional refactoring as all the branches inside the HttpException return a Result with the first element null anyway.

[Luis Munoz]

Untitled

[Luis Munoz]

in my opinion the number 1 problem with code I see is functions doing too much

[Luis Munoz]

easier to test, understand, and moves things around if everyone just made their functions smaller

[Mike]

@Luis Munoz About the only thing I'd suggest is relying on the fact that things are expressions rather than the explicit return statements in your functions.

[ivano]

thank you for the nice suggestions