Skip to main content
added 2 characters in body
Source Link
ferada
  • 11.1k
  • 25
  • 63
type User struct {
    Name     string `json:"name"`
    LastName string `json:"lastname"`
}

func (u *User) toMap() map[string]interface{} {
    return map[string]interface{}{
        "1": u.Name,
        "2": u.LastName,
    }
}

func HandleLambda(event events.SQSEvent) {
    userRecords := make([]map[string]interface{}, 0, len(event.Records))

    for _, record := range event.Records {
        ev, err := event.NewEvent(record.Body)
        if err != nil {
            continue
        }

        user, err := repository.NewUser(ev)
        if err != nil {
            continue
        }

        userRecords = append(userRecords, user.toMap())
    }

    uJSON, _ := json.Marshal(userRecords)
    // I send this to external service
}
type User struct {
    Name     string `json:"name"`
    LastName string `json:"lastname"`
}

func (u *User) toMap() map[string]interface{} {
    return map[string]interface{
        "1": u.Name,
        "2": u.LastName,
    }
}

func HandleLambda(event events.SQSEvent) {
    userRecords := make([]map[string]interface, 0, len(event.Records))

    for _, record := range event.Records {
        ev, err := event.NewEvent(record.Body)
        if err != nil {
            continue
        }

        user, err := repository.NewUser(ev)
        if err != nil {
            continue
        }

        userRecords = append(userRecords, user.toMap())
    }

    uJSON, _ := json.Marshal(userRecords)
    // I send this to external service
}
type User struct {
    Name     string `json:"name"`
    LastName string `json:"lastname"`
}

func (u *User) toMap() map[string]interface{} {
    return map[string]interface{}{
        "1": u.Name,
        "2": u.LastName,
    }
}

func HandleLambda(event events.SQSEvent) {
    userRecords := make([]map[string]interface{}, 0, len(event.Records))

    for _, record := range event.Records {
        ev, err := event.NewEvent(record.Body)
        if err != nil {
            continue
        }

        user, err := repository.NewUser(ev)
        if err != nil {
            continue
        }

        userRecords = append(userRecords, user.toMap())
    }

    uJSON, _ := json.Marshal(userRecords)
    // I send this to external service
}
added 72 characters in body
Source Link
ferada
  • 11.1k
  • 25
  • 63
  • for idx := range event.Records might as well be for idx, record := event.Records, no need have an extra line to get the actual item. And if you're never using it, for _, record := event.Records even.
  • Errors are ignored? Consider logging them if don't error our immediately.
  • user := make(map ...) I'd usually make that a literal to avoid repeating myself even more.
  • userRecords could perhaps already be pre-allocated to the right size, then there'd. Then there'd be no need for append at all. Or, orlike here, if you're usingyou still need to use continue like here, pre-allocateallocating the maximum length it could take, so it definitely won'twouldn't have to be resized might also be a good idea. Usually this also doesn't matter that much if you don't have a lot of entries, but it's good to keep in mind for situations where it does matter.
  • Also consider making the to-map conversion a new method on User, or a helper function, you might want to test it or reuse in other places.
  • repository looks like a global variable, that's usually not the best idea. Consider making that a parameter for HandleLambda, or making HandleLambda a method and put the repository as one of its members. It will help with testability in the end to not have dependencies scattered around like that.
  • for idx := range event.Records might as well be for idx, record := event.Records, no need have an extra line to get the actual item. And if you're never using it, for _, record := event.Records even.
  • Errors are ignored? Consider logging them if don't error our immediately.
  • user := make(map ...) I'd usually make that a literal to avoid repeating myself even more.
  • userRecords could already be pre-allocated to the right size, then there'd be no need for append, or if you're using continue like here, pre-allocate the maximum length so it definitely won't have to be resized. Usually also doesn't matter that much if you don't have a lot of entries, but it's good to keep in mind for situations where it does matter.
  • Also consider making the to-map conversion a new method on User, or a helper function, you might want to test it or reuse in other places.
  • repository looks like a global variable, that's usually not the best idea. Consider making that a parameter for HandleLambda, or making HandleLambda a method and put the repository as one of its members. It will help with testability in the end to not have dependencies scattered around like that.
  • for idx := range event.Records might as well be for idx, record := event.Records, no need have an extra line to get the actual item. And if you're never using it, for _, record := event.Records even.
  • Errors are ignored? Consider logging them if don't error our immediately.
  • user := make(map ...) I'd usually make that a literal to avoid repeating myself even more.
  • userRecords could perhaps already be pre-allocated to the right size. Then there'd be no need for append at all. Or, like here, if you still need to use continue, pre-allocating the maximum length it could take, so it definitely wouldn't have to be resized might also be a good idea. Usually this also doesn't matter that much if you don't have a lot of entries, but it's good to keep in mind for situations where it does matter.
  • Also consider making the to-map conversion a new method on User, or a helper function, you might want to test it or reuse in other places.
  • repository looks like a global variable, that's usually not the best idea. Consider making that a parameter for HandleLambda, or making HandleLambda a method and put the repository as one of its members. It will help with testability in the end to not have dependencies scattered around like that.
Source Link
ferada
  • 11.1k
  • 25
  • 63

I guess I might as well then. To prefix that with: It looks fine as is, there's a few details that could be improved, although those also depend on your team and style guide. Also read it in the context of a bigger project, not everything always applies, especially if it's one-off code, etc.

I'm gonna assume (since the formatting is a bit off) that you're using gofmt or goimports already, if you don't, check them out, it helps immensely.

For the User struct, even if the JSON name is lastname, consider the Go-side name to be LastName to keep in line with normal style guides. Depending on whether you need it the JSON annotation could also use an omitempty, but that depends on your external service of course.

For the HandleLambda loop a few things stand out:

  • for idx := range event.Records might as well be for idx, record := event.Records, no need have an extra line to get the actual item. And if you're never using it, for _, record := event.Records even.
  • Errors are ignored? Consider logging them if don't error our immediately.
  • user := make(map ...) I'd usually make that a literal to avoid repeating myself even more.
  • userRecords could already be pre-allocated to the right size, then there'd be no need for append, or if you're using continue like here, pre-allocate the maximum length so it definitely won't have to be resized. Usually also doesn't matter that much if you don't have a lot of entries, but it's good to keep in mind for situations where it does matter.
  • Also consider making the to-map conversion a new method on User, or a helper function, you might want to test it or reuse in other places.
  • repository looks like a global variable, that's usually not the best idea. Consider making that a parameter for HandleLambda, or making HandleLambda a method and put the repository as one of its members. It will help with testability in the end to not have dependencies scattered around like that.
type User struct {
    Name     string `json:"name"`
    LastName string `json:"lastname"`
}

func (u *User) toMap() map[string]interface{} {
    return map[string]interface{
        "1": u.Name,
        "2": u.LastName,
    }
}

func HandleLambda(event events.SQSEvent) {
    userRecords := make([]map[string]interface, 0, len(event.Records))

    for _, record := range event.Records {
        ev, err := event.NewEvent(record.Body)
        if err != nil {
            continue
        }

        user, err := repository.NewUser(ev)
        if err != nil {
            continue
        }

        userRecords = append(userRecords, user.toMap())
    }

    uJSON, _ := json.Marshal(userRecords)
    // I send this to external service
}