drl959975
2018-05-30 05:56
浏览 52
已采纳

可以使用许多匿名函数吗? [关闭]

One thing I love from golang is defer statement, but defer only work on func scope.

So, I very often to use it like this

func (s *Something) abc() error {
    func() {
        s.Lock()
        defer s.Unlock()
        // don't lock too long
    }()
    // do something else
    if err := func() error {
        resp, err := http.Get("https://example.com/api")
        if err != nil {
            return err
        }
        defer resp.Body.Close()
        if resp.StatusCode != 200 {
            return errors.New("Failed to download")
        }
        var tmp struct {
            Error bool    `json:"error"`
            Result string `json:"result"`
        }
        if err := json.NewDecoder(resp.Body).Decode(&tmp); err != nil {
            return err
        }
        if tmp.Error {
            return errors.New("API return error")
        }
        s.somedata = tmp.result
        return nil
    }(); err != nil {
        return err
    }
    func() {
        s.Lock()
        defer s.Unlock()
        // don't lock too long
    }()
    // do something else
}

basically, I wrap it into anonymous block func.

Is it common to use something like this? do any other gophers abuse this fact?

EDIT: Clarification

Okay, it looks like I didn't explain it well, There are 2 things I want to achieve

  1. What I want to achieve is to lock mutex as short as possible

    func() {
        s.Lock()
        defer s.Unlock()
        // don't lock too long
    
        // there is other code here, this func is not an empty func
    }()
    
  2. There more than one http.Get in this function, let's say after calling example.com/api I need to call example.com/api2 . We need to close resp.Body as soon as possible, so only one TCP connection is made to this server. AFAIK, http.Get will create another TCP connection if there is another HTTP connection that not closed yet (resp.Body.Close() is not called on previous response).

EDIT 2: more clarification

first and last anonymous function and lock are to sync cache. I implement cache based on map[string]string, so it needs to be sync-ed.

I need to call example.com/api first, and based on the response I need to call example.com/api2 or example.com/api3, and at that time previous http connection must be closed, it can be code like this

resp, err := http.Get("https://example.com/api")
if err != nil {
    return err
}
if resp.StatusCode != 200 {
    resp.Body.Close()
    return errors.New("Failed to download")
}
// process the body
resp.Body.Close()

but you need to explicitly write resp.Body.Close() twice

  • 写回答
  • 好问题 提建议
  • 关注问题
  • 收藏
  • 邀请回答

2条回答 默认 最新

  • drlndkhib08556095 2018-05-30 08:07
    已采纳

    You're correct in the observation that defer only works in function scope, I think that the problem you are running into is nothing to do with the behaviour of defer.

    In general it's [insert opinion] better to have smaller functions, then you would not need to create anonymous functions to maximise the usage of defer.

    Splitting up your example, something like this might be better:

    // Public method that does locking orchestration
    func (s *Something) PublicDoABC() error {
        s.doWorkStart()
        if err := s.populateSomeData(); err != nil {
            return err
        }
        s.doWorkEnd()
        return nil
    }
    
    // setup function with locking
    func (s *Something) doWorkStart() {
        s.Lock()
        defer s.Unlock()
    
        // do setup work here
    }
    
    // teardown function with locking
    func (s *Something) doWorkEnd() {
        s.Lock()
        defer s.Unlock()
    
        // do teardown work here
    }
    
    // do the actual request
    func (s *Something) populateSomeData() error {
        resp, err := http.Get("https://example.com/api")
        if err != nil {
            return err
        }
        defer resp.Body.Close()
        if resp.StatusCode != 200 {
            return errors.New("Failed to download")
        }
        var tmp struct {
            Error bool    `json:"error"`
            Result string `json:"result"`
        }
        if err := json.NewDecoder(resp.Body).Decode(&tmp); err != nil {
            return err
        }
        if tmp.Error {
            return errors.New("API return error")
        }
        s.somedata = tmp.Result
        return nil
    }
    

    I know this changes things, and you might not want to split out the body of the function, you could define a method on Something that allows arbitrary functions to be executed with locking:

    func (s *Something) PublicDoABC() error {
        s.executeLocked(func() {
            // do setup work
        })
        if err := s.populateSomeData(); err != nil {
            return err
        }
    
        s.executeLocked(func() {
            // do teardown work
        })
    
        return nil
    }
    
    // this function allows defer unlocks and unifies locking code
    func (s *Something) executeLocked(f func()) {
        s.Lock()
        defer s.Unlock()
        f()
    }
    

    To answer the original question: No, I do not believe this is that common (having multiple inline anon function). If it feels wrong, there's almost certainly a better way of doing it.

    已采纳该答案
    评论
    解决 无用
    打赏 举报
  • dqd78456 2018-05-30 06:14

    Due to your anonymous functions to Lock and Unlock you'll enter the anonymous function, lock, defer an unlock, leave the anonymous function triggering the deferred unlock, and then continue abc(). The beginning and ending anonymous functions are pretty much useless.

    Your middle anonymous function is bad style for any language. There's no reason to just not do the code and respond to any returned errors.

    I would rewrite this function as follows:

    func (s *Something) abc() error {
        s.Lock()
        defer s.Unlock()
    
        resp, err := http.Get("https://example.com/api")
        if err != nil {
                return err
        }
        defer resp.Body.Close()
        if resp.StatusCode != 200 {
                return errors.New("Failed to download")
        }
        // This struct should probably be global
        var tmp struct {
                Error bool    `json:"error"`
                Result string `json:"result"`
        }
        err = json.NewDecoder(resp.Body).Decode(&tmp)
        if err != nil {
                return err
        }
        if tmp.Error {
                return errors.New("API return error")
        }
        s.somedata = tmp.result
        return nil
    }
    

    Anonymous functions have their place. I've used them for closures as well as extremely concise go routines. One place you might use an anonymous function here is to log an error returned by resp.Body.Close() like so:

    defer func() {
        err = resp.Body.Close()
        log.Printf("error closing API response body: %s", err)
    }()
    
    评论
    解决 无用
    打赏 举报

相关推荐 更多相似问题