CODEWITHSUNDEEP
typesgo
Maxim 'Zert' Treskin
Maxim 'Zert' Treskin

Reputation: 330

Type casting issue in go

How I can rewrite following code? 

switch md.(type) {
    case *amf0.EcmaArrayType:
        ea := md.(*amf0.EcmaArrayType)
        for k, v := range (*ea) {
            log.Printf("%v = %v\n", k, v)
        }
        if width == 0 {width = uint16((*ea)["width"].(amf0.NumberType))}
        if height == 0 {height = uint16((*ea)["height"].(amf0.NumberType))}
    case *amf0.ObjectType:
        ea := md.(*amf0.ObjectType)
        for k, v := range (*ea) {
            log.Printf("%v = %v\n", k, v)
        }
        if width == 0 {width = uint16((*ea)["width"].(amf0.NumberType))}
        if height == 0 {height = uint16((*ea)["height"].(amf0.NumberType))}
}

It has two completely duplicated blocks for different types. If I declared var ea interface{} above switch state, I cannot call range (*ea) due to compilation error.

Upvotes: 0

Views: 253

Answers (3)

Stephen Weinberg
Stephen Weinberg

Reputation: 53398

It looks like both of these types have the underlying type map[string]something where "something" has a concrete type of amf0.NumberType. Every operation you did could be emulated using reflection.

switch md.(type) {
case *amf0.EcmaArrayType, *amf0.ObjectType:
    m := reflect.Indirect(reflect.ValueOf(md))
    for _, key := range m.MapKeys() {
        k, v := key.Interface(), m.MapIndex(key).Interface()
        log.Printf("%v = %v\n", k, v)
    }

    if width == 0 {
        w := m.MapIndex(reflect.ValueOf("width"))
        width = uint16(w.Interface().(amf0.NumberType))
    }

    if height == 0 {
        h := m.MapIndex(reflect.ValueOf("height"))
        height = uint16(h.Interface().(amf0.NumberType))
    }
}

It is however not uncommon to do what you did in your first example. There are times when reflect won't cut it. In those cases, I have some advice for your type switch. Instead of switch md.(type) do switch ea := md.(type). That would allow you to remove lines like ea := md.(*amf0.EcmaArrayType).

DRY code is nicer to work with. It makes making changes quicker and is less prone to bugs. However, when all your duplicated code is in one place (like a type switch), the chances of bugs are slim. It still takes longer to make changes, but it is not nearly as bad as duplicated code throughout your project. Don't fear heavily duplicated type switches like you would other duplicated code.

Upvotes: 2

Sonia
Sonia

Reputation: 28345

In this specific case, I might just add comments to future maintainers pointing out the duplicate code, or I might remove it as follows. (playground: http://play.golang.org/p/Vc9pOZSNoW)

package main

import "log"

// copied from package amf0
type NumberType float64
type StringType string
type _Object map[StringType]interface{}
type ObjectType _Object
type EcmaArrayType _Object

// test parameter.  comment out one or the other
// var md interface{} = &ObjectType{"height": NumberType(3), "width": NumberType(5)}
var md interface{} = &EcmaArrayType{"height": NumberType(5), "width": NumberType(7)}

func main() {
    var width, height uint16
    ea, ok := md.(*ObjectType)
    if !ok {
        if et, ok := md.(*EcmaArrayType); ok {
            ea = (*ObjectType)(et)
        }
    }
    if ea != nil {
        for k, v := range *ea {
            log.Printf("%v = %v\n", k, v)
        }
        if width == 0 {
            width = uint16((*ea)["width"].(NumberType))
        }
        if height == 0 {
            height = uint16((*ea)["height"].(NumberType))
        }
    }
}

The duplicated code in your original was only duplicated source code; since it was handling different types, the compiled code was different. Fortunately though, the compiled code of the ObjectType case can easily handle the EcmaArrayType case with the simple type conversion of ea = (*ObjectType)(et).

Upvotes: 1

Yogendra Singh
Yogendra Singh

Reputation: 34367

Use the type casting before calling range e.g.

     range ((* your_desired_type)(*ea))

Replace the your_desired_type with your actual type for type casting.

Upvotes: 0

Related Questions