[dev] golang dwm status

[Markus Teich]

Heyho,

the recent discussion about Go motivated me to finally rewrite my dwm status
shell script. Since this is one of my first programms written in Go, I would
love to get some feedback from you Go gurus out there. The weird characters in
the output are used for coloring and as icons/separators.

You can find the code on github[0].

Regards,
Markus

[0] https://github.com/schachmat/gods/blob/master/gods.go

[Alexander Huemer]

On Thu, Mar 13, 2014 at 08:34:04PM +0100, Markus Teich wrote:
> the recent discussion about Go motivated me to finally rewrite my dwm
> status shell script. Since this is one of my first programms written
> in Go, I would love to get some feedback from you Go gurus out there.
> The weird characters in the output are used for coloring and as
> icons/separators.

I don't have much clue about go, but I think that the content of
/proc/cpuinfo differ greatly on different architecture. Maybe the output
ls `lscpu` is better suited for that purpose.

Kind regards,
-Alex

[Markus Teich]

Alexander Huemer wrote:
> I don't have much clue about go, but I think that the content of /proc/cpuinfo
> differ greatly on different architecture. Maybe the output ls `lscpu` is
> better suited for that purpose.

Thanks for the hint. However I wanted to avoid spawning other processes as much
as possible. Is there another way to count the cpu cores just by reading a file
in /proc or maybe /sys?

--Markus

[Alexander Huemer]

Not that I know of. As it seems the linux kernel does not provide those
pieces of information in an architecture independent format. I vaguely
remember a thread somewhere that discussed this issue, but I have no
idea where that was. In the current situation lscpu is the best you'll
get.
Do you care about other operating systems then linux? E.g. the BSDs do
not have /proc/cpuinfo at all.

Kind regards,
-Alex

[Silvan Jegen]

On Thu, Mar 13, 2014 at 08:34:04PM +0100, Markus Teich wrote:

> Heyho,
>
> the recent discussion about Go motivated me to finally rewrite my
> dwm status shell script. Since this is one of my first programms
> written in Go, I would love to get some feedback from you Go gurus out
> there. The weird characters in the output are used for coloring and
> as icons/separators.

I am not a guru but a few simple things that stood out to me are:

1. The usual way to import several packages is

import (
"fmt"
"whatever"
"etc"
)

2. The same for vars, i. e.:

var (
cores = 1
rxOld = 0
...
)

3. Instead of appending to the same slice several times just use a
slice-literal like this:

http://play.golang.org/p/U8r3Z_crOK


Cheers,

Silvan

[Charlie Andrews]

On Thu, Mar 13, 2014 at 09:00:58PM +0100, Silvan Jegen wrote:
> On Thu, Mar 13, 2014 at 08:34:04PM +0100, Markus Teich wrote:
> > Heyho,
> >
> > the recent discussion about Go motivated me to finally rewrite my
> > dwm status shell script. Since this is one of my first programms
> > written in Go, I would love to get some feedback from you Go gurus out
> > there. The weird characters in the output are used for coloring and
> > as icons/separators.
>
> I am not a guru but a few simple things that stood out to me are:
>

I write go for a living, and these 3 comments were the only ones off the
top of my head as well.

> 1. The usual way to import several packages is
>
> import (
> "fmt"
> "whatever"
> "etc"
> )
>
> 2. The same for vars, i. e.:
>
> var (
> cores = 1
> rxOld = 0
> ...
> )
>
> 3. Instead of appending to the same slice several times just use a
> slice-literal like this:
>
> http://play.golang.org/p/U8r3Z_crOK

this also decreases the amount of allocations the runtime does.

>
>
> Cheers,
>
> Silvan
>

Other than that, looks great!

-Charlie

[Markus Teich]

Silvan Jegen wrote:
> 1. The usual way to import several packages is
>
> import (
> "fmt"
> "whatever"
> "etc"
> )

I know that it is possible, but what benefits do I get? I decided against it,
because on small laptop screens I get 2 loc more on the screen and the few bytes
necessary for the multiple „import“ statements should not be an issue with
todays hard disks.

> 2. The same for vars, i. e.:
>
> var (
> cores = 1
> rxOld = 0
> ...
> )

