E9674d02c33d1d711a462026f40bbaf4

language is a column in a category table...i display what is actually stored in the table in a option select field, but to get the right html class for my css highlighting i need to rewrite what was chosen to print out to a html page in a text area as a class 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
def which_class?
  if self.language == "C/C++"
     self.language = "sh_cpp"
   else self.language = self.language
   end
   
   if self.language == "C#"
     self.language = "sh_csharp"
   else self.language = self.language
   end
   
   if self.language == "CSS"
     self.language = "sh_css"
   else self.language = self.language
   end
   
   if self.language == "Flex/ActionScript"
     self.language = "sh_flex"
   else self.language = self.language
   end
   
   if self.language == "Java"
     self.language = "sh_java"
   else self.language = self.language
   end
   
   if self.language == "Javascript"
     self.language = "sh_javascript"
   else self.language = self.language
   end
   
   if self.language == "Pascal"
     self.language = "sh_pascal"
   else self.language = self.language
   end
   if self.language == "Perl"
     self.language = "sh_perl"
   else self.language = self.language
   end
   
   if self.language == "PHP"
     self.language = "sh_php"
   else self.language = self.language
   end
   
   if self.language == "Python"
     self.language = "sh_python"
   else self.language = self.language
   end
   
   if self.language == "Ruby"
     self.language = "sh_ruby"
   else self.language = self.language
   end
   
   if self.language == "Shell"
     self.language = "sh_sh"
   else self.language = self.language
   end
   
   if self.language == "SQL"
     self.language = "sh_sql"
   else self.language = self.language
   end
   
   if self.language == "XML"
     self.language = "sh_xml"
   else self.language = self.language
   end
end

Refactorings

No refactoring yet !

5170ca260dbd2cdfd5a887a4dba7636f

Jeremy Weiskotten

December 10, 2007, December 10, 2007 00:21, permalink

5 ratings. Login to rate!

A couple of initial impressions:

1. A method that ends with a question mark, by convention, is expected to return a boolean. So the method which_class? should be which_class if only to avoid confusion.

2. The use of "else self.language = self.language" is unnecessary. It doesn't add anything, and just makes the code much noisier.

3. It seems odd to be setting self.language, but I don't have enough context to really understand how this method is used.

I'll post a more extensive refactoring in a bit.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
def which_class
  if self.language == "C/C++"
     self.language = "sh_cpp"
  elsif self.language == "C#"
     self.language = "sh_csharp"
  elsif self.language == "CSS"
     self.language = "sh_css"
  elsif self.language == "Flex/ActionScript"
     self.language = "sh_flex"
  elsif self.language == "Java"
     self.language = "sh_java"
  elsif self.language == "Javascript"
     self.language = "sh_javascript"
  elsif self.language == "Pascal"
     self.language = "sh_pascal"
  elsif self.language == "Perl"
     self.language = "sh_perl"
  elsif self.language == "PHP"
     self.language = "sh_php"
  elsif self.language == "Python"
     self.language = "sh_python"
  elsif self.language == "Ruby"
     self.language = "sh_ruby"
  elsif self.language == "Shell"
     self.language = "sh_sh"
  elsif self.language == "SQL"
     self.language = "sh_sql"
  elsif self.language == "XML"
     self.language = "sh_xml"
  end
end
E9674d02c33d1d711a462026f40bbaf4

Garrett

December 10, 2007, December 10, 2007 00:26, permalink

No rating. Login to rate!

thanks, im very new to programming as you can see lol, im integrating a syntax highlighter on my school project.

5170ca260dbd2cdfd5a887a4dba7636f

Jeremy Weiskotten

December 10, 2007, December 10, 2007 00:28, permalink

4 ratings. Login to rate!

Here's another solution. Please rate this refactoring if it helps!

