Code Cleanup

[chisleu]

pep8 fixes
removed unused variables / params
use new style class declarations where appropriate
fix typos
chain comparisons for readability
make methods static as appropriate
removed unused imports
left Exception catch alls for now

What current issue(s) from Github does this address?
none
What problem does this PR solve?
code cleanup
How did you solve this problem?
cleaned code
How did you make sure your solution works?
smoke testing and ran unittests
Are there any special changes in the code that we should be aware of?
Nothing very special.
Is there anything else we should know?
In the late 90's, I got kicked out of college for violation of the acceptable use policy.

[chisleu]

Apparently I have to add before I commit ;)

[coveralls]

Coverage decreased (-0.06%) to 81.057% when pulling 0be7976 on chisleu:development into ea504e7 on CityOfZion:development.

[localhuman]

Lot to go through, taking a look at it now!

[chisleu]

It's really small stuff, but I like to get all the errors that I can out of the code so my linter doesn't freak out on me. The "raise Exception" were all over the place and hard for me to tackle immediately so I left them. I have a few performance improvements I want to get to before I'll have my head wrapped around the exception handling.

[chisleu]

The one thing I worried about was the ToName function being renamed. I'm not sure what other modules might use this, but perhaps it is unneeded. The code already deviate heavily from pep8 in order to keep API compatibility with the .NET functions I'm guessing.

I actually prefer kneeling camel and camel to underscores, but I try to make my python look pythonic.

[localhuman]

Any advantages to making these static methods? I'm ok with that, but wouldn't the following line no longer work?
https://github.com/chisleu/neo-boa/blob/0be79767af61198749c012a5d46354f4f3f7f478/boa/code/vmtoken.py#L701

[chisleu]

You can still call statics from the module with self, but you can also call it externally.

The real benefit comes in memory performance increases. It doesn't add functionality yet, but some of those static functions allow external checks against the sets, which can be nice.

Python 3.5.4 (v3.5.4:3f56838976, Aug  7 2017, 12:56:33)
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> class Test(object):
...     @staticmethod
...     def hi():
...         print("hi")
...     def yo(self):
...         self.hi()
...
>>> Test.hi()
hi
>>> x=Test()
>>> x.yo()
hi
>>>

https://julien.danjou.info/blog/2013/guide-python-static-class-abstract-methods

[localhuman]

I know how static methods work, I just normally wouldn't use them from inside a class instance for a situation where I wouldn't want outside objects calling the methods.

[chisleu]

Sorry, I was confused about your question. They look like functions that would be useful to reference outside.

To be sure, if I was going for performance I would probably migrate the lists of functions to a module static and eliminate the functions entirely, but I was trying to address high spots in linting before diving into the other changes. That way if I mess up something with linting or typing, I can tell by the errors instead of constantly checking linting errors.

[localhuman]

That makes sense. I would say in general, however, that performance is a minimal consideration for me at the moment. Not that it shouldn't be considered, but usability and I guess kind of readability are more crucial-

[chisleu]

I was talking about the output AVM code's performance. There is a ton of unnecessary calls and since you get charged per op code, it would mean a big deal at scale, especially if the 10 GAS free gets modified at some point.

[chisleu]

There are a lot of undocumented functions that I can document as I read through things and understand them. Also I'm with you on usability. It's not terrible, but there are some major gains to be found there. The testing needs a lot of work, but it's at least got a great start. I can't really do performance increases without adding testing so I know I didn't break the functionality.

[localhuman]

Ah, yes VM performance is the most important... I think we're on the same page :)