chat.go

package main

import (
	"bufio"
	"fmt"
	"log"
	"net"
	"strings"
)

const (
	ServerPort     = ":9091"
	WelcomeMessage = `Welcome to the chat server!
Please be respectful to others.
`
)

type Nickname string

type Client struct {
	Conn net.Conn
	Nick Nickname
}

// SendMessage sends a message to the client
func (c *Client) SendMessage(msg string) {
	fmt.Fprint(c.Conn, msg)
}

// SendBytes sends raw bytes to the client
func (c *Client) SendBytes(data []byte) {
	c.Conn.Write(data)
}

type addClientRequest struct {
	ClientToAdd     *Client
	SuccessCallback chan bool
}

type broadcastMessageRequest struct {
	ClientSending *Client
	Message       string
}

func main() {
	// Create a listener to listen on the chat server port
	l, err := net.Listen("tcp", ServerPort)
	if err != nil {
		log.Fatal(err)
	}
	defer l.Close()

	// Channels for clients to interact with the
	// connection manager
	addRequest := make(chan addClientRequest)
	disconnected := make(chan Nickname) // Nickname disconnected
	broadcastRequest := make(chan broadcastMessageRequest)

	// Connection manager to handle interop between
	// clients
	go func() {
		clients := make(map[Nickname]Client)

		for {
			select {
			case r := <-addRequest:
				// Check if the nickname already exists, if not, add it.
				if _, found := clients[r.ClientToAdd.Nick]; found {
					r.SuccessCallback <- false
					continue
				}
				clients[r.ClientToAdd.Nick] = *r.ClientToAdd

				// Send a welcome message with a list of users connected
				nicks := make([]string, 0, len(clients))
				for nick := range clients {
					nicks = append(nicks, string(nick))
				}
				r.ClientToAdd.SendBytes([]byte(WelcomeMessage))
				r.ClientToAdd.SendMessage(fmt.Sprintf("Welcome %s!  Clients online: %q\r\n", r.ClientToAdd.Nick, nicks))

				r.SuccessCallback <- true
			case nick := <-disconnected:
				// Remove the clients from the map, send a disconnect message
				delete(clients, nick)
				for _, client := range clients {
					client.SendMessage(fmt.Sprintf("%s has left the chat.\r\n", nick))
				}
			case req := <-broadcastRequest:
				// Broadcast a message to every client except the one sending
				for nick, client := range clients {
					if nick == req.ClientSending.Nick {
						continue
					}
					client.SendMessage(req.Message)
				}
			}
		}
	}()

	for {
		// Accept a new client connection
		c, err := l.Accept()
		if err != nil {
			log.Println(err)
			continue
		}

		// Goroutine to handle this client
		go func() {
			defer c.Close()
			scanner := bufio.NewScanner(c)
			me := Client{
				Conn: c,
			}

			// Loop until a valid nickname is given
			for validNick := false; !validNick; {
				me.SendMessage("Enter your nickname: ")
				if !scanner.Scan() {
					// Client disconnected
					return
				}

				nickAddSuccess := make(chan bool)

				me.Nick = Nickname(scanner.Text())
				addRequest <- addClientRequest{
					ClientToAdd:     &me,
					SuccessCallback: nickAddSuccess,
				}
				validNick = <-nickAddSuccess

				if !validNick {
					me.SendMessage("Sorry, That nick is already taken.\r\n")
				}
			}
			me.SendMessage("> ")
			broadcastRequest <- broadcastMessageRequest{
				ClientSending: &me,
				Message:       fmt.Sprintf("%s has joined the chat\r\n", me.Nick),
			}

			// Handle messages
			for scanner.Scan() {
				me.SendMessage("> ")
				msg := strings.TrimSpace(scanner.Text())
				if msg == "" {
					continue
				}

				broadcastRequest <- broadcastMessageRequest{
					ClientSending: &me,
					Message:       fmt.Sprintf("[%s] %s\r\n", me.Nick, scanner.Text()),
				}
			}

			// Client disconnected
			disconnected <- me.Nick
		}()
	}
}

