HiveBrain v1.2.0
Get Started
← Back to all entries
patternModerate

Is this IRC bot utility library Racket-y enough?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisenoughutilitybotirclibraryracket

Problem

To help myself learn Racket, I ported a simple JavaScript ircbot module I wrote for Node.js to Racket. The Racket version is built atop the Racket irc package, so the low-level code is handled by that. My module simply provides some utility functions to make it easier to implement IRC bots.

My code is as follows. I know it's rather long and probably fairly unreadable, and for that I apologize. I'm not looking for anything extremely specific, but I would like to know if I'm violating any major conventions of the language in any obvious ways.

```
#lang racket

(provide ircbot-connect
ircbot-say
ircbot-listen-action
ircbot-listen-chat
ircbot-listen-trigger
ircbot-listen-message
ircbot-listen-self
ircbot-listen-self-action
ircbot-listen-join
ircbot-listen-part
ircbot-listen-quit
ircbot-listen-nick
ircbot-listen)

;; ---------------------------------------------------------------------------------------------------

(require (for-syntax racket/syntax))

(require racket/async-channel)
(require irc)
(require srfi/13)

(struct ircbot-connection (server nick username realname channels triggers pmtrigger
action-handlers chat-handlers trigger-handlers message-handlers
self-handlers self-action-handlers join-handlers part-handlers
quit-handlers nick-handlers
connection))

(define (ircbot-connect #:server [server "localhost"]
#:port [port 6667]
#:nick [nick "racketircbot"]
#:username [username "racketircbot"]
#:realname [realname "racketircbot"]
#:channels [channels (list)]
#:triggers [triggers (list)]
#:pmtrigger [pmtrigger #f])
(define-values (connection ready-event)

Solution

Here are my comments after a brief reading:

  • First, if you intend to make this library fit for general use, add documentation in comments for the provided functions. See the Racket style guide for examples



  • Also, if you intend to make this a more general library, don't include default arguments (e.g. for ircbot-connect) unless they make sense for all users. Most users will connect to port 6667, but most users will not want to use the nick "racketircbot"



  • I agree with Greg Hendershott, I don't you don't need boxes in your struct - just use the #:mutable keyword



  • Break your code up into shorter functions where the logic is complex (e.g. the PRIVMSG handling should be a separate function). This is a good practice for all languages, not just Racket.



  • The use of curry seems awkward, but I haven't read the code in-depth enough to figure out if it's needed or not



  • Allowing the channels argument to ircbot-say be either a list or a single channel strikes me as strange - document this at the very least, but consider changing it to just take a list.



  • Your code is quite imperative in places, while Racket favors a more functional style. When you find yourself using set!, try to find a way to use just define instead, and only define each variable once.



I rewrote ircbot-say in a more functional form (although it's still not perfect):

(define (ircbot-say connection message [channels (ircbot-connection-channels connection)])
  (define channel-list (if (list? channels) channels (list channels)))
  (cond ([(string-contains message "/me ")
          (for ([channel channels])
            (irc-send-command (ircbot-connection-connection connection)
                              "PRIVMSG"
                              channel (format ":\u0001ACTION ~a\u0001" (substring message 4))))]
         [else
          (for ([channel channels])
            (irc-send-message (ircbot-connection-connection connection) channel message))])))


Otherwise, though, I think your code looks Racket-y. The match branches in ircbot-listen are exactly the kind of thing I envisioned when I wrote the irc package.

Code Snippets

(define (ircbot-say connection message [channels (ircbot-connection-channels connection)])
  (define channel-list (if (list? channels) channels (list channels)))
  (cond ([(string-contains message "/me ")
          (for ([channel channels])
            (irc-send-command (ircbot-connection-connection connection)
                              "PRIVMSG"
                              channel (format ":\u0001ACTION ~a\u0001" (substring message 4))))]
         [else
          (for ([channel channels])
            (irc-send-message (ircbot-connection-connection connection) channel message))])))

Context

StackExchange Code Review Q#62707, answer score: 16

Revisions (0)

No revisions yet.