mrhappyasthma / Sega-Genesis-Checksum-Utility

A simple command line utility to verify (or correct) Sega Genesis ROM checksums.
17 stars 4 forks source link

Python 3 support #1

Closed Zubayer204 closed 3 years ago

Zubayer204 commented 3 years ago

I have edited the code a bit so that it is compatible with python 3

mrhappyasthma commented 3 years ago

I would prefer we revert most of the comments that you added. The majority of them are self-documented by the code.

https://en.wikipedia.org/wiki/Self-documenting_code

For instance:

   if header_checksum == computed_checksum: #if the computed checksum and retrived checksum are same then it is a valid file
      print '\nChecksums match. Horray!')

I don't think this comment is needed. It's clear that we are checking if two checksums are equal based on the first line . And then it has a print statement confirming the match. The comment doesn't provide any additional clarity.

The only one that I think adds more context than the code itself describes is:

console_name = memorybuffer[0x100:0x110].decode('utf-8') #get the 15bits console_name and decode it with unicode

But I think instead of leaving a comment we can make the code a bit more self-describing. Perhaps by using local variable constants.

Maybe we could do something like:

CONSOLE_NAME_OFFSET = 0x100
CONSOLE_NAME_MAX_SIZE = 15

console_name = memorybuffer[CONSOLE_NAME_OFFSET:(CONSOLE_NAME_OFFSET + CONSOLE_NAME_MAX_SIZE)].decode('utf-8')

This way the local variable constants have meaningful names to describe the magic numbers (the header offset of 0x100 and the max size of 15).

And the value we are decoding from the memory buffer is stored in console_name. So then the code itself becomes more clear (without the need for any comment) as to what we are reading from memory and storing in the console_name.

mrhappyasthma commented 3 years ago

Any update on this?

Zubayer204 commented 3 years ago

Any update on this?

Sorry, I am freelancer. I didn't get much time to work on this.

mrhappyasthma commented 3 years ago

No worries. Take your time. Just wanted to check that this was still on your radar.

mrhappyasthma commented 3 years ago

I'm also using python 3 now, so merging this in.

I forgot to squash it myself, so I did so using a rebase in https://github.com/mrhappyasthma/Sega-Genesis-Checksum-Utility/commit/1add81ee34f8dabd8abe4f5080c5aa1553e1b8c9