RFC - Review request for a project done in Golang

[Sankar]

Hi

I was recently asked in an interview to write a golang program for a problem that involves working with a million nodes. I did write a program that solved the problem statement. However, I was told that the solution was "poorly structured", but I did not get any detailed review comments though. 

So, I recreated the solution in github and wanted to know if anyone could give some review comments as to what you see as bad things in the code.

The problem statement, code and the instructions are at: https://github.com/psankar/network-monitor 

I personally felt that the code (written in about 6 hours for the interview) is good and I would've hired anyone writing this, but may be I am biased because it is written by me. I want to improve my Golang skills and your review comments would be helpful. Any help ?

If the golang list is unsuitable for this, you can even email me, individually, with the review comments.

Thanks.

Sankar

[Tamás Gulácsi]

Minion/minion? For what?

Also APIServer.go has some goroutine starting loops that are very similar - should be a function.
I'd prefer splitting the HTTP handlers to parser + executor + responder (a'la go-kit), clarifying the business logic.

Just my 2¢.

[Sankar P]

2018-05-14 17:28 GMT+05:30 Tamás Gulácsi <tgula...@gmail.com>:

Minion/minion? For what?

So that the minion package structs can be shared between Minion/Minion.go server and the APIServer.go server.

 

Also APIServer.go has some goroutine starting loops that are very similar - should be a function.
I'd prefer splitting the HTTP handlers to parser + executor + responder (a'la go-kit), clarifying the business logic.

I moved the actual network call to the contactMinion function, but as you said it is probably a good idea to split the handler. Thanks.

 

Just my 2¢.

2018. május 14., hétfő 13:35:23 UTC+2 időpontban Sankar a következőt írta:

Hi

I was recently asked in an interview to write a golang program for a problem that involves working with a million nodes. I did write a program that solved the problem statement. However, I was told that the solution was "poorly structured", but I did not get any detailed review comments though. 

So, I recreated the solution in github and wanted to know if anyone could give some review comments as to what you see as bad things in the code.

The problem statement, code and the instructions are at: https://github.com/psankar/network-monitor 

I personally felt that the code (written in about 6 hours for the interview) is good and I would've hired anyone writing this, but may be I am biased because it is written by me. I want to improve my Golang skills and your review comments would be helpful. Any help ?

If the golang list is unsuitable for this, you can even email me, individually, with the review comments.

Thanks.

Sankar

--
You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/SpoC7siQrS8/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Sankar P
http://psankar.blogspot.com

[Chris Hopkins]

There's an old Dilbert about this:

Don't know how to solve a problem? No problem! Don't hire a consultant; hold an interview and choose the best answer you receive from all the candidates.

I'll get my coat.

Chris

[matthe...@gmail.com]

They might have been looking for something like this:

github.com/psankar/network-monitor

    package monitor code files

    cmd/

        minion/

            package main code files

        server/

            package main code files

In a code review I would mention the use of packages as not being ideal, but that’s just my opinion. You seem able to write Go code, but there aren’t tests in this solution. How did you verify it? Maybe you passed but there was a better candidate?

Matt

[Sankar P]

2018-05-14 20:39 GMT+05:30 <matthe...@gmail.com>:

They might have been looking for something like this:

github.com/psankar/network-monitor

    package monitor code files

    cmd/

        minion/

            package main code files

        server/

            package main code files

The reviewer mentioned that the code was not easy to work with and unstructured. So I am almost sure that it has nothing to do with the package structure. Your mail however gave me an idea about using gocyclo and it found a score of 23 for the code, which is bordering on bad code. May be I could restructure into different functions a bit. Thanks.

 

In a code review I would mention the use of packages as not being ideal, but that’s just my opinion. You seem able to write Go code, but there aren’t tests in this solution. How did you verify it?

I had some tests but I did not include them in the github repo. They were not very exhaustive, but just covered the basic cases, through a script that will add/remove files and contents.

 

Maybe you passed but there was a better candidate?

Could be. 

 

Matt

On Monday, May 14, 2018 at 6:35:23 AM UTC-5, Sankar wrote:

Hi

I was recently asked in an interview to write a golang program for a problem that involves working with a million nodes. I did write a program that solved the problem statement. However, I was told that the solution was "poorly structured", but I did not get any detailed review comments though. 

So, I recreated the solution in github and wanted to know if anyone could give some review comments as to what you see as bad things in the code.

The problem statement, code and the instructions are at: https://github.com/psankar/network-monitor 

I personally felt that the code (written in about 6 hours for the interview) is good and I would've hired anyone writing this, but may be I am biased because it is written by me. I want to improve my Golang skills and your review comments would be helpful. Any help ?

If the golang list is unsuitable for this, you can even email me, individually, with the review comments.

Thanks.

Sankar

--

You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/SpoC7siQrS8/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Sankar P]

In case someone is curios, I did refactor the code a lot more, to bring down the gocyclo complexity from 23 to 11. I feel that this should be in a better shape now. Thanks everyone for the comments. Let me know if you still find any scope for optimisation or cleanup.

I would add some test cases too after I feel satisfied with the code.