Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add clean code example #2701

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Here you can find the most **delicious** recipes to cook delicious meals using o
- [Amazon Web Services (AWS) Elastic Beanstalk](/aws-eb)
- [AWS SAM](/aws-sam)
- [Certificates from Let's Encrypt](/autocert)
- [Clean Code](/clean-code/)
- [Clean Architecture](/clean-architecture)
- [Cloud Run](/cloud-run)
- [Colly Scraping using Fiber and PostgreSQL](/fiber-colly-gorm)
Expand Down
1 change: 1 addition & 0 deletions clean-code/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
db_data
8 changes: 8 additions & 0 deletions clean-code/Dockerfile-local
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
FROM golang:1.23
RUN apt update && apt upgrade -y && apt install -y git

WORKDIR /go/src/app
COPY app ./
RUN go mod tidy && go mod verify

ENTRYPOINT [ "go", "run", "." ]
42 changes: 42 additions & 0 deletions clean-code/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
## Clean code example for Fiber and PostgreSQL
Copy link
Member

Choose a reason for hiding this comment

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

pls add a link in the README.md in root folder to this file

Copy link
Author

Choose a reason for hiding this comment

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

Added


This is an example of a RESTful API built using the Fiber framework (https://gofiber.io/) and PostgreSQL as the database.
Copy link
Member

Choose a reason for hiding this comment

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

please describe more about the part of the clean code here

Copy link
Author

Choose a reason for hiding this comment

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

Added


### Description of Clean Code

Clean code is a philosophy and set of practices aimed at writing code that is easy to understand, maintain, and extend. Key principles of clean code include:

- **Readability**: Code should be easy to read and understand.
- **Simplicity**: Avoid unnecessary complexity.
- **Consistency**: Follow consistent coding standards and conventions.
- **Modularity**: Break down code into small, reusable, and independent modules.
- **Testability**: Write code that is easy to test.

This Fiber app is a good example of clean code because:

- **Modular Structure**: The code is organized into distinct modules, making it easy to navigate and understand.
- **Clear Separation of Concerns**: Different parts of the application (e.g., routes, handlers, services) are clearly separated, making the codebase easier to maintain and extend.
- **Error Handling**: Proper error handling is implemented to ensure the application behaves predictably.

### Start

1. Build and start the containers:
```sh
docker compose up --build
```

1. The application should now be running and accessible at `http://localhost:3000`.

### Endpoints

- `GET /api/v1/books`: Retrieves a list of all books.
```sh
curl -X GET http://localhost:3000/api/v1/books
```

- `POST /api/v1/books`: Adds a new book to the collection.
```sh
curl -X POST http://localhost:3000/api/v1/books \
-H "Content-Type: application/json" \
-d '{"title":"Title"}'
```
24 changes: 24 additions & 0 deletions clean-code/app/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package main

import "os"

// Configuration is used to store values from environment variables
type Configuration struct {
Port string
DatabaseURL string
}

// NewConfiguration reads environment variables and returns a new Configuration
func NewConfiguration() *Configuration {
Copy link
Member

Choose a reason for hiding this comment

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

image
https://github.com/Pungyeon/clean-go-article#Comments

can you add comments to all public vars/functions/structs

Copy link
Author

Choose a reason for hiding this comment

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

I think it most useful for libraries but added here also where it could be useful

return &Configuration{
Port: getEnvOrDefault("PORT", "3000"),
DatabaseURL: getEnvOrDefault("DATABASE_URL", ""),
}
}
norri marked this conversation as resolved.
Show resolved Hide resolved

func getEnvOrDefault(key, defaultValue string) string {
if value, exists := os.LookupEnv(key); exists {
return value
}
return defaultValue
}
41 changes: 41 additions & 0 deletions clean-code/app/config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package main

import (
"os"
"testing"

"github.com/stretchr/testify/assert"
)
Comment on lines +1 to +8
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using a more appropriate package name.

In a clean architecture setup, using package main for test files might not be the best practice. Consider using a more descriptive package name that reflects the module being tested (e.g., config, configuration).

-package main
+package config
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
package main
import (
"os"
"testing"
"github.com/stretchr/testify/assert"
)
package config
import (
"os"
"testing"
"github.com/stretchr/testify/assert"
)


func TestNewConfiguration(t *testing.T) {
os.Setenv("PORT", "8080")
os.Setenv("DATABASE_URL", "postgres://user:pass@localhost:5432/dbname")
defer os.Unsetenv("PORT")
defer os.Unsetenv("DATABASE_URL")

conf := NewConfiguration()

assert.Equal(t, "8080", conf.Port)
assert.Equal(t, "postgres://user:pass@localhost:5432/dbname", conf.DatabaseURL)
}
norri marked this conversation as resolved.
Show resolved Hide resolved

func TestNewConfiguration_Defaults(t *testing.T) {
os.Unsetenv("PORT")
os.Unsetenv("DATABASE_URL")

conf := NewConfiguration()

assert.Equal(t, "3000", conf.Port)
assert.Equal(t, "", conf.DatabaseURL)
}

func TestGetEnvOrDefault(t *testing.T) {
os.Setenv("TEST_ENV", "value")
defer os.Unsetenv("TEST_ENV")

value := getEnvOrDefault("TEST_ENV", "default")
assert.Equal(t, "value", value)

value = getEnvOrDefault("NON_EXISTENT_ENV", "default")
assert.Equal(t, "default", value)
}
9 changes: 9 additions & 0 deletions clean-code/app/datasources/data_sources.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package datasources

import "app/datasources/database"

// DataSources is a struct that contains all the data sources
// It is used to pass different data sources to the server and services
type DataSources struct {
DB database.Database
}
44 changes: 44 additions & 0 deletions clean-code/app/datasources/database/db.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package database

import (
"context"
"fmt"
"log"
"strings"
)

// Book represents a book in the database
type Book struct {
ID int
Title string
}

// NewBook represents a new book to be created to the database
type NewBook struct {
Title string
}
norri marked this conversation as resolved.
Show resolved Hide resolved

// Database is an interface for interacting with the database
// With using this the implementation can be changed without affecting the rest of the code.
type Database interface {
LoadAllBooks(ctx context.Context) ([]Book, error)
CreateBook(ctx context.Context, newBook NewBook) error
CloseConnections()
}

// NewDatabase creates a new Database instance
func NewDatabase(ctx context.Context, databaseURL string) (Database, error) {
if databaseURL == "" {
log.Printf("Using in-memory database")
return newMemoryDB(), nil
} else if strings.HasPrefix(databaseURL, "postgres://") {
db, err := newPostgresDB(ctx, databaseURL)
if err != nil {
return nil, fmt.Errorf("failed to create postgres database: %w", err)
}
log.Printf("Using Postgres database")
return db, nil
}
return nil, fmt.Errorf("unsupported database: %s", databaseURL)

norri marked this conversation as resolved.
Show resolved Hide resolved
}
27 changes: 27 additions & 0 deletions clean-code/app/datasources/database/db_mock.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package database

import (
"context"

"github.com/stretchr/testify/mock"
)

type DatabaseMock struct {
mock.Mock
}

func (m *DatabaseMock) LoadAllBooks(ctx context.Context) ([]Book, error) {
args := m.Called(ctx)
if args.Get(0) == nil {
return nil, args.Error(1)
}
return args.Get(0).([]Book), args.Error(1)
}
norri marked this conversation as resolved.
Show resolved Hide resolved

func (m *DatabaseMock) CreateBook(ctx context.Context, newBook NewBook) error {
args := m.Called(ctx, newBook)
return args.Error(0)
}

func (m *DatabaseMock) CloseConnections() {
}
34 changes: 34 additions & 0 deletions clean-code/app/datasources/database/db_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package database

import (
"context"
"reflect"
"testing"

"github.com/stretchr/testify/assert"
)

func TestNewDatabase_MemoryDB(t *testing.T) {
ctx := context.Background()
db, err := NewDatabase(ctx, "")
assert.Nil(t, err)
assert.Equal(t, "*database.memoryDB", reflect.TypeOf(db).String())
}

func TestNewDatabase_PostgresDB(t *testing.T) {
ctx := context.Background()
db, err := NewDatabase(ctx, "postgres://localhost:5432")
assert.Nil(t, err)
assert.Equal(t, "*database.postgresDB", reflect.TypeOf(db).String())
}

func TestNewDatabase_InvalidDatabaseConfiguration(t *testing.T) {
ctx := context.Background()
_, err := NewDatabase(ctx, "invalid")
assert.ErrorContains(t, err, "unsupported database")
}

func assertBook(t *testing.T, book Book, expectedID int, expected NewBook) {
assert.Equal(t, expectedID, book.ID)
assert.Equal(t, expected.Title, book.Title)
}
32 changes: 32 additions & 0 deletions clean-code/app/datasources/database/memory_db.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package database

import "context"

// This is just an example and not for production use
func newMemoryDB() Database {
return &memoryDB{
records: make([]Book, 0, 10),
idCounter: 0,
}
}

type memoryDB struct {
records []Book
idCounter int
}
norri marked this conversation as resolved.
Show resolved Hide resolved

func (db *memoryDB) LoadAllBooks(_ context.Context) ([]Book, error) {
return db.records, nil
}
norri marked this conversation as resolved.
Show resolved Hide resolved

func (db *memoryDB) CreateBook(_ context.Context, newBook NewBook) error {
db.records = append(db.records, Book{
ID: db.idCounter,
Title: newBook.Title,
})
db.idCounter++
return nil
}

func (db *memoryDB) CloseConnections() {
}
44 changes: 44 additions & 0 deletions clean-code/app/datasources/database/memory_db_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package database

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
)

func TestMemoryDB_LoadBooks(t *testing.T) {
db := newMemoryDB()
books, err := db.LoadAllBooks(context.Background())
assert.Nil(t, err)
assert.Equal(t, 0, len(books))
}

func TestMemoryDB_SaveBook(t *testing.T) {
db := newMemoryDB()
newBook := NewBook{Title: "Title"}
err := db.CreateBook(context.Background(), newBook)
assert.Nil(t, err)

books, err := db.LoadAllBooks(context.Background())
assert.Nil(t, err)
assert.Equal(t, 1, len(books))
assertBook(t, books[0], 0, newBook)
}

func TestMemoryDB_SaveBookMultiple(t *testing.T) {
db := newMemoryDB()
newBook1 := NewBook{Title: "Title1"}
err := db.CreateBook(context.Background(), newBook1)
assert.Nil(t, err)

newBook2 := NewBook{Title: "Title2"}
err = db.CreateBook(context.Background(), newBook2)
assert.Nil(t, err)

books, err := db.LoadAllBooks(context.Background())
assert.Nil(t, err)
assert.Equal(t, 2, len(books))
assertBook(t, books[0], 0, newBook1)
assertBook(t, books[1], 1, newBook2)
}
62 changes: 62 additions & 0 deletions clean-code/app/datasources/database/postgres_db.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package database

import (
"context"
"fmt"

"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgconn"
"github.com/jackc/pgx/v5/pgxpool"
)

// PostgresPool is an interface for interacting with the database connection pool.
// Needed for mocking the database connection pool in tests.
type PostgresPool interface {
Query(ctx context.Context, sql string, args ...interface{}) (pgx.Rows, error)
Exec(ctx context.Context, sql string, args ...interface{}) (pgconn.CommandTag, error)
Close()
}

func newPostgresDB(ctx context.Context, databaseURL string) (Database, error) {
// For production use set connection pool settings and validate connection with ping
dbpool, err := pgxpool.New(ctx, databaseURL)
if err != nil {
return nil, fmt.Errorf("unable to create connection pool: %v", err)
}
return &postgresDB{
pool: dbpool,
}, nil
}

type postgresDB struct {
pool PostgresPool
}

// LoadAllBooks loads all books from the database
func (db *postgresDB) LoadAllBooks(ctx context.Context) ([]Book, error) {
rows, err := db.pool.Query(ctx, "SELECT id, title FROM books")
if err != nil {
return nil, fmt.Errorf("failed to query books table: %w", err)
}
defer rows.Close()

books, err := pgx.CollectRows(rows, pgx.RowToStructByName[Book])
if err != nil {
return nil, fmt.Errorf("failed to collect rows: %w", err)
}
return books, nil
}

// CreateBook creates a new book in the database
func (db *postgresDB) CreateBook(ctx context.Context, newBook NewBook) error {
_, err := db.pool.Exec(ctx, "INSERT INTO books (title) VALUES ($1)", newBook.Title)
if err != nil {
return fmt.Errorf("failed to insert book: %w", err)
}
return nil
}

// CloseConnections closes the database connection pool
func (db *postgresDB) CloseConnections() {
db.pool.Close()
}
Comment on lines +60 to +62
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error handling in CloseConnections

The Close method should return an error to allow proper cleanup handling.

-func (db *postgresDB) CloseConnections() {
+func (db *postgresDB) CloseConnections() error {
-    db.pool.Close()
+    return db.pool.Close()
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (db *postgresDB) CloseConnections() {
db.pool.Close()
}
func (db *postgresDB) CloseConnections() error {
return db.pool.Close()
}

Loading