Ytukyg

This is some code which prints a hexdump of a list of 16 bit numbers, at 16 bytes distance for each, after the "print data" comment:

#!/usr/bin/python3



# creating test data

data = 

for i in range(500):

    data = data + [ i & 0xff, i >> 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ]



# print data

c = 0

out = ""

for i in range(int(len(data) / 16)):

    if i == 512: break

    low = data[i * 16]

    high = data[i * 16 + 1]

    d = low | (high << 8)

    out = out + ("%04x " % d)

    if (i % 16) == 15: out = out + "n"

print(out)

It works, but I think I can write it simpler with "join" and maybe list comprehension? But shouldn't be a cryptic one-liner or something, I'm just looking for a more idiomatic Python solution. I want to print the first 512 numbers, but the data array can be shorter or longer.

Here are some comments on your code:

• I assume the structure of the input data is to be taken as it is: one long list where only the 1 out of 8 values contribute to the output.


• 
  c = 0 is defined but never used


• It is not recommended to use compound statements (see PEP8 - "Other Recommendations"): use a separate line for an if and the statement under condition.


• 
  / performs a true division (giving a float). To avoid switching between float and int, use integer division: //
  


• 
  if i == 512: break could me omitted if you would limit the range of the for loop immediately. Instead of len(data) use min(512*16, len(data))
  


• The multiplication i * 16 can be avoided if you use the step argument of range() so that i takes multiples of 16.


• Instead of "%04x " % d use the newer f-strings
  


• Instead of calculating d, you could just pass high and low to the string template and format each independently.


• Instead of if (i % 16) == 15: you could use a nested loop that deals with one output line


• Your code produces a blank at the end of each line. That seems unnecessary. With " ".join you would not have this extra blank.


• 
  out has a terminating n, but print also prints a newline as terminator (by default). With "n".joinyou would not have this extra newline

Here is how it could look:

# Set a maximum to the output

length = min(512*16, len(data))



# Format data

lines = 

for line in range(0, length, 256):

    items = 

    for i in range(line, min(line+256, length), 16):

        items.append(f"{data[i+1]:02x}{data[i]:02x}")

    lines.append(" ".join(items))

out = "n".join(lines)



# Output

print(out)

Here is how the above data formatting translates when using list comprehension. You can split the expression over multiple lines to improve readability:

# Format data

out = "n".join([

            " ".join([

                f"{data[i+1]:02x}{data[i]:02x}"

                for i in range(line, min(line + 256, length), 16)

            ])

            for line in range(0, length, 256)

        ])

The review by trinket has covered most, if not all, of the deficits in the code.

The one elephant left in the room is: does out really need to be built up as a single string, just to be printed? Could it not be printed line by line, or even per data value, and not waste time on the string concatenation operations?

Another question is where are these 16-bit values coming from? Are they really entering Python as a list of integers, or perhaps are they coming in as a bytes or bytearray memory buffer type structure? `Cause we can manipulate those to extract the data easier...

data = bytes(data)         # convert from list of ints into a byte array



mv = memoryview(data)      # convert to a memory view...

mv = mv.cast('H')          # treat every 2-bytes as a 16-bit value

mv = mv[:8*512]            # truncate to the first 512 groups of 8 values

mv = mv[::8]               # slice off the first of every 8 values



# Or as one statement...

mv = memoryview(data).cast('H')[:8*512:8]



# Print out each row of the hexdump:

for i in range(0, len(mv), 16):

   print(" ".join(f"{val:04x}" for val in mv[i:i+16]))



# Or construct the big out string, and print as one unit:

out = "n".join(" ".join(f"{val:04x}" for val in mv[i:i+16])

                for i in range(0, len(mv), 16)

print(out)

Note: The above assumes a little-endian architecture, so that when cast('H') is performed, the correct values are returned. If on a big-endian architecture, the code will need to be modified. See sys.byteorder.

I like the improvements by @trincot. Another improvement would be to use constants instead of numbers, makes it easier to understand and change instead of using magic numbers scattered in the code. And the f-syntax is nice, but I don't want to install Python 3.6 on my Debian machine, it still runs Python 3.5 and might break things. And I might need the word later for other things as well, and it makes the intention more clear that it is a 16 bit word, so I kept my extra word calculation. And no need to collect all lines in a string, this was just a side product of my solution to avoid multiple lines with resetting the current line and then printing it.

My final code:

max_lines = 32

words_per_line = 16

step = 16

line_length = words_per_line * step

length = min(max_lines * line_length, len(data))

for line in range(0, length, line_length):

    items = 

    for i in range(line, min(line + line_length, length), step):

        d = data[i] + (data[i + 1] << 8)

        items.append("%04x" % d)

    print(" ".join(items))

Will be used for my ADC4 project, which returns 16 bytes per sample, which is the reason for the big step.