1
2
3
4
5
6
7
8
9
  def which_class?    
    lang = self.language.downcase
    lang.gsub!('+', 'p')
    lang.sub!('c/', '')
    lang.sub!('/actionscript', '')
    lang.sub!('#', 'sharp')
    lang.sub!('shell', 'sh')
    self.language = "sh_#{lang}"
  end
5170ca260dbd2cdfd5a887a4dba7636f

Jeremy Weiskotten

December 10, 2007, December 10, 2007 00:31, permalink

3 ratings. Login to rate!

Here's another way to do it, using a simple lookup table:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
  def which_class?
    self.language = 'sh_' + 
    {
      "C/C++" => "cpp",
      "C#" => "csharp",
      "CSS" => "css",
      "Flex/ActionScript" => "flex",
      "Java" => "java",
      "Javascript" => "javascript",
      "Pascal" => "pascal",
      "Perl" => "perl",
      "PHP" => "php",
      "Python" => "python",
      "Ruby" => "ruby",
      "Shell" => "sh",
      "SQL" => "sql",
      "XML" => "xml",
    }[self.language]
  end
4d1c9dad17af98e55cb65b4efce27c42

Ben Burkert

December 10, 2007, December 10, 2007 00:32, permalink

4 ratings. Login to rate!

I think what you are trying to do is filter the class column in the database. I'd suggest using a map or a case statement instead of those ifs. Also, you can define a new setter for language that can handle the filtering.
Also naming the method which_class? is not kosher. This is a dangerous method, and does not return a boolean, so it should be called which_class!

1
2
3
4
5
6
7
8
9
10
LANGUAGE_MAP = {
  "C/C++" => "sh_cpp",
  "C#"    => "sh_csharp",
  #...
  "XML"   => "sh_xml"
}

def language=(value)
  write_attribute(:language, LANGUAGE_MAP[value])
end
4d1c9dad17af98e55cb65b4efce27c42

Ben Burkert

December 10, 2007, December 10, 2007 00:36, permalink

4 ratings. Login to rate!

Here it is with a case statement.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
def language=(value)
  write_attribute(:language, language_filter(value))
end

def language_filter(value)
  case value
  when "C/C++"
    "sh_cpp"
  when "C#"
    "sh_csharp"
  #...
  else
    value
  end
end
1945ab4cdb87eaf5a5c906fa884c29f1

Scott Robinson

December 10, 2007, December 10, 2007 00:38, permalink

5 ratings. Login to rate!

The regular expression version is cute. But, it isn't very future proof or robust to odd inputs.

1
2
3
4
5
6
7
8
9
10
11
def which_class
  langs = {"C/C++" => "cpp", "C#" => "csharp",
	   "CSS" => "css", "Flex/ActionScript" => "flex",
	   "Java" => "java", "Javascript" => "javascript",
	   "Pascal" => "pascal", "Perl" => "perl",
	   "PHP" => "php", "Python" => "python",
	   "Ruby" => "ruby", "Shell" => "sh",
	   "SQL" => "sql", "XML" => "xml"}

  self.language = 'sh_' + (langs[self.language] or 'unknown')
end
90ec841f866ac35c4ce90ee3cfffbdc3

Vime

December 10, 2007, December 10, 2007 13:11, permalink

2 ratings. Login to rate!

You can use the 'default' value for unmatched hashes

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
def which_class
  langs = { "C/C++"             => "sh_cpp", 
            "C#"                => "sh_csharp",
	    "CSS"               => "sh_css", 
            "Flex/ActionScript" => "sh_flex",
	    "Java"              => "sh_java", 
            "Javascript"        => "sh_javascript",
	    "Pascal"            => "sh_pascal", 
            "Perl"              => "sh_perl",
	    "PHP"               => "sh_php", 
            "Python"            => "sh_python",
	    "Ruby"              => "sh_ruby", 
            "Shell"             => "sh_sh",
	    "SQL"               => "sh_sql", 
            "XML"               => "sh_xml" }

   langs.default = "unknown"

  self.language = langs[self.language]
end

Your refactoring





Format Copy from initial code

or Cancel