Same argument applies here. This could be used to visually group a subset of
variables which belong together in some way, but I did not find such a case in
my code.

> 3. Instead of appending to the same slice several times just use a
> slice-literal like this:
>
> http://play.golang.org/p/U8r3Z_crOK

I actually had it this way before, but changed it, because I found it very
confusing and unclear to read. Also I don't think it has a performance impact
since the slice-literal gets filled with dynamic-length return values each time.

Nevertheless, thanks for the feedback, Silvan.

Regards,
Markus

[Markus Teich]

Charlie Andrews wrote:
> I write go for a living, and these 3 comments were the only ones off the
> top of my head as well.

If you like, please explain the benefits of the first 2 points a bit more
detailed (see my other mail for my reasoning)

> > 3. Instead of appending to the same slice several times just use a
> > slice-literal like this:
> >
> > http://play.golang.org/p/U8r3Z_crOK
>
> this also decreases the amount of allocations the runtime does.

Ok, I will try to find a readable way for the slice-literal then. Thanks.

Regards,
Markus

[Paul Onyschuk]

On Thu, 13 Mar 2014 20:50:38 +0100
Markus Teich <markus...@stusta.mhn.de> wrote:

> Thanks for the hint. However I wanted to avoid spawning other
> processes as much as possible. Is there another way to count the cpu
> cores just by reading a file in /proc or maybe /sys?

You can get there by using sysconf(3), from man page:

"The sysconf() function conforms to IEEE Std 1003.1-1988 (``POSIX.1'').
The constants _SC_NPROCESSORS_CONF and _SC_NPROCESSORS_ONLN are not
part of the standard, but are provided by many systems."

Calling "sysconf(_SC_NPROCESSORS_CONF)" should work on many systems.
I'm clueless, when it comes to accessing C from Go, so this can be
wrong solution in your case.


--
Paul Onyschuk

[Charlie Andrews]

On Thu, Mar 13, 2014 at 09:21:09PM +0100, Markus Teich wrote:
>
> If you like, please explain the benefits of the first 2 points a bit more
> detailed (see my other mail for my reasoning)

In both cases, I believe it's much more readable and maintainable to
have a section for imports and a section for vars (global-ish variabls).
Readability is subjective I guess, but maintainability is not, and it is
much easier this way to add imports and vars with fewer key strokes
later. If I wanted to log a random part all I would have to do is pop in
"log" into the list and go on my merry way. This is a very small
workflow optimization, but who knows what a few seconds saved here could
do.

> Ok, I will try to find a readable way for the slice-literal then. Thanks.

I find this very readable:

newSlice := []string{
"string1",
"string2",
"string3", // yes you need the comma here
}

but again, readability is subjective, so it's up to you.

-Charlie

[Markus Teich]

Charlie Andrews wrote:
> In both cases, I believe it's much more readable and maintainable to
> have a section for imports and a section for vars (global-ish variabls).
> Readability is subjective I guess, but maintainability is not, and it is
> much easier this way to add imports and vars with fewer key strokes
> later. If I wanted to log a random part all I would have to do is pop in
> "log" into the list and go on my merry way. This is a very small
> workflow optimization, but who knows what a few seconds saved here could
> do.

In this small programm it probably will not matter, but I fixed it to get used
to it for bigger projects. ;)

> I find this very readable:
>
> newSlice := []string{
> "string1",
> "string2",
> "string3", // yes you need the comma here
> }

This is also way better than the solution[0] I came up with. Thanks!

--Markus

[0] https://github.com/schachmat/gods/blob/b48e7442315156209edfee3b77d3f5e9dfe06fe3/gods.go#L145

[Silvan Jegen]

On Thu, Mar 13, 2014 at 04:33:16PM -0400, Charlie Andrews wrote:
> On Thu, Mar 13, 2014 at 09:21:09PM +0100, Markus Teich wrote:
> >
> > If you like, please explain the benefits of the first 2 points a bit more
> > detailed (see my other mail for my reasoning)
>
> In both cases, I believe it's much more readable and maintainable to
> have a section for imports and a section for vars (global-ish variabls).
> Readability is subjective I guess, but maintainability is not, and it is
> much easier this way to add imports and vars with fewer key strokes
> later. If I wanted to log a random part all I would have to do is pop in
> "log" into the list and go on my merry way. This is a very small
> workflow optimization, but who knows what a few seconds saved here could
> do.

