Clean Code
Hamdi Ahmadi Muzakkiy
Outline
- Why ?
- Meaningful Name
- Function
- Comments
- Error Handling
- Unit Test
- Case
-
Golang Coding Tools
Why ?
- Collaborate With Other
- Easy to Improve
- Make Development Faster
- Minimize Bug On Future
Meaningful Name
Use Intention-Revealing Names
avoid :
- var d int // days
use :
- var days int // days
Make Meaningful Distinction
avoid :
- func CopyString(s1, s2 string){ ... }
use :
- func CopyString(source, destination string){ ... }
Class Name
class, object or struct better use noun
avoid :
- type Data struct {}
use :
- type Customer struct { }
Method Name
method name better use verb
avoid :
- func (customer Customer) Name(){ ... }
use :
- func (customer Customer) GetName(){ ... }
Don't Be Cute
avoid :
- func (cart Cart) HolyHandGrenade(){ ... }
use :
- func (cart Cart) DeleteItem(){ ... }
Pick 1 Word Perconcept
method name better use verb
avoid :
- func (customer Customer) GetName(){ ... }
- func (customer Customer) FetchAge(){ ... }
- func (customer Customer) RetrieveHobby(){ ... }
func CompareAppVersion(s1, s2 string) int {
tmps1 := strings.Split(s1, ".")
tmps2 := strings.Split(s2, ".")
s1L := len(s1)
s2L := len(s2)
ss1 := 0
ss2 := 0
for {
if ss1 == s1L && ss2 == s2L {
return 0
}
if ss1 == s1L {
return 1
}
if ss2 == s2L {
return -1
}
a1, _ := strconv.Atoi(tmps1[ss1])
a2, _ := strconv.Atoi(tmps2[ss2])
if a1 > a2 {
return -1
} else if a1 < s2 {
return 1
}
ss1++
ss2++
}
return 0
}
func CompareAppVersion(appVersion, appVersionCompared string) int {
splitedAppVersion := strings.Split(appVersion, ".")
splitedAppVersionCompared := strings.Split(appVersionCompared, ".")
splitedAppVersionLength := len(splitedAppVersion)
splitedAppVersionComparedLength := len(splitedAppVersionCompared)
start := 0
startCompared := 0
for {
if start == splitedAppVersionLength && startCompared == splitedAppVersionComparedLength {
return APPVERSION_EQUAL // 0
}
if start == splitedAppVersionLength {
return COMPARED_APPVERSION_HIGHER // 1
}
if startCompared == splitedAppVersionComparedLength {
return APPVERSION_HIGHER // -1
}
numAppVersion, _ := strconv.Atoi(splitedAppVersion[start])
numAppVersionCompared, _ := strconv.Atoi(splitedAppVersionCompared[startCompared])
if numAppVersion > numAppVersionCompared {
return -1
} else if numAppVersion < numAppVersionCompared {
return 1
}
start++
startCompared++
}
return APPVERSION_EQUAL
}
Const
func (payment Payment) EvaluatePayment() int {
if ... {
return 1
}
...
if ... {
return 2
}
return 3
}
func (payment Payment) EvaluatePayment() int {
if ... {
return PAYMENT_ABUSE
}
...
if ... {
return PAYMENT_FAILED
}
return PAYMENT_SUCCESS
}
Function
Small!
- should be small
- should be smaller than that
- should be no bigger than a screen-full
func (adDataGroup *AdDataGroup) GetAdsRawData(from, to string) error {
var result AdDataGroup
param := url.Values{}
param.Add("api_token", AMod.Config.API["blacklotus"].Token)
param.Add("from", from)
param.Add("to", to)
agent := utils.NewHTTPRequest()
agent.Url = AMod.Config.API["blacklotus"].URL
agent.Path = AMod.Config.API["blacklotus"].Path
agent.Method = "GET"
agent.Param = param
agent.Timeout = time.Duration(60 * 60 * time.Second)
body, err := agent.DoReq()
if err != nil {
utils.Error(err)
return err
}
repostCSVString := string(*body)
reportCSVGroup := csv.NewReader(strings.NewReader(repostCSVString))
flags := true
for {
record, err := reportCSVGroup.Read()
// passed first rec
if flags {
flags = !flags
continue
}
if err == io.EOF {
break
}
if err != nil {
utils.Error(err)
return err
}
var adData AdData
adData.actionSetReport(record)
result = append(result, adData)
}
*adDataGroup = result
return nil
}
func (adDataGroup *AdDataGroup) GetAdsRawData(from, to string) *io.Reader {
var result AdDataGroup
param := url.Values{}
param.Add("api_token", AMod.Config.API["blacklotus"].Token)
param.Add("from", from)
param.Add("to", to)
agent := utils.NewHTTPRequest()
agent.Url = AMod.Config.API["blacklotus"].URL
agent.Path = AMod.Config.API["blacklotus"].Path
agent.Method = "GET"
agent.Param = param
agent.Timeout = time.Duration(60 * 60 * time.Second)
body, err := agent.DoReq()
if err != nil {
utils.Error(err)
return err
}
repostCSVString := string(*body)
reportCSVGroup := csv.NewReader(strings.NewReader(repostCSVString))
return reportCSVGroup
}
func (adDataGroup *AdDataGroup) MappingAdsCSVData (csvData *io.Reader) {
flags := true
for {
record, err := csvData.Read()
// passed first rec
if flags {
flags = !flags
continue
}
if err == io.EOF {
break
}
if err != nil {
utils.Error(err)
return err
}
var adData AdData
adData.actionSetReport(record)
result = append(result, adData)
}
*adDataGroup = result
return nil
}
Do One Thing
-
Should do one thing
Arguments
-
passed data from argument rather instance
// Avoid
func (adData AdData) isOperatorContainOnOperatorList() bool {
operatorGroup := GetOperatorGroup()
searchedOperator := strings.ToLower(adData.Operator)
for _, operator := range operatorGroup {
if strings.Contains(searchedOperator, operator) {
return true
}
}
return false
}
// Use
func (adData AdData) isOperatorContainOnOperatorList(operatorGroup []string) bool {
searchedOperator := strings.ToLower(adData.Operator)
for _, operator := range operatorGroup {
if strings.Contains(searchedOperator, operator) {
return true
}
}
return false
}
Arguments
- avoid boolean arguments
// Avoid
func rander(flag bool) {
if flag {
...
return
}
...
}
// Use
func randerHomePage() {
...
}
func randerLoginPage() {
...
}
Arguments
-
Ideal:
- Zero ( niladic ) // Use
- One ( Monodic )
- Two ( dyadic )
- three ( triadic ) // Avoid
- more then three ( polyadic ) // Don't Use
// Avoid
func MakeCircle(x, y ,radius float) Circle {
...
}
// strategy : make x and y an object/struct
// Use
func MakeCircle(point Center, radius float) Circle {
...
}
Have No Side Effects
-
Make sure a function have no side effect
func CheckPassword(username, password string) bool {
var user User
user = getUserPassword(username)
if user.Password == password {
user.SetSession()
return true
}
return false
}
func (cartGroup *[]Cart) CalculateTotalPaymentWithDiscount() int {
for _, cart := cartGroup {
if cart.Amount < 100000 {
cartGroup.Total -= (cart.Amount/0.1)
} else {
cartGroup.Total -= (cart.Amount/0.5)
}
}
return cartGroup.Total
}
Comments
Good Comments
truly good comment is the comment you found a way not to write
Legal Comments
// Copyright (C) 2003,2004,2005 by Object Mentor, Inc. All rights reserved. // Released under the terms of the GNU General Public License version 2 or later.
Informative Comments
// format matched kk:mm:ss EEE, MMM dd, yyyy
Warning of Consequences
// Don't run unless you // have some time to kill.
Bad Comments
-
Redundant Comments (take more time to read comment then read the code )
- Journal Comments
- Noise Comment ( Comment for obvious function )
Error Handling
f, err := os.Open("filename.ext")
if err != nil {
...
}
if err := os.Unsetenv("test"); err != nil {
...
}
Unit Test
http://slides.com/hamdiahmadi/tdd#/
Casê
type Event struct {
Name string
Schedule time.Time
}
func NotifyEventA() { ... }
func NotifyEventB() { ... }
func NotifyEventC() { ... }
func (event Event) SetCron(option string) {
cronClient := cron.New()
if option == "event a" {
cronClient.AddFunc(FormatingSchedule(event.Schedule), func(){
NotifyEventA()
})
} else if option == "event b" {
cronClient.AddFunc(FormatingSchedule(event.Schedule), func(){
NotifyEventB()
})
} else if option == "event c" {
cronClient.AddFunc(FormatingSchedule(event.Schedule), func(){
NotifyEventC()
})
}
cronClient.Start()
}
func main() {
event := Event{
Name : "Event Something"
Schedule : time.Now().Add(24 * time.Hour * 10)
}
event.SetCron("event a")
}
type Event struct {
Name string
Schedule time.Time
}
type NotifyJob func()
func NotifyEventA() { ... }
func NotifyEventB() { ... }
func NotifyEventC() { ... }
func (event Event) SetCron(job NotifyJob) {
cronClient := cron.New()
cronClient.AddFunc(FormatingSchedule(event.Schedule), func(){
job()
})
cronClient.Start()
}
func main() {
event := Event{
Name : "Event Something"
Schedule : time.Now().Add(24 * time.Hour * 10)
}
event.SetCron(NotifyEventA)
}
Golang Coding Tools
Go vet
- Useless assignments
- Common mistakes when using sync/atomic
-
Invalid +build tags
- Using composite literals without keyed fields
- Passing locks by value
- Comparing functions with nil
- Using wrong printf format specifiers
- Closing over loop variables the wrong way
- Struct tags that do not follow the canonical format
- Unreachable code
- Misuse of unsafe.Pointer
- Mistakes involving boolean operators
go vet github.com/username/repository/src/package
Err Check
errcheck github.com/username/repository/src/package
Go Lint
golint src/package
Gofmt
gofmt -w src/package
Clean Code
By hamdi ahmadi
Clean Code
- 417