package main

import (
	"bufio"
	"fmt"
	"log"
	"net"
	"strings"
)

const (
	ServerPort = ":9091"
	// REVIEW: nit: Does this need exported?
	WelcomeMessage = `Welcome to the chat server!
Please be respectful to others.
`
)

type Nickname string

type Client struct {
	Conn net.Conn // REVIEW: Do these need exported?
	Nick Nickname // REVIEW:
}

// SendMessage sends a message to the client
func (c *Client) SendMessage(msg string) {
	// REVIEW: Should [Client.SendMessage] include termination logic to prevent
	// mistakes?
	//
	// fmt.Fprint(c.Conn, msg+"\r\n")
	//
	// We can make a dedicated function for user prompts to handle that single
	// use-case.
	//
	// func (c *Client) Prompt(msg string) (string, error) {
	//   scanner := bufio.NewScanner(c.Conn)
	//   c.SendMessage(msg+": ")
	//   if !scanner.Scan() {
	//     return "", errors.New("client disconnected")
	//   }
	//   return scanner.Text(), nil
	fmt.Fprint(c.Conn, msg)
}

// SendBytes sends raw bytes to the client
//
// REVIEW:
//  1. Is this needed?
//  2. If so, should it just be an [io.Writer]?
//     'func (c *Client) Write(data []byte) (int, error) { return c.Conn.Write(data) }'
func (c *Client) SendBytes(data []byte) {
	c.Conn.Write(data)
}

type addClientRequest struct {
	// REVIEW: nit: We already know this is a client to add. addClientRequest.ClientToAdd => addClientRequest.Client
	ClientToAdd     *Client   // REVIEW: Do these need exported?
	SuccessCallback chan bool // REVIEW:
}

type broadcastMessageRequest struct {
	ClientSending *Client // REVIEW: Do these need exported?
	Message       string  // REVIEW:
}

