GWI challenge by stevenoddity · Pull Request #47 · GlobalWebIndex/platform-go-challenge

stevenoddity · 2025-10-07T10:06:47Z

No description provided.

dzacharakis

Thanks for your work on this assignment!

I added some comments on your PR with a few questions. I’d love to hear your feedback when you get a chance.

Thanks,
Dimitris

dzacharakis · 2025-10-22T08:26:18Z

+		defer func() {
+			if err := recover(); err != nil {


Is recovering from panics in Go generally considered a good practice?

In general no, as it "hides" the issue.

dzacharakis · 2025-10-22T09:18:35Z

+
+// GetUsers handles the HTTP request to retrieve a list of users.
+func GetUsers(w http.ResponseWriter, r *http.Request) {
+	// This function currently does not implement any logic.


Any reason you’re keeping these?

These were just placeholders for enhancements that I didn't implement.

dzacharakis · 2025-10-22T09:29:46Z

+func main() {
+	router := routes.RegisterRoutes()
+	fmt.Println("🚀 Server running on http://localhost:8080")
+	log.Fatal(http.ListenAndServe(":8080", middleware.RecoverMiddleware(router)))


Why did you choose log.Fatal here? Would log.Error have been more appropriate?

if it fails, the program cannot recover, so that's why I chose it.

dzacharakis · 2025-10-22T09:33:47Z

+
+// ValidateFavorite validates the given Favorite struct.
+// It returns an error if the validation fails, otherwise it returns nil.
+func ValidateFavorite(fav *favorite_model.Favorite) error {


Is this method being used anywhere?

Nope, I didn't utilize it at the end.

dzacharakis · 2025-10-22T10:14:13Z

+type Asset struct {
+	ID          int             `json:"id"`          // Unique identifier for the asset
+	UserID      int             `json:"user_id"`     // Identifier for the user who owns the asset
+	Description json.RawMessage `json:"description"` // Description of the asset


Using json.RawMessage for the Description field is a poor design choice. Why? Any better approaches?

It hides the structure of description and it makes quering difficult with slow performance. A better approach would be to define a separate structure from the description.

dzacharakis · 2025-10-22T10:36:59Z

@@ -0,0 +1,11 @@
+package favorite_model
+
+import asset_model "gwi/models/asset"


Is an alternative name needed for this import declaration?

I found out that import "gwi/models/asset" works as well.

dzacharakis · 2025-10-22T10:51:54Z

+	asset_model "gwi/models/asset"
+	favorite_model "gwi/models/favorite"


Don't you find your package structure over-granular? Could you point out a main principle it’s meant to support and how this granularity impacts it?

My main goal is the separation of concerns: each package should be responsible of one thing and as much decoupled as possible.
This granularity adds too much noise and it's an overkill for such a small app, but I wanted to demonstrate a more generic approach.

dzacharakis · 2025-10-22T10:57:35Z

@@ -0,0 +1,10 @@
+package asset_model


What would you change if this had to grew to, let's say, 20 models?

I would add a base struct.

stevenoddity

Hello Dimitris,
Thanks a lot for your time!
I answered your questions and looking forward for a follow-up.

stevenoddity · 2025-10-22T20:13:10Z

+func main() {
+	router := routes.RegisterRoutes()
+	fmt.Println("🚀 Server running on http://localhost:8080")
+	log.Fatal(http.ListenAndServe(":8080", middleware.RecoverMiddleware(router)))


if it fails, the program cannot recover, so that's why I chose it.

stevenoddity · 2025-10-22T20:15:42Z

+		defer func() {
+			if err := recover(); err != nil {


In general no, as it "hides" the issue.

stevenoddity · 2025-10-22T20:23:34Z

@@ -0,0 +1,10 @@
+package asset_model


I would add a base struct.

stevenoddity · 2025-10-22T20:33:59Z

+type Asset struct {
+	ID          int             `json:"id"`          // Unique identifier for the asset
+	UserID      int             `json:"user_id"`     // Identifier for the user who owns the asset
+	Description json.RawMessage `json:"description"` // Description of the asset


It hides the structure of description and it makes quering difficult with slow performance. A better approach would be to define a separate structure from the description.

stevenoddity · 2025-10-22T20:35:24Z

@@ -0,0 +1,11 @@
+package favorite_model
+
+import asset_model "gwi/models/asset"


I found out that import "gwi/models/asset" works as well.

stevenoddity · 2025-10-22T20:51:47Z

+	asset_model "gwi/models/asset"
+	favorite_model "gwi/models/favorite"


My main goal is the separation of concerns: each package should be responsible of one thing and as much decoupled as possible.
This granularity adds too much noise and it's an overkill for such a small app, but I wanted to demonstrate a more generic approach.

stevenoddity · 2025-10-22T20:52:19Z

+
+// GetUsers handles the HTTP request to retrieve a list of users.
+func GetUsers(w http.ResponseWriter, r *http.Request) {
+	// This function currently does not implement any logic.


These were just placeholders for enhancements that I didn't implement.

stevenoddity · 2025-10-22T20:52:57Z

+
+// ValidateFavorite validates the given Favorite struct.
+// It returns an error if the validation fails, otherwise it returns nil.
+func ValidateFavorite(fav *favorite_model.Favorite) error {


Nope, I didn't utilize it at the end.

Stefanos Samonas and others added 30 commits October 4, 2025 18:06

Initial structure.

cb74af2

Go packages refactor.

2ac5bbe

Enhance routing.

589081c

Error handling.

3f4c928

Use mock data as a database.

4338af1

Add go.sum

1414ce1

Initiliaze Dockerfile.

8eb7afb

Change on jwt secret.

4c58a6d

Initial input validator.

9567ab0

Integrate error handler and input validator.

f25c3d6

Add basic doc strings.

d463721

Update go version.

adbf43f

Initialize README.

5626742

Feature: list favorites.

f38a061

Feature: edit asset.

c2c5d50

Update README.

e92ba75

Move configuration.

31717d2

Enhance APIs with authorization.

66c8b56

README update.

ca1a174

README enhancement.

492f3f0

README enhancement.

b9d8489

Refactor.

ab7690c

Init tests.

3ea91e6

Enhance testing for favorites.

123f33e

Remove redundant code.

d3377a8

Integration test for favorite.

dde88e2

Unit testing for asset.

4c3d687

Update dependencies.

5154347

Integration test for asset.

66310ed

Tests tuning.

ff602cb

Minor README update.

8807dc7

dzacharakis reviewed Oct 22, 2025

View reviewed changes

stevenoddity commented Oct 22, 2025

View reviewed changes

		@@ -0,0 +1,11 @@
		package favorite_model

		import asset_model "gwi/models/asset"

		asset_model "gwi/models/asset"
		favorite_model "gwi/models/favorite"

Conversation

stevenoddity commented Oct 7, 2025

Uh oh!

dzacharakis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevenoddity left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants