Skip to content
Merged
Changes from all 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
154 changes: 154 additions & 0 deletions challenge-7/submissions/onomica/solution-template.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
package challenge7

import (
"fmt"
"sync"
)

type BankAccount struct {
ID string
Owner string
Balance float64
MinBalance float64
mu sync.Mutex
}

const (
MaxTransactionAmount = 10000.0
)

type AccountError struct {
ID string
Owner string
}

func (e *AccountError) Error() string {
return fmt.Sprintf("account error: %s: %s", e.ID, e.Owner)
}

type InsufficientFundsError struct {
Balance float64
Amount float64
}

func (e *InsufficientFundsError) Error() string {
return fmt.Sprintf("insufficient funds: balance $%.2f, attempted to withdraw $%.2f", e.Balance, e.Amount)
}

type NegativeAmountError struct {
Amount float64
}

func (e *NegativeAmountError) Error() string {
return fmt.Sprintf("negative amount: $%.2f", e.Amount)
}

type ExceedsLimitError struct {
Amount float64
Limit float64
}

func (e *ExceedsLimitError) Error() string {
return fmt.Sprintf("amount $%.2f exceeds limit $%.2f", e.Amount, e.Limit)
}

func NewBankAccount(id, owner string, initialBalance, minBalance float64) (*BankAccount, error) {
if id == "" || owner == "" {
return nil, &AccountError{
ID: id,
Owner: owner,
}
}

if initialBalance < 0 {
return nil, &NegativeAmountError{Amount: initialBalance}
}
if minBalance < 0 {
return nil, &NegativeAmountError{Amount: minBalance}
}
if initialBalance < minBalance {
return nil, &InsufficientFundsError{
Balance: initialBalance,
Amount: minBalance,
}
}

return &BankAccount{
ID: id,
Owner: owner,
Balance: initialBalance,
MinBalance: minBalance,
}, nil
}

func (a *BankAccount) Deposit(amount float64) error {
if err := validateAmount(amount); err != nil {
return err
}

a.mu.Lock()
defer a.mu.Unlock()
a.Balance += amount
return nil
}

func (a *BankAccount) Withdraw(amount float64) error {
if err := validateAmount(amount); err != nil {
return err
}

a.mu.Lock()
defer a.mu.Unlock()

if a.Balance-amount < a.MinBalance {
return &InsufficientFundsError{
Balance: a.Balance,
Amount: amount,
}
}

a.Balance -= amount
return nil
}

func (a *BankAccount) Transfer(amount float64, target *BankAccount) error {
if a == target {
return nil
}
Comment on lines +115 to +117

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Self-transfer skips amount validation.

The a == target early return at Line 115 happens before validateAmount at Line 119, so Transfer(-100, account) or Transfer(MaxTransactionAmount+1, account) returns nil instead of the expected NegativeAmountError/ExceedsLimitError. Move validation before the self-transfer check to keep the error contract consistent with Deposit and Withdraw.

🔧 Proposed fix
 func (a *BankAccount) Transfer(amount float64, target *BankAccount) error {
+	if err := validateAmount(amount); err != nil {
+		return err
+	}
+
 	if a == target {
 		return nil
 	}
 
-	if err := validateAmount(amount); err != nil {
-		return err
-	}
-
 	first, second := a, target
📝 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
if a == target {
return nil
}
func (a *BankAccount) Transfer(amount float64, target *BankAccount) error {
if err := validateAmount(amount); err != nil {
return err
}
if a == target {
return nil
}
first, second := a, target


if err := validateAmount(amount); err != nil {
return err
}

first, second := a, target
if first.ID > second.ID {
first, second = second, first
}

first.mu.Lock()
defer first.mu.Unlock()
second.mu.Lock()
defer second.mu.Unlock()

if a.Balance-amount < a.MinBalance {
return &InsufficientFundsError{
Balance: a.Balance,
Amount: amount,
}
}

a.Balance -= amount
target.Balance += amount

return nil
}
Comment on lines +114 to +144

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for Transfer-related tests in the challenge-7 test file
rg -n -C3 'TestTransfer' challenge-7/solution-template_test.go

Repository: RezaSi/go-interview-practice

Length of output: 302


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the Transfer test and nearby helpers with line numbers
sed -n '240,360p' challenge-7/solution-template_test.go | cat -n

Repository: RezaSi/go-interview-practice

Length of output: 4295


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for any other transfer-focused tests or self-transfer coverage
rg -n -C2 'self-transfer|Transfer\(|Concurrent.*Transfer|TestTransfer' challenge-7

Repository: RezaSi/go-interview-practice

Length of output: 17520


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the Transfer implementation and the test file around any concurrency helpers
sed -n '1,220p' challenge-7/submissions/onomica/solution-template.go | cat -n

Repository: RezaSi/go-interview-practice

Length of output: 4110


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for any dedicated self-transfer or concurrent transfer test cases
rg -n -C3 'self.?transfer|concurrent transfer|Transfer test|TestTransfer|go func\(\).*Transfer|\.Transfer\(' challenge-7/solution-template_test.go challenge-7

Repository: RezaSi/go-interview-practice

Length of output: 2771


Add self-transfer and concurrency cases to TestTransfer. The current test table covers invalid amounts and insufficient funds, but not the a == target early return or concurrent transfers that exercise the lock ordering.


func validateAmount(amount float64) error {
if amount < 0 {
return &NegativeAmountError{Amount: amount}
}
if amount > MaxTransactionAmount {
return &ExceedsLimitError{Amount: amount, Limit: MaxTransactionAmount}
}
return nil
}
Loading