Artificial System Clock Code review critique

Reply

Join Date: Jun 2009
Posts: 1
Reputation: JohnBoy2 is an unknown quantity at this point 
Solved Threads: 0
JohnBoy2 JohnBoy2 is offline Offline
Newbie Poster

Artificial System Clock Code review critique

 
0
  #1
Jun 26th, 2009
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:

  1. require 'date'
  2. require 'net/http'
  3. require 'open-uri'
  4.  
  5. class TimeChannels
  6. @@tcURL = "http://www.timechannels.com/GetChannel"
  7. @@tcURLms = "http://www.timechannels.com/GetChannelMS"
  8. @@tcAscState = "TimeChannels.state.Production"
  9. @@tcChannel = "0"
  10. @@tcAscDateTime = nil
  11. @@tcServiceURL = "http://www.timechannels.com/GetChannel"
  12. @@tcMillisecondMode = "false"
  13.  
  14.  
  15. def TimeChannels.setToProductionState
  16. @@tcAscState = "TimeChannels.state.Production"
  17. end
  18.  
  19.  
  20. def TimeChannels.setToStaticTestState(aDateTimeValue)
  21. @@tcAscState = "TimeChannels.state.TestDate"
  22. @@tcAscDateTime = aDateTimeValue
  23. end
  24.  
  25.  
  26. def TimeChannels.setToTestState(aChannel)
  27. @@tcAscState = "TimeChannels.state.TestChannel"
  28. @@tcChannel = aChannel
  29. end
  30.  
  31.  
  32. def TimeChannels.setToMillisecondMode()
  33. @@tcMillisecondMode = "true"
  34. @@tcServiceURL = @@tcURLms
  35. end
  36.  
  37.  
  38. def TimeChannels.setToStandardMode
  39. @@tcMillisecondMode = "false"
  40. @@tcServiceURL = @@tcURL
  41. end
  42.  
  43.  
  44. def TimeChannels.getSystemDate
  45. if @@tcAscState == "TimeChannels.state.Production"
  46. return DateTime.now
  47. end
  48.  
  49. if @@tcAscState == "TimeChannels.state.TestDate"
  50. return @@tcAscDateTime
  51. end
  52.  
  53. if @@tcAscState == "TimeChannels.state.TestChannel"
  54. return getChannelFromTimeChannelsService(@@tcChannel)
  55. end
  56.  
  57. DateTime.now
  58. end
  59.  
  60.  
  61. def TimeChannels.getHTTPContent(url)
  62. #res = Net::HTTP.get_response(URI.parse(url))
  63. #data = res.body
  64.  
  65. data = open(url).read
  66. data
  67. end
  68.  
  69.  
  70. def TimeChannels.getChannelFromTimeChannelsService(aChannel)
  71. url = @@tcServiceURL + "?channel=" + aChannel
  72. result = getHTTPContent(url)
  73.  
  74. if (result.nil?)
  75. return Time.now
  76. end
  77.  
  78. if (@@tcMillisecondMode == "true")
  79. retVal = result
  80. #retVal = DateTime.parse(result)
  81. else
  82. retVal = DateTime.parse(result)
  83. end
  84.  
  85. retVal
  86. end
  87.  
  88. end

Many thanks in advance to all who reply...

John
Reply With Quote Quick reply to this message  
Join Date: Jul 2009
Posts: 8
Reputation: slac3dork is an unknown quantity at this point 
Solved Threads: 1
slac3dork's Avatar
slac3dork slac3dork is offline Offline
Newbie Poster

Re: Artificial System Clock Code review critique

 
0
  #2
Jul 29th, 2009
I have a suggestion, I think your code need a exception handler when your internet connection is lost.
Code Snippet Web log:
http://snippet.c0de.me
Reply With Quote Quick reply to this message  
Join Date: Jun 2006
Posts: 7,618
Reputation: ~s.o.s~ has much to be proud of ~s.o.s~ has much to be proud of ~s.o.s~ has much to be proud of ~s.o.s~ has much to be proud of ~s.o.s~ has much to be proud of ~s.o.s~ has much to be proud of ~s.o.s~ has much to be proud of ~s.o.s~ has much to be proud of ~s.o.s~ has much to be proud of 
Solved Threads: 467
Super Moderator
Featured Poster
~s.o.s~'s Avatar
~s.o.s~ ~s.o.s~ is offline Offline
Failure as a human

Re: Artificial System Clock Code review critique

 
0
  #3
Aug 8th, 2009
Though I'm no Ruby expert, here are a few suggestions:
  1. if (result.nil?)
  2. return Time.now
  3. end
can also be written as:
  1. 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:
  1. def TimeChannels.getHTTPContent(url)
  2. data = open(url).read
  3. data
  4. end
can be replaced by:
  1. def TimeChannels.getHTTPContent(url)
  2. open(url).read
  3. 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
Last edited by ~s.o.s~; Aug 8th, 2009 at 12:22 pm.
I don't accept change; I don't deserve to live.
Reply With Quote Quick reply to this message  
Reply

This thread is more than three months old.
Perhaps start a new thread instead?
Message:


Thread Tools Search this Thread



About Us | Contact Us | Advertise | DaniWeb | Acceptable Use Policy | RSS Feed

©2003 - 2009 DaniWeb® LLC