Format of hexeditor

Question

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

Answer 1

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

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

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

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

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

5. 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.

6. 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.

7. 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.

8. 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.

9. 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.

10. 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!

Answer 2

This has been thoroughly answered already I'd say, but I'll have a go regardless...

You're loading 2 files (I don't know why, but I'll roll with it), and doing the same things with both. That calls for a method, or a loop, or both.

I'm thinking something like this:

%w(TESTFILE.txt TESTFILE2.txt).each.with_index do |path, i|
  data = File.read(path)
  hex = data.unpack('h*')
  encoding = data.encoding.name
  puts "%4s %25s %11s" % [i + 1, hex, encoding]
end

The contents of the loop can be moved to method, if you so choose. If you have to do more stuff than the above, it'd probably be a good idea.

Note, by the way, the syntactatic sugar for printf. As QPaysTaxes says, string interpolation is also an option (perhaps a better one), but format % args is a neat trick and lets you use puts directly instead of calling it afterward, or manually appending "\n" to the stuff you're printing.

If you want to use string interpolation instead, you can use String#rjust and String#ljust to pad the strings with spaces.

Answer 3

file = File.read('TESTFILE.txt')
file2 = File.read("TESTFILE2.txt")

It's not clear why you would need 2 files to read, nor what purpose each one is fulfilling: file and file2 are bad names that make the code require a comment to explain what the files are - better names would capture that meaning instead.

a = 1

Again, naming. This should really be line, or currentLine. And it could be defined/assigned closer to usage, too.

b = file.unpack('h*')
c = file.encoding.name
d = file2.unpack('h*')
e =file2.encoding.name

Inconsistent spacing, and names are becoming problematic at that point. If this is the foundation for a hex editor, you're building a very frail foundation.

unpacked1 = file1.unpack('h*')
unpacked2 = file2.unpack('h*')
encoding1 = file1.encoding.name
encoding2 = file2.encoding.name

Now, whenever one feels the need to have a numeric suffix to identifier names, it usually smells of a need for some data structure. I know nothing of ruby, but if it can define a struct or some object (eh, the tag wiki states it's an object-oriented language), you'd have one with some UnpackedHex and EncodingName members, perhaps called HexInfo - and then you could have an array of HexInfo objects to work with.