patternrubyMinor
Checking host status for clients on server
Viewed 0 times
clientscheckingstatushostforserver
Problem
I run a small server, and to check whether clients are being hosted on my server, I wrote this. I'm new to object oriented programming. How stratified should my programs really be? Do I want to have every possible variable and piece in their own function? Also, how am I doing in general? Any recommendations?
class Domain_check
def vhost_grab
return full_domain_path = `ls /etc/httpd/conf.d/vhost_* | grep -v 000_defaults.conf`.chomp.split(' ')
end
def vhost_stripper
prefix = Domain_check.new.vhost_grab
vhost_stripped = []
prefix.each_index do |x|
vhost_stripped[x] = `echo '#{prefix[x]}' | awk -F'vhost_' '{print $2}' | awk -F'.conf' '{print $1}'`
end
return vhost_stripped
end
def vhost_display
puts "\n%s %40s %43s" %["Domain name", "IP Address Listed", "IP Address Currently in Use"]
final_vhost = vhost_stripper
final_vhost.each_index { |x|
padding = 50
print final_vhost[x].strip
padding = padding.to_i - final_vhost[x].length
print "%0#{padding}s" %[`grep '' #{Domain_check.new.vhost_grab[x]} | awk -F'<VirtualHost' '{print $2}'|awk -F':' '{print $1}'`.strip.to_s]
puts "%40s" %[`dig #{final_vhost[x].strip} +short`]
}
end
end
d1 = Domain_check.new
d1.vhost_displaySolution
Some observations:
-
Regarding the "stratification". It's hard to give practical advice, this is something that comes from experience and personal taste. Some basics to start with: a) write classes/modules with low coupling to achieve real modularization, b) write fairly short methods than do only one thing. c) Keep model-view separations. d) Don't use global variables.
-
-
Don't write an explicit
-
Don't assign a variable on this last expression, why would be it different from the method name itself?
-
Don't call to external commands (ls, grep, awk, ...), Ruby is more than able to perform those tasks, check the standard library.
-
About this:
-
You are using a
-
Init empty + iterate with
-
Having only a C background your code has a serious problem, it's very, very imperative. Functional programming allows far better abstraction and clarity, check this wiki page I maintain.
-
If every method is prefixed with
-
Regarding the "stratification". It's hard to give practical advice, this is something that comes from experience and personal taste. Some basics to start with: a) write classes/modules with low coupling to achieve real modularization, b) write fairly short methods than do only one thing. c) Keep model-view separations. d) Don't use global variables.
-
class Domain_check: that's against Ruby practices, a class or module names are named CamelCase: class DomainCheck.-
Don't write an explicit
return on the last line of a method/block, it's non idiomatic. -
Don't assign a variable on this last expression, why would be it different from the method name itself?
-
Don't call to external commands (ls, grep, awk, ...), Ruby is more than able to perform those tasks, check the standard library.
-
About this:
prefix = DomainCheck.new.vhost_grab. You are already in the instance, just prefix = vhost_grab.-
You are using a
class just nominaly, you don't use any of its facilities. You can convert all these methods to classmethods if you store nothing in the instances.-
Init empty + iterate with
each + push is an anti-pattern in Ruby (and in any language with decent functional capabilities, for that matter). Use Enumerable#map. Also, you use each_index to iterate in a C fashion, use each (or better, map, select, inject, as required, read the Enumerable documentation from start to finish).-
Having only a C background your code has a serious problem, it's very, very imperative. Functional programming allows far better abstraction and clarity, check this wiki page I maintain.
-
If every method is prefixed with
vhost_ there's no point in prefixing anything. Besides, if all the methods start with vhost_, the class should probably be called VirtualHostChecker instead (@Flambino).Context
StackExchange Code Review Q#18796, answer score: 7
Revisions (0)
No revisions yet.