6
\$\begingroup\$

I am learning Go and want to do things the go-way, please have a look this code suggest how to make it better Go.

This is a simulation to determine how items of a particular type of clothing you need. Rules are as follows:

  • Simulation starts with all the clothes in the clean pile
  • Every morning you take one item from the clean pile and add it to the dirty pile (assuming an item of clothing becomes dirty the moment you put it on)
  • On wash days - before you put on a new item - all the items in the dirty pile go into washing pile
  • Every morning all items in the washing pile are put into the clean pile

Assuming there is a washing day every Monday, how many items of clothing do you need to never run out of clean ones?

There are some extra rules (I have made them flags):

  • Every second Thursday is also a washing day
  • There is an extra step in the washing process - drying. After an item is washed, the next day it is put into the drying pile, and only after drying does it go into the clean pile.

Code in the go-playground at: https://play.golang.org/p/3jawv55Y3p

package main

import (
    "flag"
    "fmt"
    "time"
)

func even(number int) bool {
    return number%2 == 0
}

func isWashingDay(today time.Time, thursday bool) bool {
    if today.Format("Mon") == "Mon" {
        fmt.Println("Monday!")
        return true
    } else if thursday && today.Format("Mon") == "Thu" {
        _, week := today.ISOWeek()
        if even(week) {
            fmt.Println("Thursday!")
            return true
        }
    }
    return false
}

func main() {
    numberOftshirtsPtr := flag.Int("shirts", 9, "a int number of tshirts")
    thursdayPtr := flag.Bool("thursday", true, "a bool use Thursday")
    dryingPtr := flag.Bool("drying", true, "a bool use drying")
    flag.Parse()

    var clean, minClean = *numberOftshirtsPtr, *numberOftshirtsPtr
    var dirty, washing, drying int
    today, _ := time.Parse(time.RFC3339, "2017-02-06T00:00:00+00:00")

    fmt.Println("Date       | C | D | W | Y ")

    testDays := 365
    for i := 0; i < testDays; i++ {
        if *dryingPtr {
            clean += drying
            drying = 0

            drying = washing
            washing = 0
        } else {
            clean += washing
            washing = 0
        }

        if clean == 0 {
            fmt.Println("Run out of clean shirts!")
            break
        }

        if isWashingDay(today, *thursdayPtr) {
            washing = dirty
            dirty = 0
        }

        // take a clean tshirt and wear it - it immediately becomes dirty
        clean--
        dirty++

        if clean < minClean {
            minClean = clean
        }

        fmt.Printf("%s | %d | %d | %d | %d \n", today.Format("2006-01-02"), clean, dirty, washing, drying)

        today = today.AddDate(0, 0, 1)
    }

    fmt.Println("Total days:", testDays)
    fmt.Println("Minimum clean shirts:", minClean)
}
\$\endgroup\$
2
\$\begingroup\$

For a beginner to go your program is good. You have obviously read up on flag handling, and you've got a grasp on the pointers. I have come to like the flag processing in go (though having previously learned/understood getopt and written my own library in Java, I feel like go should allow more powerful commandline handling).

Flags

So, having said you have a good grasp on those concepts, I am going to recommend that you change the flag handling (and as a consequence, you remove the pointers).

In addition, you should be putting in the number of days to simulate, and perhaps the starting date, as flags too.

So, flag functions include both ...Ptr and ...Var variants. I recommend using the ...Var variants where you can. Your code:

numberOftshirtsPtr := flag.Int("shirts", 9, "a int number of tshirts")
thursdayPtr := flag.Bool("thursday", true, "a bool use Thursday")
dryingPtr := flag.Bool("drying", true, "a bool use drying")
flag.Parse()

I would write like:

numberOftshirts := 9
doThursdays := true
doDrying := true

flag.IntVar(&numberOftshirts, "shirts", numberOftshirts, "the number of tshirts")
flag.BoolVar(&doThursdays, "thursday", doThursdays, "use Thursday")
flag.BoolVar(&doDrying, "drying", doDrying, "use drying")
flag.Parse()

By taking the address of the var and giving it to flag, we can then use the variables as-is later without having to keep the pointer handling at all, so code like:

var clean, minClean = *numberOftshirtsPtr, *numberOftshirtsPtr

becomes:

var clean, minClean = numberOftshirts, numberOftshirts

Note that, by convention, using hungarian notation in go is not good code-style. You should not suffix pointer variables with Ptr. A pointer to an int containing the number of T-Shirts is still numberOftshirts and not numberOftshirtsPtr

OK, so the above code changes the flag handling to be Var-based, and it reads easier, and removes all pointer references later.

I would also add flags for the starting date, and number of days to simulate.

Washing Day

Your washingday function is a good idea, but you are doing string processing in places where the time library has better options to offer. Note that time has constantes for the days-of-week, and those constants are declared as a Weekday type, and that type has a String() function available: https://golang.org/pkg/time/#Weekday - what this means is that you can avoid the string-conversions in the function. I personally would probably use the String() option too to print the days. Actually, I would remove the println from the function because it is making the function do too much - (computation and presentation). I would have your function as:

func isWashingDay(today time.Time, doThursday bool) bool {

    if today.Weekday() == time.Monday {
        //fmt.Printf("%v!\n", time.Monday)
        return true
    }

    if doThursday && today.Weekday() == time.Thursday {
        _, week := today.ISOWeek()
        if even(week) {
            //fmt.Printf("%v!\n", time.Thursday)
            return true
        }
    }

    return false
}

