[nanomsg] Re: Review of mangos

  • From: Gonzalo Diethelm <gonzalo.diethelm@xxxxxxxxx>
  • To: nanomsg <nanomsg@xxxxxxxxxxxxx>
  • Date: Tue, 6 May 2014 12:52:16 -0400

Garrett, my apologies for misspelling your name!



On Tue, May 6, 2014 at 12:46 PM, Gonzalo Diethelm <
gonzalo.diethelm@xxxxxxxxx> wrote:

> Hello Garret,
>
> I am going over your mangos code in order to improve my go-fu. Great job!
>
> I started with macat.go, to make things easier for me, and I have several
> questions / suggestions for you, which you will find below. I can provide
> patches for anything you agree should be modified, but remember some / all
> of these may be wrong, due to my ignorance of go.
>
> Thanks and best regards.
>
> ---
>
> # macat.go
>
> Line 73: are you missing a check for if err != nil?
>
> Line 128: isn't it necessary to close the file (maybe in a defer)?
>
> Line 169, 177: should you check for the existence (and readability) of the
> file?
>
> Line 273, 281, 290, 317: this code is repeated four times; move it to a
> helper?
>
> Line 376: you are creating the Writer even if it will not be used (when
> format is "no"). Additionally, you are creating the Writer each time you
> call printMsg(); can that be optimized (and is it necessary)?
>
> Line 393: maybe add some other special cases, like \t and \'.
>
> Line 457: I don't understand why there is a loop in there; are you sending
> many copies of sendData? Or is it because the other side may not be up yet?
>
> Line 458, 491: this code is repeated twice; move it to a helper?
>
> Line 531: shouldn't the sock.SetOption() be after the if sock == nil?
>
> Line 554: why is it subscribing to every possible message by default? This
> could be a bad idea...
>
> --
> Gonzalo Diethelm
> gonzalo.diethelm@xxxxxxxxx
>



-- 
Gonzalo Diethelm
gonzalo.diethelm@xxxxxxxxx

Other related posts: