Hi - I'm very new here (1st post). I read the introductory docs, and looked at the categories, but I didn't see a specific place for posting code for review/critique. My apologies if this is not the appropriate place to do that...

Here is some code I am hoping to get reviewed/critiqued by people who know Ruby. I don't know squat about Ruby, but I am pretty familiar with modern OO lang's (C++, Java, C#, PHP, etc.), and I wrote this after reading a few tutorials.

It is an implementation of an Artificial System Clock (ASC). The idea is that you will use the getSystemDate method of this class everywhere in your app. that gets the date/time directly. Then you can change the date-time context for the whole app. for testing purposes. When in "channel" mode, it will go fetch the date-time value from TimeChannels.com, so that you can integration-test with multiple interfacing apps. all on the same shared ASC value.

Here's the TimeChannels class I wrote in Ruby:

require 'date'
require 'net/http'
require 'open-uri'

class TimeChannels
  @@tcURL = "http://www.timechannels.com/GetChannel"
  @@tcURLms = "http://www.timechannels.com/GetChannelMS"
  @@tcAscState = "TimeChannels.state.Production"
  @@tcChannel = "0"
  @@tcAscDateTime = nil
  @@tcServiceURL = "http://www.timechannels.com/GetChannel"
  @@tcMillisecondMode = "false"


  def TimeChannels.setToProductionState
    @@tcAscState = "TimeChannels.state.Production"
  end


  def TimeChannels.setToStaticTestState(aDateTimeValue)
    @@tcAscState = "TimeChannels.state.TestDate"
    @@tcAscDateTime = aDateTimeValue
  end


  def TimeChannels.setToTestState(aChannel)
    @@tcAscState = "TimeChannels.state.TestChannel"
    @@tcChannel = aChannel
  end


  def TimeChannels.setToMillisecondMode()
    @@tcMillisecondMode = "true"
    @@tcServiceURL = @@tcURLms
  end


  def TimeChannels.setToStandardMode
    @@tcMillisecondMode = "false"
    @@tcServiceURL = @@tcURL
  end


  def TimeChannels.getSystemDate
    if @@tcAscState == "TimeChannels.state.Production"
      return DateTime.now
    end

    if @@tcAscState == "TimeChannels.state.TestDate"
      return @@tcAscDateTime
    end

    if @@tcAscState == "TimeChannels.state.TestChannel"
      return getChannelFromTimeChannelsService(@@tcChannel)
    end

    DateTime.now
  end


  def TimeChannels.getHTTPContent(url)
    #res = Net::HTTP.get_response(URI.parse(url))
    #data = res.body

    data = open(url).read
    data
  end


  def TimeChannels.getChannelFromTimeChannelsService(aChannel)
    url = @@tcServiceURL + "?channel=" + aChannel
    result = getHTTPContent(url)

    if (result.nil?) 
      return Time.now
    end

    if (@@tcMillisecondMode == "true")
      retVal = result
      #retVal = DateTime.parse(result)
    else
      retVal = DateTime.parse(result)
    end
      
    retVal
  end

end

Many thanks in advance to all who reply...

John

I have a suggestion, I think your code need a exception handler when your internet connection is lost.

Though I'm no Ruby expert, here are a few suggestions:

if (result.nil?)
  return Time.now
end

can also be written as:

return Time.now if result.nil?

= Instead of using @@tcMillisecondMode = "false" , why not just use @@tcMillisecondMode = false ?

= Your ASC state can be better represented using Ruby Symbols.

= In Ruby in absence of a return statement, the value of the last expression evaluated is returned hence the method:

def TimeChannels.getHTTPContent(url)
    data = open(url).read
    data
end

can be replaced by:

def TimeChannels.getHTTPContent(url)
    open(url).read
end

though as already mentioned, handling exceptions in case establishment of a connection fails should come first.

= The Ruby way of naming variables and methods is *not* to use camelCase but use under_scores; though it's a matter of taste. However it pays to follow and use the conventions used by the community in general to facilitate easier understanding of others' code and vice-versa.

In case you require a more in-depth review, feel free to post your code on the official ruby forums and you'll surely get more feedback.

HTH
-sos

This article has been dead for over six months. Start a new discussion instead.