-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add solution for Challenge 7 by livingpool #1882
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,138 @@ | ||||||||||||||||||||||||||||||||||||
| // Package challenge7 contains the solution for Challenge 7: Bank Account with Error Handling. | ||||||||||||||||||||||||||||||||||||
| package challenge7 | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||||||||
| "fmt" | ||||||||||||||||||||||||||||||||||||
| "sync" | ||||||||||||||||||||||||||||||||||||
| // Add any other necessary imports | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // BankAccount represents a bank account with balance management and minimum balance requirements. | ||||||||||||||||||||||||||||||||||||
| type BankAccount struct { | ||||||||||||||||||||||||||||||||||||
| ID string | ||||||||||||||||||||||||||||||||||||
| Owner string | ||||||||||||||||||||||||||||||||||||
| Balance float64 | ||||||||||||||||||||||||||||||||||||
| MinBalance float64 | ||||||||||||||||||||||||||||||||||||
| mu sync.Mutex // For thread safety | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Constants for account operations | ||||||||||||||||||||||||||||||||||||
| const ( | ||||||||||||||||||||||||||||||||||||
| MaxTransactionAmount = 10000.0 // Example limit for deposits/withdrawals | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Custom error types | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // AccountError is a general error type for bank account operations. | ||||||||||||||||||||||||||||||||||||
| type AccountError struct { | ||||||||||||||||||||||||||||||||||||
| StatusCode int | ||||||||||||||||||||||||||||||||||||
| Message string | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| func (e *AccountError) Error() string { | ||||||||||||||||||||||||||||||||||||
| // Implement error message | ||||||||||||||||||||||||||||||||||||
| return fmt.Sprintf("Account Error, code=%d, message=%s", e.StatusCode, e.Message) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // InsufficientFundsError occurs when a withdrawal or transfer would bring the balance below minimum. | ||||||||||||||||||||||||||||||||||||
| type InsufficientFundsError struct { | ||||||||||||||||||||||||||||||||||||
| // Implement this error type | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| func (e *InsufficientFundsError) Error() string { | ||||||||||||||||||||||||||||||||||||
| // Implement error message | ||||||||||||||||||||||||||||||||||||
| return "" | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // NegativeAmountError occurs when an amount for deposit, withdrawal, or transfer is negative. | ||||||||||||||||||||||||||||||||||||
| type NegativeAmountError struct { | ||||||||||||||||||||||||||||||||||||
| // Implement this error type | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| func (e *NegativeAmountError) Error() string { | ||||||||||||||||||||||||||||||||||||
| // Implement error message | ||||||||||||||||||||||||||||||||||||
| return "" | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // ExceedsLimitError occurs when a deposit or withdrawal amount exceeds the defined limit. | ||||||||||||||||||||||||||||||||||||
| type ExceedsLimitError struct { | ||||||||||||||||||||||||||||||||||||
| // Implement this error type | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| func (e *ExceedsLimitError) Error() string { | ||||||||||||||||||||||||||||||||||||
| // Implement error message | ||||||||||||||||||||||||||||||||||||
| return "" | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // NewBankAccount creates a new bank account with the given parameters. | ||||||||||||||||||||||||||||||||||||
| // It returns an error if any of the parameters are invalid. | ||||||||||||||||||||||||||||||||||||
| func NewBankAccount(id, owner string, initialBalance, minBalance float64) (*BankAccount, error) { | ||||||||||||||||||||||||||||||||||||
| if id == "" || owner == "" { | ||||||||||||||||||||||||||||||||||||
| return nil, &AccountError{} | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if minBalance < 0 || initialBalance < 0 { | ||||||||||||||||||||||||||||||||||||
| return nil, &NegativeAmountError{} | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if initialBalance < minBalance { | ||||||||||||||||||||||||||||||||||||
| return nil, &InsufficientFundsError{} | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| return &BankAccount{ID: id, Owner: owner, Balance: initialBalance, MinBalance: minBalance, mu: sync.Mutex{}}, nil | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Deposit adds the specified amount to the account balance. | ||||||||||||||||||||||||||||||||||||
| // It returns an error if the amount is invalid or exceeds the transaction limit. | ||||||||||||||||||||||||||||||||||||
| func (a *BankAccount) Deposit(amount float64) error { | ||||||||||||||||||||||||||||||||||||
| // Implement deposit functionality with proper error handling | ||||||||||||||||||||||||||||||||||||
| if amount < 0 { | ||||||||||||||||||||||||||||||||||||
| return &NegativeAmountError{} | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if amount > MaxTransactionAmount { | ||||||||||||||||||||||||||||||||||||
| return &ExceedsLimitError{} | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| a.mu.Lock() | ||||||||||||||||||||||||||||||||||||
| a.Balance += amount | ||||||||||||||||||||||||||||||||||||
| a.mu.Unlock() | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Withdraw removes the specified amount from the account balance. | ||||||||||||||||||||||||||||||||||||
| // It returns an error if the amount is invalid, exceeds the transaction limit, | ||||||||||||||||||||||||||||||||||||
| // or would bring the balance below the minimum required balance. | ||||||||||||||||||||||||||||||||||||
| func (a *BankAccount) Withdraw(amount float64) error { | ||||||||||||||||||||||||||||||||||||
| // Implement withdrawal functionality with proper error handling | ||||||||||||||||||||||||||||||||||||
| if amount < 0 { | ||||||||||||||||||||||||||||||||||||
| return &NegativeAmountError{} | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if amount > MaxTransactionAmount { | ||||||||||||||||||||||||||||||||||||
| return &ExceedsLimitError{} | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if a.Balance-amount < a.MinBalance { | ||||||||||||||||||||||||||||||||||||
| return &InsufficientFundsError{} | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| a.mu.Lock() | ||||||||||||||||||||||||||||||||||||
| a.Balance -= amount | ||||||||||||||||||||||||||||||||||||
| a.mu.Unlock() | ||||||||||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+110
to
+116
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win TOCTOU race: balance check is outside the mutex. Line 110 reads 🔒 Proposed fix if amount > MaxTransactionAmount {
return &ExceedsLimitError{}
}
- if a.Balance-amount < a.MinBalance {
- return &InsufficientFundsError{}
- }
- a.mu.Lock()
- a.Balance -= amount
- a.mu.Unlock()
+ a.mu.Lock()
+ defer a.mu.Unlock()
+ if a.Balance-amount < a.MinBalance {
+ return &InsufficientFundsError{}
+ }
+ a.Balance -= amount
return nil📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Transfer moves the specified amount from this account to the target account. | ||||||||||||||||||||||||||||||||||||
| // It returns an error if the amount is invalid, exceeds the transaction limit, | ||||||||||||||||||||||||||||||||||||
| // or would bring the balance below the minimum required balance. | ||||||||||||||||||||||||||||||||||||
| func (a *BankAccount) Transfer(amount float64, target *BankAccount) error { | ||||||||||||||||||||||||||||||||||||
| // Implement transfer functionality with proper error handling | ||||||||||||||||||||||||||||||||||||
| if amount < 0 { | ||||||||||||||||||||||||||||||||||||
| return &NegativeAmountError{} | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if amount > MaxTransactionAmount { | ||||||||||||||||||||||||||||||||||||
| return &ExceedsLimitError{} | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| if a.Balance-amount < a.MinBalance { | ||||||||||||||||||||||||||||||||||||
| return &InsufficientFundsError{} | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| a.mu.Lock() | ||||||||||||||||||||||||||||||||||||
| a.Balance -= amount | ||||||||||||||||||||||||||||||||||||
| target.Balance += amount | ||||||||||||||||||||||||||||||||||||
| a.mu.Unlock() | ||||||||||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+122
to
+137
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: sed -n '1,260p' challenge-7/submissions/livingpool/solution-template.goRepository: RezaSi/go-interview-practice Length of output: 4049 🏁 Script executed: rg -n --hidden --glob '!**/.git/**' -e 'Transfer' -e 'MinBalance' -e 'deadlock' -e 'concurr' challenge-7 .Repository: RezaSi/go-interview-practice Length of output: 50385 🏁 Script executed: sed -n '250,430p' challenge-7/solution-template_test.go && printf '\n--- README ---\n' && sed -n '1,120p' challenge-7/README.md && printf '\n--- learning ---\n' && sed -n '110,150p' challenge-7/learning.mdRepository: RezaSi/go-interview-practice Length of output: 8909 Transfer should lock both accounts and guard nil targets. |
||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
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
Empty
Error()strings defeat the purpose of the error interface.InsufficientFundsError,NegativeAmountError, andExceedsLimitErrorall return"", so logging or wrapping these errors yields no diagnostic text. The tests only inspect the concrete type via%T, which masks this, but any real caller printing the error gets a blank message. Give each a meaningful message (following theAccountErrorpattern).♻️ Example implementation
func (e *InsufficientFundsError) Error() string { - // Implement error message - return "" + return "insufficient funds: withdrawal would bring balance below minimum" }func (e *NegativeAmountError) Error() string { - // Implement error message - return "" + return "amount must not be negative" }func (e *ExceedsLimitError) Error() string { - // Implement error message - return "" + return fmt.Sprintf("amount exceeds transaction limit of %.2f", MaxTransactionAmount) }📝 Committable suggestion