I agree.

> > Ok, I will try to find a readable way for the slice-literal then. Thanks.
>
> I find this very readable:
>
> newSlice := []string{
> "string1",
> "string2",
> "string3", // yes you need the comma here
> }
>
> but again, readability is subjective, so it's up to you.

I see it the same way. These are probably personal preferences for the
most part. While we are talking about personal preferences...

When scanning the code I saw several cases where you declared and
initialized variables on the same line like this

var void = 0 // target for unused values
var dev, rx, tx, rxNow, txNow = "", 0, 0, 0, 0
var scanner = bufio.NewScanner(file)

if you do not want to bother with type declarations I would just eliminate
'var' completely by using the ':=' operator.

void := 0 // target for unused values
dev, rx, tx, rxNow, txNow := "", 0, 0, 0, 0
...

In any case, your code looks like idiomatic Go to me.

[Markus Teich]

Paul Onyschuk wrote:
> You can get there by using sysconf(3), from man page:

Oh man, I just found this:
http://golang.org/pkg/runtime/#NumCPU
-.-

--Markus

[Sebastián Ferrara]

2014-03-13 16:50 GMT-03:00 Markus Teich <markus...@stusta.mhn.de>:
>
> Thanks for the hint. However I wanted to avoid spawning other processes as much
> as possible. Is there another way to count the cpu cores just by reading a file
> in /proc or maybe /sys?

See http://golang.org/pkg/runtime/#NumCPU

func NumCPU() int

NumCPU returns the number of logical CPUs on the local machine.

Cheers

[Markus Teich]

Silvan Jegen wrote:
> When scanning the code I saw several cases where you declared and
> initialized variables on the same line like this
>
> var void = 0 // target for unused values
> var dev, rx, tx, rxNow, txNow = "", 0, 0, 0, 0
> var scanner = bufio.NewScanner(file)
>
> if you do not want to bother with type declarations I would just eliminate
> 'var' completely by using the ':=' operator.
>
> void := 0 // target for unused values
> dev, rx, tx, rxNow, txNow := "", 0, 0, 0, 0
> ...

This is also intended. I wanted all variables (apart from the short lived ones
declared in for and if headers) to be easily findable with grep var. Anything
wrong with that?
I struggled a bit between the different kinds of variable declarations and tried
to keep it as consistent as possible with this rule:
In control structure headers, ":=" is allowed, everywhere else use "var".

> In any case, your code looks like idiomatic Go to me.

Thanks, much appreciated.

Regards,
Markus

[Silvan Jegen]

On Thu, Mar 13, 2014 at 10:13:25PM +0100, Markus Teich wrote:
> Silvan Jegen wrote:
> > When scanning the code I saw several cases where you declared and
> > initialized variables on the same line like this
> >
> > var void = 0 // target for unused values
> > var dev, rx, tx, rxNow, txNow = "", 0, 0, 0, 0
> > var scanner = bufio.NewScanner(file)
> >
> > if you do not want to bother with type declarations I would just eliminate
> > 'var' completely by using the ':=' operator.
> >
> > void := 0 // target for unused values
> > dev, rx, tx, rxNow, txNow := "", 0, 0, 0, 0
> > ...
>
> This is also intended. I wanted all variables (apart from the short lived ones
> declared in for and if headers) to be easily findable with grep var. Anything
> wrong with that?

Not that I am aware of.

> I struggled a bit between the different kinds of variable declarations and tried
> to keep it as consistent as possible with this rule:
> In control structure headers, ":=" is allowed, everywhere else use "var".

I think that the var keyword used without a type declaration is redundant
(so the following may be my rule if you will).

If you are not going to write something like

var scanner bufio.Scanner
scanner = bufio.NewScanner(file)

which lets you easily see what the type of 'scanner' is you can just
write

scanner := bufio.NewScanner(file)

and get rid of the 'var'.

As was said before, these things are highly subjective though.