2
\$\begingroup\$

I've created a program that will search our work server where Ghostscripts occur. How it accomplishes this is by first sshing to the server specified, running a bash command to search the server for Ghostscripts caused by our guy that uses them, most frequently. If it finds one, it e-mails somebody on my team. It will also e-mail if it doesn't find one.

I'm curious to know if there are other ways I can do this specific task. Better syntax, etc..

#!/local/usr/bin/ruby

require 'rubygems'
require '<mailgem>'
require 'etc'
require 'net/ssh'

class GhostScript

  attr_accessor :host, :usr, :pass

  def initialize(host, usr, pass)
    @host = host
    @usr  = usr
    @pass = pass
  end

  def script_search
    ssh = Net::SSH.start(host, usr, :password => pass)
    res = ssh.exec!('ps -u <user>|grep gs')
    if res == nil
      MailGem::Mail.new do |m|
        m.to      = '<email-address>'
        m.subject = 'GhostScript not found'
        m.body    = "GS not found on server: #{host}"
      end.send
      else
      alert(res)
    end
  end

  def alert(res)
    MailGem::Mail.new do |m|
      m.to      = '<email-address>'
      m.subject = 'GhostScript found'
      m.body    = "GS script found on server: #{host}",
                  "Results of search: #{res}"
    end.send
  end
end

check = GhostScript.new('<server>', Etc.getlogin, nil)
check.script_search
\$\endgroup\$
  • 1
    \$\begingroup\$ Isn't res always a (possibly empty) string? \$\endgroup\$ – Spike Jan 18 '16 at 13:16
  • \$\begingroup\$ @Spike I've had this conversation with someone once before, it runs a bash command and either returns nil, an empty string, or the command. For some reason nil always works for me, so I just use it because it's easy. \$\endgroup\$ – Bam Jan 18 '16 at 15:51
1
\$\begingroup\$

Looks pretty good, just some minor details:

  • if res == nil -> if !res

  • DRY the sending of the email by creating a new method: send_email(to, subject, body)

  • Subjective: I'd not include a blank lines when there is a change of indentation level (between class and attr_accesor)

\$\endgroup\$
  • \$\begingroup\$ I just think the space makes it look a little bit neater. I'm confused as to what this would do exactly !res, does it do the same thing as the == nil just shorter? Also, the method is a good idea, I'll work that in somehow. \$\endgroup\$ – Bam Jan 18 '16 at 16:22
  • 1
    \$\begingroup\$ 1) The rationale for not adding this blank line is consistency: why do you add there and not between def initialize(host, usr, pass) and @host = host, for exemple? 2) if !res translates to if res == nil || res == false, which is not exactly what you had, but probably it works all the same. It's more idiomatic (because it's more declarative). \$\endgroup\$ – tokland Jan 19 '16 at 8:39

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.