func main() {
	// Create a listener to listen on the chat server port
	//
	// REVIEW: Since this is used in the entire function body, 'lis' is a common name
	// for listeners.
	l, err := net.Listen("tcp", ServerPort)
	if err != nil {
		log.Fatal(err)
	}
	// REVIEW: may want to check the close error for logging?
	//
	// defer func() {
	//   if err := l.Close(); err != nil {
	//     // Handle / log
	//   }
	// }()
	defer l.Close()

	// Channels for clients to interact with the
	// connection manager
	addRequest := make(chan addClientRequest)
	disconnected := make(chan Nickname) // Nickname disconnected
	broadcastRequest := make(chan broadcastMessageRequest)

	// Connection manager to handle interop between
	// clients
	go func() {
		// REVIEW: nit: When I see 'make()', I expect a pre-allocated size or capacity.
		// If there isn't one, just do 'map[Nickname]Client{}'
		clients := make(map[Nickname]Client)

		for {
			select {
			case r := <-addRequest:
				// Check if the nickname already exists, if not, add it.
				//
				// REVIEW: nit: This fits under the "comma ok" idiom: https://go.dev/doc/effective_go#maps
				if _, found := clients[r.ClientToAdd.Nick]; found {
					r.SuccessCallback <- false
					continue
				}
				// REVIEW: Does this actually need to be passed as a pointer if we're
				// gonna store the dereferenced value? Because on the next map lookup,
				// we'll get a shallow copy of it anyway.
				//
				// If so, should do a nil-pointer check. But because Client.Conn is
				// already a pointer under the hook, we can just throw the value over
				// the channel.
				clients[r.ClientToAdd.Nick] = *r.ClientToAdd

				// Send a welcome message with a list of users connected
				nicks := make([]string, 0, len(clients))
				for nick := range clients {
					nicks = append(nicks, string(nick))
				}
				// REVIEW: nit: r.ClientToAdd.SendMessage(WelcomeMessage)
				r.ClientToAdd.SendBytes([]byte(WelcomeMessage))
				// REVIEW: [Client.SendMessage] could be changed to make this easier
				//
				// func (c *Client) SendMessage(msg string, a ...any) {
				//   fmt.Fprintf(c.Conn, msg+"\r\n", a...)
				// }
				//
				// r.ClientToAdd.SendMessage("Welcome %s!", r.ClientToAdd.Nick)
				r.ClientToAdd.SendMessage(fmt.Sprintf("Welcome %s!  Clients online: %q\r\n", r.ClientToAdd.Nick, nicks))

				r.SuccessCallback <- true
			case nick := <-disconnected:
				// Remove the clients from the map, send a disconnect message
				delete(clients, nick)
				for _, client := range clients {
					client.SendMessage(fmt.Sprintf("%s has left the chat.\r\n", nick))
				}
			case req := <-broadcastRequest:
				// Broadcast a message to every client except the one sending
				for nick, client := range clients {
					if nick == req.ClientSending.Nick {
						continue
					}
					client.SendMessage(req.Message)
				}
			}
		}
	}()

	for {
		// Accept a new client connection
		c, err := l.Accept()
		if err != nil {
			log.Println(err)
			continue
		}

		// Goroutine to handle this client
		go func() {
			defer c.Close()
			scanner := bufio.NewScanner(c)
			me := Client{
				Conn: c,
			}

			// Loop until a valid nickname is given
			//
			// REVIEW: There's a risk of goroutine growth if clients connect without
			// choosing a nickname. We can add a timeout:
			//
			// ctx, cancel := context.WithTimeout(time.Minute)
			// defer cancel()
			// go func() {
			//   for range ctx.Done() {
			//     c.Close()
			//   }
			// }
			//
			// for {
			//   nickname, err := me.Prompt("Enter your nickname")
			//   if err != nil {
			//     // Client disconnected or we timed them out
			//     return
			//   }
			//   if err := engine.Register(c, nickname); err != nil {
			//     continue
			//   }
			// }
			// cancel()
			for validNick := false; !validNick; {
				// REVIEW: If applying earlier feedback:
				//
				// nickname, err := me.Prompt("Enter your nickname")
				me.SendMessage("Enter your nickname: ")
				if !scanner.Scan() {
					// Client disconnected
					return
				}

				// REVIEW: Passing channels through a channel for responses can be
				// super useful, but ideally it's hidden inside the implementation.
				// Otherwise a client can pass a nil or buffered channel, risking panic
				// or unexpected semantics.
				//
				// I would wrap the "engine" in it's own type holding the map of
				// clients, with a 'loop()' method or something that selects over those
				// three channels.
				//
				// But then have a separate method that hides the success callback:
				//
				// func (e *engine) Register(c Conn, nickname string) error {
				//   success := make(chan bool)
				//   e.addRequest <- addClientReqeuest{/* ... */}
				//   return <-success
				// }
				//
				// Then the callsite here looks like:
				//
				// if err := engine.Register(c, nickname); err != nil {
				//   me.SendMessage("Sorry, that nick is already taken.")
				//   continue
				// }
				nickAddSuccess := make(chan bool)

				me.Nick = Nickname(scanner.Text())
				addRequest <- addClientRequest{
					ClientToAdd:     &me,
					SuccessCallback: nickAddSuccess,
				}
				validNick = <-nickAddSuccess

				if !validNick {
					me.SendMessage("Sorry, That nick is already taken.\r\n")
				}
			}
			me.SendMessage("> ")
			broadcastRequest <- broadcastMessageRequest{
				ClientSending: &me,
				Message:       fmt.Sprintf("%s has joined the chat\r\n", me.Nick),
			}

			// Handle messages
			for scanner.Scan() {
				me.SendMessage("> ")
				msg := strings.TrimSpace(scanner.Text())
				if msg == "" {
					continue
				}

				broadcastRequest <- broadcastMessageRequest{
					ClientSending: &me,
					// REVIEW: Formatting can be done by the engine (it knows the client), so this could just be the result of scanner.Text()
					Message:       fmt.Sprintf("[%s] %s\r\n", me.Nick, scanner.Text()),
				}
			}

			// Client disconnected
			disconnected <- me.Nick
		}()
	}
}