retrieving input from an array

Question

I have my system class

class System
    @os
    @base
    @number
    def setOs(newos)
        @os = newos
    end
    def getOs()
        return @os
    end
    def setBase(newbase)
        @base = newbase
    end
    def getBase()
        return @base
    end
    def setSystemNumber(newnumber)
        @number = newnumber
    end
    def getSystemNumber()
        return @number
    end
end

and I have my method to add multiple instances of system to an array

def readXMLSystems

    doc = Nokogiri::XML(File.open("/Users/lewisardern/Documents/Security-Simulator/lib/xml/boxesconfig.xml"))
    # puts doc
    systeminstance = ""
    systemArray = []

    doc.search('//systems/system').each do |system|
        systeminstance = System. new 
        number = system.at('@number').text
        systeminstance.setSystemNumber number
        os = system.at('@os').text
        systeminstance.setOs os
        base = system.at('@basebox').text
        systeminstance.setBase base

        systemArray.insert(systeminstance)
    end

    return systemArray
end

and i call that method by

sys = readXMLSystems
puts sys

how come i can't read the XML code? if i write inside the loop it spits out

puts systeminstance.getSystemNumber #retrieves input

1
2
3

I want to be able to have multiple systems inside this array but it doesn't seem to be returning... where am i going wrong?

Rafa Paez · Accepted Answer · 2014-03-07 01:04:47Z

2

Use

systemArray.push(systeminstance) # or systemArray << systeminstance

instead of

systemArray.insert(systeminstance)

Also please, take a look at the Ruby style guide.

Edit: Just to show how the same can be achieved in a Ruby style way (not tested). As you can see I removed more than the 80% of the code. A code like this is more OOP, functional, clean and will lend you to have much less bugs.

System = Struct.new(:os, :base, :number)

def read_systems_xml
    filename = "/Users/lewisardern/Documents/Security-Simulator/lib/xml/boxesconfig.xml"
    doc = Nokogiri::XML(File.open(filename))

    doc.search('//systems/system').map do |system|
        System.new(
          system.at('@os').text, 
          system.at('@basebox').text, 
          system.at('@number').text)
    end
end

edited Mar 7, 2014 at 1:04

answered Mar 7, 2014 at 0:48

Rafa Paez

4,87023 silver badges35 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Lewis Over a year ago

that worked, jesus christ seems i have a lot to learn. - using your method instead much cleaner.

Rafa Paez Over a year ago

Yes, and you can do it! PS.: A former C# developer.

miken32 · Accepted Answer · 2022-10-18 15:50:52Z

Well for starters, this is pretty non-idiomatic Ruby

def setOs(newos)
    @os = newos
end
def getOs()
    return @os
end

You don't have to hand-write getters/setters. And explicit return calls are infrequently needed.

This should be written using the attr_accessor macro

class System
  attr_accessor :os
end

If you want to write it by hand

class System

  def os
    @os
  end

  def os= os
    @os = os
  end

end

Don't bring idioms from your previous teachings to new languages you're learning.

Anyway, this is broken. There's no such method as Array#insert

systemArray.insert(systeminstance)

Correct this to

systemArray << systeminstance

Other griefs

Ruby is dynamically typed, so don't add type names to your variables: systemArray or systeminstance should be called systems and system respectively.
Ruby doesn't use camelCase: even if systemArray was an appropriate variable name, it would be system_array. Same goes for method names. setSystemName should be system_name=

Collectives™ on Stack Overflow

retrieving input from an array

2 Answers 2

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related