Defect #38145
closed
  
Unreachable branch in ApplicationHelper#format_object due to the use of the deprecated Fixnum class
 
        
        Added by Go MAEDA almost 3 years ago.
        Updated almost 3 years ago.
        
Category:
Code cleanup/refactoring
 
  
  
  
  Description
  
  There is an unreachable branch in ApplicationHelper#format_object. This is because a deprecated class name `Fixnum` is given to the `when` keyword.
	Reference:
Unify Fixnum and Bignum into Integer
https://bugs.ruby-lang.org/issues/12005
	While the branch is unreachable, fortunately, the result of the code below `when 'Fixnum'` is the same as the result of the code for the unknown class below `else`, so no problem is occurring for now.
	The following patch fixes the issue.
Index: app/helpers/application_helper.rb
===================================================================
--- app/helpers/application_helper.rb    (revision 22022)
+++ app/helpers/application_helper.rb    (working copy)
@@ -263,7 +263,7 @@
       format_time(object)
     when 'Date'
       format_date(object)
-    when 'Fixnum'
+    when 'Integer'
       object.to_s
     when 'Float'
       sprintf "%.2f", object
</diff>
   
 
  
  Files
  
 
  
  
    
    
    
    I think the problem (or "code smell") is that the case argument is object.class.name and not object. If the code was like this, using Fixnum would be fine because Fixnum equals Integer:
case object
when Array
  formatted_objects = object.map {|o| format_object(o, html)}
  html ? safe_join(formatted_objects, ', ') : formatted_objects.join(', ')
when Time
  format_time(object)
when Date
  format_date(object)
when Fixnum
  object.to_s
when Float
  sprintf "%.2f", object
 
   
  
  
    
    
    
    
    Thomas Löber wrote:
	I think the problem (or "code smell") is that the case argument is object.class.name and not object. If the code was like this, using Fixnum would be fine because Fixnum equals Integer:
	Thank you for suggesting the improved code. But we should not use Fixnum because the constant was removed in Ruby 3.2.
https://github.com/ruby/ruby/blob/ruby_3_2/NEWS.md#removed-constants
	Attaching an updated patch.
 
   
  
  
    
    
    
    I agree with the use of Integer instead of Fixnum.
	However, I don't think the patch works, because the case expression is object.class, but it should be object.
	The case statement calls === on the expression(s) that come after when, with the case expression as a parameter.
>> object = 1
=> 1
>> Integer === object        # Integer.===(object)
=> true
>> Integer === object.class  # Integer.===(object.class)
=> false
 
   
  
  
    
    
    
    
    Thomas Löber wrote:
	However, I don't think the patch works, because the case expression is object.class, but it should be object.
	Thank you for pointing it out, I attached the wrong patch.
	I have updated the patch.
 
   
  
  
    
    
    
    
       - Target version changed from Candidate for next major release to 5.1.0
Setting the target version to 5.1.0.
 
   
  
  
    
    
    
    
       - Status changed from New to Closed
- Assignee set to Go MAEDA
- Resolution set to Fixed
 
   
  
 
  
  
  
 
Also available in:  Atom
  PDF