Note that I no longer have any raw text in there (it's all time variables, etc), and also note that I no longer have if ... else ... statements. When the if part of a condition always has a return in it, there's no need for the else at all.

Main

Your program is essentially contained inside the main-method. This makes the main method bulky, and the code is not reusable. Even in example progams and learning exercises, you should try to break your code in to functions that can be run, and tested independently. Go has a strong toolset related to unit-tests and benchmarks, and you should get in the habit of creating small, independent functions that are easy to process in the testing systems too.

One way to improve the bulkiness of the main method is to declare a struct to contain the state of a given day. You can get clever with some function processing too. I would create a state struct, and use it for a bunch of the logic....

type ClothesState struct {
    day     time.Time
    clean   int
    dirty   int
    washing int
    drying  int
}

In addition, to separate out your printing of the state, I would create a bit of a string helper function... I'll explain it later, but show you here, now:

func format(day, clean, dirty, washing, drying interface{}) string {
    return fmt.Sprintf("%-11v| %-2v| %-2v| %-2v| %2-v", day, clean, dirty, washing, drying)
}

With this function, we can add a String() method to our state too that uses it:

func (cs ClothesState) String() string {
    return format(cs.day.Format("2006-01-02"), cs.clean, cs.dirty, cs.washing, cs.drying)
}

Now we can use the %v (or %s) style fmt to print the state:

fmt.Printf("%v\n", state)

Further, I would take the state-tansition logic and make it a method on the struct too:

func (cs ClothesState) Advance(doDrying, doThursdays bool) ClothesState {
    tomorrow := cs.day.AddDate(0, 0, 1)

    clean := cs.clean
    clean += cs.drying

    drying := cs.washing
    if !doDrying {
        // take all the drying directly in to clean
        clean += drying
        drying = 0
    }

    washing := 0
    dirty := cs.dirty
    if isWashingDay(tomorrow, doThursdays) {
        washing = dirty
        dirty = 0
    }

    clean--
    dirty++

    return ClothesState{
        day:     tomorrow,
        clean:   clean,
        dirty:   dirty,
        washing: washing,
        drying:  drying,
    }

}

Now you have an immutable state struct that you can then advance through the daily logic. Each advance returns a new state.

Conclusion

You've made a great start in to Go. I encourage the use of structs to localize logic, the use of smaller, single-purpose functions, and separating the presentation code (printlns) from the calculation code.

I have taken your code and re-worked it in a way I would consider an improvement, and I have tweaked the starting state to match your logic (and also kept the println statements in the isWashingDay so that it matches the output of your program.

Have a look, and see how the logic is separated, how the flags are used, and so on: https://play.golang.org/p/B62HmIoxNc

package main

import (
    "flag"
    "fmt"
    "time"
)

func even(number int) bool {
    return number%2 == 0
}

func isWashingDay(today time.Time, doThursday bool) bool {

    if today.Weekday() == time.Monday {
        fmt.Printf("%v!\n", time.Monday)
        return true
    }

    if doThursday && today.Weekday() == time.Thursday {
        _, week := today.ISOWeek()
        if even(week) {
            fmt.Printf("%v!\n", time.Thursday)
            return true
        }
    }

    return false
}

func format(day, clean, dirty, washing, drying interface{}) string {
    return fmt.Sprintf("%-11v| %-2v| %-2v| %-2v| %-2v", day, clean, dirty, washing, drying)
}

type ClothesState struct {
    day     time.Time
    clean   int
    dirty   int
    washing int
    drying  int
}

func (cs ClothesState) String() string {
    return format(cs.day.Format("2006-01-02"), cs.clean, cs.dirty, cs.washing, cs.drying)
}

func (cs ClothesState) Advance(doDrying, doThursdays bool) ClothesState {
    tomorrow := cs.day.AddDate(0, 0, 1)

    clean := cs.clean
    clean += cs.drying

    drying := cs.washing
    if !doDrying {
        // take all the drying directly in to clean
        clean += drying
        drying = 0
    }

    washing := 0
    dirty := cs.dirty
    if isWashingDay(tomorrow, doThursdays) {
        washing = dirty
        dirty = 0
    }

    clean--
    dirty++

    return ClothesState{
        day:     tomorrow,
        clean:   clean,
        dirty:   dirty,
        washing: washing,
        drying:  drying,
    }

}

func main() {
    numberOftshirts := 9
    doThursdays := true
    doDrying := true

    flag.IntVar(&numberOftshirts, "shirts", numberOftshirts, "the number of tshirts")
    flag.BoolVar(&doThursdays, "thursday", doThursdays, "use Thursday")
    flag.BoolVar(&doDrying, "drying", doDrying, "use drying")
    flag.Parse()

    today, _ := time.Parse(time.RFC3339, "2017-02-06T00:00:00+00:00")

    // Set the state with 1 dirty shirt to match OP logic
    state := ClothesState{
        day:   today,
        clean: numberOftshirts - 1,
        dirty: 1,
    }

    minClean := state.clean

    fmt.Println(format("Date", "C", "D", "W", "Y"))
    fmt.Printf("%v\n", state)

    testDays := 365
    for i := 0; i < testDays; i++ {
        state = state.Advance(doDrying, doThursdays)
        fmt.Printf("%v\n", state)

        if state.clean < minClean {
            minClean = state.clean
        }

    }

    fmt.Println("Total days:", testDays)
    fmt.Println("Minimum clean shirts:", minClean)
}
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.