Mostrando las entradas con la etiqueta refactor. Mostrar todas las entradas
Mostrando las entradas con la etiqueta refactor. Mostrar todas las entradas

lunes, mayo 26, 2014

Eliminar switch

Hace mucho que no programo en java o algo parecido, últimamente hago mucho groovy y estoy aprendiendo clojure, pero bueno, revisando código en el trabajo me topé con una estructura switch, y de inmediato llamo mi atención:


  List<Role> findInviteRoles() {
    List roles = []
    def authorities = springSecurityService.getAuthentication().authorities
    authorities.each {role ->
      switch (role){
        case 'ROLE_ROOT':
          roles += ['ROLE_ROOT', 'ROLE_ADMIN']
          break
        case 'ROLE_ADMIN':
          roles += ['ROLE_ADMIN']
          break
        case 'ROLE_1':
          roles += ['ROLE_2']
          break
      }
    }
    Role.where {
      authority in roles.unique()
    }.list()
  }


A lo cual me pareció que el switch iba sobrado y decidimos hacer un refactor. Se puede ver que ahora están separados los datos de la lógica, lo cuál en algunos casos facilita la mantenibilidad del código.

 List<Role> findInviteRoles() {
    List roles = []
    def permisos = ['ROLE_ROOT':['ROLE_ROOT', 'ROLE_ADMIN'], 
                    'ROLE_ADMIN':['ROLE_ADMIN'], 
                    'ROLE_1':['ROLE_2']]
    def authorities = springSecurityService.getAuthentication().authorities
    roles = authorities.collect {
      permisos."${it}"
    }.flatten().unique()
    Role.where {
      authority in roles
    }.list()
  }

A mi punto de vista queda más limpio y claro el objetivo del collect que del switch. Dejo la prueba unitaria.
  @Unroll
  def "test list granted invite roles with #userRole and result #listResult"() {
    setup: "mockering"
    SpringSecurityService.metaClass.getAuthentication = { [authorities: [userRole]] }
    def springSecurityService = Mock(SpringSecurityService)
    service.springSecurityService = springSecurityService

    systemRoles.each {
      def role = new Role(authority: it)
      role.save validate: false
    }

    when:
    def list = service.findInviteRoles()

    then:
    list?.authority == listResult

    where:
    userRole       | systemRoles                                                || listResult
    'ROLE_ROOT'    | ['ROLE_ADMIN', 'ROLE_ROOT', 'ROLE_GARBAGE']                || ['ROLE_ROOT', 'ROLE_ADMIN']
    'ROLE_ADMIN'   | ['ROLE_ADMIN', 'ROLE_ROOT', 'ROLE_GARBAGE']                || ['ROLE_ADMIN']
    'ROLE_1'       | ['ROLE_ADMIN', 'ROLE_ROOT', 'ROLE_GARBAGE', 'ROLE_2']      || ['ROLE_2']
    'ROLE_2'       | ['ROLE_ADMIN', 'ROLE_ROOT', 'ROLE_GARBAGE', 'ROLE_2']      || []
  }