Skip to content

Conversation

@quenbyako
Copy link

panic is awesome for testing, or benchmarking, but not for real logic. it would be so great, if locjson.Load() method returns an error!

@quenbyako
Copy link
Author

also, may be it would be a cool idea to processing these errors, bu i don't know how yet (=


// Load takes the JSON file name and returns string resources
// in a format compatible with loc.Resources map values.
// It will panic if there's a problem with loading/unmarshaling JSON.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment needs to be changed accordingly if the code no longer panics

@iafan
Copy link
Collaborator

iafan commented Apr 12, 2019

The idea behind using panic was that this kind of error would realistically happen only if the localization file is an invalid JSON file, in which case it's probably best to let the developer known ASAP. I.e. it's more of a development error rather than a runtime error.

Having said that, there's a small chance that loading resource would fail due to a runtime (e.g. I/O) error, and having the method return an error gives flexibility on how to handle this.

See these lines that need to be adjusted as a part of your PR:
https://github.com/iafan/go-l10n/blob/master/examples/desktop/main.go#L15-L17

I recommend creating a wrapper loadJSON() method in this example that will load the localized resource and panic on error (which is fine for the purposes of an example).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants