Merge pull request #2 from crmejia/cm/improving_CLI

improving cli and linter

README.md

@@ -9,13 +9,9 @@ habit. You can list all your streaks with `habit`. Also, you can create a weekly
 like so `habit -f weekly piano`
 ```
 Usage of habit:
-  -f string
-    	Set the frecuency of the habit: daily, weekly. (shorthand) (default "daily")
-  -frequency string
-    	Set the frecuency of the habit: daily, weekly. (default "daily")
-  -s	Runs habit as a HTTP Server (shorthand)
-  -server
-    	Runs habit as a HTTP Server
+-f string
+    	Set the frequency of the habit: daily, weekly. (default "daily")
+  -s	Runs habit as a HTTP Server
 ```
 
 ### Server Mode

cli.go

@@ -7,23 +7,30 @@ import (
 	"log"
 )
 
+const (
+	frequencyUsage  = "Set the frequency of the habit: daily, weekly."
+	serverModeUsage = "Runs habit as a HTTP Server"
+	helpIntro       = `habit is an application to assist you in building habits
+     habit <options> <HABIT_NAME> -- to create/update a new habit
+habit  -- to list all habits`
+)
+
+//RunCLI parses arguments and runs habit tracker
 func RunCLI(filename string, args []string, output io.Writer) {
-	flagSet := flag.NewFlagSet("habit", flag.ExitOnError)
+	flagSet := flag.NewFlagSet("habit", flag.ContinueOnError)
 	flagSet.SetOutput(output)
 
-	var frequency string
-	flagSet.StringVar(&frequency, "frequency", "daily", frequency_usage)
-	flagSet.StringVar(&frequency, "f", "daily", frequency_usage+shorthand)
+	frequency := flagSet.String("f", "daily", frequencyUsage)
+	serverMode := flagSet.Bool("s", false, serverModeUsage)
 
-	var serverMode bool
-	flagSet.BoolVar(&serverMode, "server", false, serverMode_usage)
-	flagSet.BoolVar(&serverMode, "s", false, serverMode_usage+shorthand)
-
-	flagSet.Parse(args)
+	err := flagSet.Parse(args)
+	if err != nil {
+		log.Fatal(err)
+	}
 
 	if len(flagSet.Args()) > 1 {
 		fmt.Fprintln(output, "too many args")
-		fmt.Fprintln(output, help_intro)
+		fmt.Fprintln(output, helpIntro)
 		flagSet.Usage()
 		return
 	}
@@ -32,50 +39,39 @@ func RunCLI(filename string, args []string, output io.Writer) {
 	if err != nil {
 		log.Fatal(err)
 	}
-	//TODO move this to runCLI or at least move after parsing
-	//this might mean making changing to AllHabits(store) instead of a method on Tracker
+
 	if len(args) == 0 {
 		if len(tracker) > 0 {
 			fmt.Fprintln(output, AllHabits(store))
 			return
-		} else {
-			// no previous habits
-			fmt.Fprintln(output, help_intro)
-			flagSet.Usage()
-			return
 		}
+		// no previous habits
+		fmt.Fprintln(output, helpIntro)
+		flagSet.Usage()
+		return
 	}
 
-	if len(flagSet.Args()) == 0 && !serverMode {
+	if len(flagSet.Args()) == 0 && !(*serverMode) {
 		// no habit specified
-		fmt.Fprintln(output, help_intro)
+		fmt.Fprintln(output, helpIntro)
 		flagSet.Usage()
 		return
 	}
 
-	if serverMode {
+	if *serverMode {
 		address := defaultTCPAddress
 		if len(flagSet.Args()) > 0 {
 			address = flagSet.Args()[0]
 		}
 		runHTTPServer(store, address)
 	} else {
 		habitName := flagSet.Args()[0]
-		runCLI(store, habitName, frequency)
+		runCLI(store, habitName, *frequency)
 	}
 	return
 }
 
-const (
-	frequency_usage  = "Set the frecuency of the habit: daily, weekly."
-	serverMode_usage = "Runs habit as a HTTP Server"
-	shorthand        = " (shorthand)"
-	help_intro       = `habit is an application to assist you in building habits
-     habit <options> <HABIT_NAME> -- to create/update a new habit
-habit  -- to list all habits`
-)
-
-func runCLI(store Storable, habitName, frequency string) {
+func runCLI(store Store, habitName, frequency string) {
 	ht := NewTracker(store)
 	habit, ok := ht.FetchHabit(habitName)
 
@@ -91,7 +87,10 @@ func runCLI(store Storable, habitName, frequency string) {
 			fmt.Printf("unknown frecuency %s", frequency)
 			return
 		}
-		ht.CreateHabit(habit)
+		err := ht.CreateHabit(habit)
+		if err != nil {
+			log.Fatal(err)
+		}
 	}
 
 	err := store.Write(&ht)
@@ -102,7 +101,7 @@ func runCLI(store Storable, habitName, frequency string) {
 	fmt.Println(habit)
 }
 
-func runHTTPServer(store Storable, address string) {
+func runHTTPServer(store Store, address string) {
 	server := NewServer(store, address)
 	server.Run()
 }

habit.go

@@ -10,6 +10,23 @@ import (
 	"errors"
 )
 
+const (
+	NewMessage MessageKind = iota
+	RepeatMessage
+	StreakMessage
+	BrokenMessage
+
+	DailyInterval  = 24 * time.Hour
+	WeeklyInterval = 7 * 24 * time.Hour
+
+	newHabit      = "Good luck with your new habit '%s'! Don't forget to do it again %s."
+	streakHabit   = "Nice work: you've done the habit '%s' for %d %s in a row now. Keep it up!"
+	repeatedHabit = "You already logged '%s' today. Keep it up!"
+	brokeStreak   = "You last did the habit '%s' %.0f %s ago, so you're starting a new streak today. Good luck!"
+	habitStatus   = "You're currently on a %d-day streak for '%s'. Stick to it!"
+)
+
+// A Habit represents a Habit state
 type Habit struct {
 	Name     string
 	Streak   int
@@ -18,10 +35,11 @@ type Habit struct {
 	Message  string
 }
 
+// A Tracker is a map of habits
 type Tracker map[string]*Habit
 
-func NewTracker(store Storable) Tracker {
-	//err := tracker.Load
+//NewTracker returns a new tracker. If there are previous habits they're loaded onto the tracker.
+func NewTracker(store Store) Tracker {
 	tracker, err := store.Load()
 
 	if err != nil {
@@ -30,6 +48,7 @@ func NewTracker(store Storable) Tracker {
 	return tracker
 }
 
+//FetchHabit returns an existing habit. If the habit doesn't exist it returns false.
 func (ht *Tracker) FetchHabit(name string) (*Habit, bool) {
 	habit, ok := (*ht)[name]
 	if !ok {
@@ -53,6 +72,7 @@ func (ht *Tracker) FetchHabit(name string) (*Habit, bool) {
 	return habit, true
 }
 
+//CreateHabit creates a new habit and inserts it in the tracker
 func (ht *Tracker) CreateHabit(habit *Habit) error {
 	_, ok := (*ht)[habit.Name]
 	if ok {
@@ -66,7 +86,9 @@ func (ht *Tracker) CreateHabit(habit *Habit) error {
 	(*ht)[habit.Name] = habit
 	return nil
 }
-func AllHabits(store Storable) string {
+
+//AllHabits returns a summary of all habits. If there are no habits it returns and empty string.
+func AllHabits(store Store) string {
 	ht := NewTracker(store)
 	var message string
 	if len(ht) > 0 {
@@ -82,6 +104,7 @@ func (h Habit) String() string {
 	return h.Message
 }
 
+//GenerateMessage creates the appropriate message for a given habit.
 func (h *Habit) GenerateMessage(kind MessageKind) {
 	var intervalString string
 	switch kind {
@@ -113,36 +136,17 @@ func (h *Habit) GenerateMessage(kind MessageKind) {
 	}
 }
 
-const (
-	newHabit      = "Good luck with your new habit '%s'! Don't forget to do it again %s."
-	streakHabit   = "Nice work: you've done the habit '%s' for %d %s in a row now. Keep it up!"
-	repeatedHabit = "You already logged '%s' today. Keep it up!"
-	brokeStreak   = "You last did the habit '%s' %.0f %s ago, so you're starting a new streak today. Good luck!"
-	habitStatus   = "You're currently on a %d-day streak for '%s'. Stick to it!"
-)
-
+//MessageKind represents the message to be displayed
 type MessageKind int
 
-const (
-	NewMessage MessageKind = iota
-	RepeatMessage
-	StreakMessage
-	BrokenMessage
-)
-
-//func SameDay returns true if the days are the same ignoring hours, mins,etc
+// SameDay returns true if the days are the same ignoring hours, minutes,etc
 func SameDay(d1, d2 time.Time) bool {
 	if d1.Year() == d2.Year() && d1.Month() == d2.Month() && d1.Day() == d2.Day() {
 		return true
 	}
 	return false
 }
 
-const (
-	DailyInterval  time.Duration = 24 * time.Hour
-	WeeklyInterval               = 7 * 24 * time.Hour
-)
-
 var validInterval = map[time.Duration]bool{
 	DailyInterval:  true,
 	WeeklyInterval: true,

server.go

@@ -7,16 +7,18 @@ import (
 	"time"
 )
 
-//todo chicken or the egg?
-//Should the Tracker have server inside? but since Tracker is a map
-//it is not possible. Think about the best away to represent this relationship
+const (
+	defaultTCPAddress = "127.0.0.1:8080"
+)
+
 type server struct {
 	Server  *http.Server
 	Tracker *Tracker
-	Store   Storable
+	Store   Store
 }
 
-func NewServer(store Storable, address string) *server {
+//NewServer returns a new server
+func NewServer(store Store, address string) *server {
 	tracker := NewTracker(store)
 	server := server{
 		Server: &http.Server{
@@ -27,8 +29,9 @@ func NewServer(store Storable, address string) *server {
 	return &server
 }
 
+//Run listens and serves http
 func (server *server) Run() {
-	router := server.Handler()
+	router := server.Router()
 	server.Server.Handler = router
 
 	err := server.Server.ListenAndServe()
@@ -37,24 +40,25 @@ func (server *server) Run() {
 	}
 }
 
-func (server *server) Handler() http.Handler {
+//Router returns a http.Handler with the appropriate routes
+func (server *server) Router() http.Handler {
 	router := http.NewServeMux()
 	router.HandleFunc("/", server.HabitHandler)
 
 	return router
 }
 
+//HabitHandler Handler that servers habits.
 func (server *server) HabitHandler(w http.ResponseWriter, r *http.Request) {
 	//parsing querystring
 	habitName := r.FormValue("habit")
 	if r.RequestURI == "/all" || r.RequestURI == "/" {
 		if len(*server.Tracker) > 0 {
 			fmt.Fprint(w, AllHabits(server.Store))
 			return
-		} else {
-			http.Error(w, "not found", http.StatusNotFound)
-			return
 		}
+		http.Error(w, "not found", http.StatusNotFound)
+		return
 	} else if habitName == "" || r.URL.Path != "/" {
 		http.Error(w, "cannot parse querystring", http.StatusBadRequest)
 		return
@@ -86,7 +90,3 @@ func (server *server) HabitHandler(w http.ResponseWriter, r *http.Request) {
 	}
 	fmt.Fprint(w, habit)
 }
-
-const (
-	defaultTCPAddress = "127.0.0.1:8080"
-)

server_test.go

@@ -123,7 +123,7 @@ func TestRouting(t *testing.T) {
 	}
 	store := habit.FileStore{Tracker: tracker}
 	habitServer := habit.NewServer(store, localHostAddress)
-	testServer := httptest.NewServer(habitServer.Handler())
+	testServer := httptest.NewServer(habitServer.Router())
 	defer testServer.Close()
 
 	testCases := []struct {