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

Format of hexeditor

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

Problem

Is there a cleaner way to write this code? It's the start of a hexeditor I'm creating.

I'm aware that the variables aren't descriptive, this is just a test run for when I imbed it into a File Converter program.

def hexeditor
    file = File.read('TESTFILE.txt')
    file2 = File.read("TESTFILE2.txt")
    a = 1
    b = file.unpack('h*')
    c = file.encoding.name
    d = file2.unpack('h*')
    e =file2.encoding.name
    printf("%6s %20s %35s", "LINE:", "HEX:", "ENCODING:\n")
    printf("%4s %25s %11s", a, b, c)
    puts " "
    printf("%4s %25s %11s", a.next, d, e)
    puts " "
end

hexeditor


Currently, the output looks like this:

LINE: HEX: ENCODING:
1 ["4584943502943502140245543545026494c454a0"] US-ASCII
2 ["4584943502943502140245543545026494c454a0"] US-ASCII

Solution

I've just got a few little tips for this tiny little program:

-
Use consistent spacing. You mostly have a = b, but in one place you have a =b. Probably just a typo, but worth mentioning.

-
In Ruby, you have string interpolation, which (in my experience) tends to be preferred to printf.

-
puts " " is equivalent to puts, at least the way you're using it.

-
a, b, c, d, and e are uninformative names. Use names that are what they represent -- for example, e would be file2encoding.

-
Use arrays, instead of multiple variables. Make use of map to turn a list of paths into a list of arrays, which contain the encoding, content, etc. of a file. Maybe make a class to represent the files -- oh, wait, there already is one, called File, which you already use. Then again, it doesn't do much of what you need.

-
Prefer single-quotes when you don't have interpolation, and double-quotes when you do. Or just always use double-quotes. Don't flip-flop back and forth for no reason.

-
Your printing-out-bit will not print out each line. It will print out an array, calling it the first line of the first file, which contains every line in the file. That's what the ["..."] around the output means. You want to loop through the array there and, for each one, print out the line number, line, etc. etc.

-
This isn't Python; you don't need to wrap your main code in a function, then call it at the end. Just write your code. If you want to copy/paste into another file and you're worried about indenting, stop using tools that weren't made for coding and get a decent IDE.

-
You're using File::read, which isn't a real method. The method you're really using is IO::read. Please try to use the right class when you're calling methods.

-
The "encoding of the file" that you're outputting is really the encoding of the string that's returned by IO::read. As such, it'll change depending on the system, not depending on the file. This is what the docs are for.

With my advice, here's what your code looks like (untested as of yet, but should work):

files = ["TESTFILE.txt", "TESTFILE2.txt"] # Could also use %w[], but this is safer.
files.each do |file|
  puts "Filename: #{file}"
  puts "Data:"
  IO.foreach(file).with_index do |line, num| # For each line in `file`
    puts "#{num+1}:\t#{line.unpack('h*')}"
  end
end


Much simpler! Notice the .with_index after .foreach -- that's to keep track of the index for you, so you don't have to!

Code Snippets

files = ["TESTFILE.txt", "TESTFILE2.txt"] # Could also use %w[], but this is safer.
files.each do |file|
  puts "Filename: #{file}"
  puts "Data:"
  IO.foreach(file).with_index do |line, num| # For each line in `file`
    puts "#{num+1}:\t#{line.unpack('h*')}"
  end
end

Context

StackExchange Code Review Q#113184, answer score: 10

Revisions (0)

No revisions yet.