HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Stack class in Ruby to calculate maximum number, get the average, pop, push

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numbermaximumthestackaveragegetpushcalculaterubyclass

Problem

Is the class optimized enough to accept 1 million items in the stack?

class Stack

attr_accessor :stacks

def initialize() 
    @stacks = []
end

def max ()
    #first we could Think by 
    #sort the stack 
    #then take the last element 
    #but I think it is not efficent 
    if @stacks.size == 0
        return puts"Error no input"
    elsif @stacks.size == 1
        @stacks.first           
    else
    return @stacks.max
    end     
end

def push(value)
    @stacks.push(value)
end

def pop()
    if @stacks.size == 0
        return puts"Error no input" 
    else
        @stacks.pop
    end
    return @stacks
end

end

class Extras < Stack

def average()
    if @stacks.size == 0
        return puts"Error no input" 
    elsif @stacks.size == 1
        return @stacks.first
    else    
        #used inject instead of loop to get more familiar with ruby 
        sum = @stacks.inject(0, :+)
        lenght = @stacks.size
        averages = sum / lenght
        return averages
    end
end
end

users = Extras.new
puts users.push(1)
puts users.push(5)
puts users.push(3)
users.pop
puts users.push(10)
puts users.average

Solution

Your code here is really just wrapping the Array class from the ruby library. As such, your performance will be exactly the same as just using Array itself. The array can hold as many items as you can hold in RAM and the performance will be determined by whatever machine you are running on.

The ruby array class is written in C and performs fairly well. You ask about optimization but you don't given any criteria, so I can't really speculate beyond that. If you need something that is extremely fast or needs to scale across millions or billions of items, you may want to look at a different language or even a database.

Looking at your code, you have several instances of the pattern if-elsif-else. The last two are redundant and can be combined. For example, the max of an array with one item will be the one item, you don't have to break it out. Likewise for doing the average; you can average a list with one item and you will still get the correct answer

def max ()
    # returning 'puts' is _very_ unusual.  It would be better to return nil, which Array#max would do anyway
    return puts "Error no input" if @stacks.empty?   
    @stacks.max     # the last expression is returned automatically
end

def average()
    return puts"Error no input" if @stacks.empty?

   sum = @stacks.inject(0, :+)
   sum / @stacks.size # note that if you sum an array of integers this way, the final average will also be an integer unless you convert something to a float first
end

Code Snippets

def max ()
    # returning 'puts' is _very_ unusual.  It would be better to return nil, which Array#max would do anyway
    return puts "Error no input" if @stacks.empty?   
    @stacks.max     # the last expression is returned automatically
end

def average()
    return puts"Error no input" if @stacks.empty?

   sum = @stacks.inject(0, :+)
   sum / @stacks.size # note that if you sum an array of integers this way, the final average will also be an integer unless you convert something to a float first
end

Context

StackExchange Code Review Q#150398, answer score: 2

Revisions (0)

No revisions yet.