Skip to content
This repository was archived by the owner on Jun 23, 2025. It is now read-only.

Conversation

@sidharthchadha2
Copy link
Collaborator

main.go - excecise 1 ( http )
main_1.go - excercise 2 ( http )
palindrome.test - excercise 1 ( testing )


var userResponse UserResponse
err = json.Unmarshal(body, &userResponse)
if err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comes down to personal preferences, so other people may have different suggestions on this. But a more idiomatic way of retrieving an error and checking if it's nil, is like this:

if err = json.Unmarshal(body, &userResponse); err != nil {
	return User{}, err
}

The main benefit of this approach is that the scope of the err variable is limited to the if statement. This style is used whenever the function that you want to check (in this case, json.Unmarshal()) only returns an error object, and you want to immediately check the error.


Note: your solution is also perfectly valid, since the err variable has already been declared in a previous statement. The code that I recommended does not actually limit the scope of the outer err variable, but it's still considered more "go idiomatic", aka it's more common.

return nil, err
}

accounts := make([]Account, len(accountResponses))
Copy link
Collaborator

@mavrikis-kostas mavrikis-kostas Apr 22, 2025

Choose a reason for hiding this comment

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

Great use of the make function here, this will create a new slice that has preconfigured capacity 👍


accounts := make([]Account, len(accountResponses))
for i, accountResponse := range accountResponses {
accounts[i] = accountResponse.Attributes
Copy link
Collaborator

@mavrikis-kostas mavrikis-kostas Apr 22, 2025

Choose a reason for hiding this comment

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

Nitpick; your solution works great, but there is a safer solution that I would recommend.

In this file, you have already created a slice accounts with preconfigured capacity, and inside this loop, you are assigning a value to each element:

accounts := make([]Account, len(accountResponses))
for i, accountResponse := range accountResponses {
	accounts[i] = accountResponse.Attributes
}

This will work great if you are 100% sure that the capacity is correct, and the slice does not need to be resized. However, the code will cause panics if the capacity is not big enough.


A more common approach, is to use append for each new element. If the capacity is correct, then append will have exactly the same behavior as your code. If the capacity is not enough, then append will resize the slice, preventing panics.

accounts := make([]Account, len(accountResponses))
for i, accountResponse := range accountResponses {
	accounts = append(accounts, accountResponse.Attributes)
}

There are some online articles that compare the performance between assignments accounts[i] = ... and appends accounts = append(accounts, ...), and they show that the performance is almost the same. For this exercise, we know that the capacity is definitely correct, so both approaches are 100% acceptable. So it is up to personal preference.

url := fmt.Sprintf("https://sample-accounts-api.herokuapp.com/users/%d", userID)
resp, err := http.Get(url)
if err != nil {
return User{}, err
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function has great error handling! But it is generally recommended to use error wrapping (nested errors) when you propagate errors to parent functions. This way, you can immediately identify where an error comes from, just by looking at the error message. Error wrapping is provided by the errors package, you can check the official documentation for more information.

In this case, instead of returning the err object, you can use fmt.Errorf and the verb %w to create a new error:

returnUser{}, fmt.Errorf("fetching user %v: %w", userID, err)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ohhh i see , the error stack will have 2 similar logs this way..gotcha


func TestReverse(t *testing.T) {
testCases := []struct {
name string
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file has a mix of whitespaces and tabs, if you auto-format the file in your IDE, it will be fixed 👌 (Go only uses tab characters for indentation)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants