Hacker News new | ask | show | jobs
by dewey 2954 days ago
As this seems to be a beginner project to learn here's some thoughts on the code:

1) You should really not build your json by manually building the string. That's very error prone and hard to extend.

  fmt.Fprintf(f,
  "{\n\t\"from\": {\n\t\t\"name\": \"%s\",

Use https://golang.org/pkg/encoding/json/ (https://gobyexample.com/json) instead. Working with json in Go is really one of the nicer features of Go (It's Go, not "GoLang")

2) By setting / typoing a config option you could accidentally delete files from your computer, even your home directory

  err := os.Remove(jsonfile)
4 comments

A similar comment for the HTML portions of the message. Use html/template. It's got a couple of quirks (mostly around how it sort of weirdly conflates having one particular template with having a set of templates), but it's one of the safest template languages to just pick up and throw some stuff out; it's quite strong against injection attacks.

To assist in using JSON, you'll want to declare a struct holding all your config, at which point it becomes quite surprisingly simple to load it from and save it to a file.

Once you have that functionality, I suggest using it for your initial creation as well. I have a program here where I've been using the pattern that I create a default config sample within the program, and if I can't find the config you specify, I print out an error, and the sample JSON file that constitutes a full, legal config for the user's convenience. I wouldn't ship anything that I'd expect to end up in a Linux distro or something that way, but for an internal tool it seems a decent enough pattern.

Finally, I'd suggest reddit.com/r/golang is a better sort of place for this sort of thing if you want a review, and there's probably even better places, as I'm not convinced that a karma-based voting site is all that great for reviews for beginners. (It's really easy for a post like that to pick up a couple of early downvotes, and consequently lose all visibility to the people who are willing to help.) But if you're going to use one, /r/golang would be better.

Thanks for the suggestion :D

I am not shipping it to any distro or something like that. I was using this for myself and just made it open source.

Thank you for taking your time and reviewing this. As for /r/golang, I will look into it. :)

It's GoLang, because that's easier to search for.
I was just nitpicking, it's not really an important point.

People usually prefer "Go" or "Golang", I only mentioned it because I noticed it in the project description.

I concur. :D
Thank you for taking your time to review my code. Yes, it is a beginner's project :D

1) How do I format the JSON for readability or should I ignore that?

2) Thanks for that. Didn't know about it. Any ideas on how I can fix that? Maybe I can check if it is a file and not a directory and ask for input again for y/n?

You're welcome.

1) What do you mean exactly? The usual way would be to keep your data in a struct like:

  type configuration struct {
    Name   string `json:"name"`
  }
and then marshall it into a json string like: https://play.golang.org/p/uWvwBY4h03n

If you need more details feel free to contact me. Address is in my profile.

No, I meant the readability in .json file. Should I care for that anyway?

Also, great website! :D

https://golang.org/pkg/encoding/json/#MarshalIndent if you want to "pretty print" the json output
That's great! :D Thanks!
You don’t have to worry about that and don’t have to manually format it.
i do agree that it makes sense to use the stdlib where possible.

however i am a bit wary... my first "real" (but small) project in go was to process some xml. turns out the stdlib parser doesn't handle self-closing tags very well, and would simply not pick up some important content from files i was working with.

i would wager the json stuff is probably a much hotter code path so it's probably fine.

Thanks for your suggestion. :)