New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make code slightly more Pythonic #16

Closed
wants to merge 19 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@msoucy
Contributor

msoucy commented May 23, 2014

This doesn't make EVERYTHING more pythonic, but it's a huge step forward - mostly taking advantage of Python builtins and syntax, with some formatting.

@msoucy msoucy changed the title from Refactor/pythonic to Make code slightly more Pythonic May 23, 2014

@msoucy

This comment has been minimized.

Show comment
Hide comment
@msoucy

msoucy May 23, 2014

Contributor

I apologize for the size of the request, I started hacking on it and time just flew by.

Contributor

msoucy commented May 23, 2014

I apologize for the size of the request, I started hacking on it and time just flew by.

@polaris-

This comment has been minimized.

Show comment
Hide comment
@polaris-

polaris- May 23, 2014

Owner

Woah. That's a lot of changes. Github won't let me automatically merge everything so it might take me a little bit to go through them all, but I'll try to get them all before tonight or tomorrow. Thanks a lot! Neither Toad King or I are really Python programmers so something like this was a long time coming.

Owner

polaris- commented May 23, 2014

Woah. That's a lot of changes. Github won't let me automatically merge everything so it might take me a little bit to go through them all, but I'll try to get them all before tonight or tomorrow. Thanks a lot! Neither Toad King or I are really Python programmers so something like this was a long time coming.

@msoucy

This comment has been minimized.

Show comment
Hide comment
@msoucy

msoucy May 23, 2014

Contributor

Yeah, sorry about that - if you want to wait until tonight I can attempt to merge master into my branch, to make it easier.

Yeah, I was a bit curious about the choice to use Python when your code style was (somewhat obviously, if you don't mind me saying) very C-like.

Most of these commits are related to doing things using builtin members and syntax instead of manually recreating them.

I've planned another change for gs_query that will greatly shrink the code down and make it nicer, assuming that what I believe about the query strings is correct (-separated key-value pairs, separated with , the "final" key doesn't have a value because it ends the list for that query). I also REALLY want to clean up the parser you're manually implementing in gamespy_backend_server, I feel that it can be made much simpler than using string indices directly.

Unfortunately, this pull request won't put everything into a more pythonic style, but it's a start.

Contributor

msoucy commented May 23, 2014

Yeah, sorry about that - if you want to wait until tonight I can attempt to merge master into my branch, to make it easier.

Yeah, I was a bit curious about the choice to use Python when your code style was (somewhat obviously, if you don't mind me saying) very C-like.

Most of these commits are related to doing things using builtin members and syntax instead of manually recreating them.

I've planned another change for gs_query that will greatly shrink the code down and make it nicer, assuming that what I believe about the query strings is correct (-separated key-value pairs, separated with , the "final" key doesn't have a value because it ends the list for that query). I also REALLY want to clean up the parser you're manually implementing in gamespy_backend_server, I feel that it can be made much simpler than using string indices directly.

Unfortunately, this pull request won't put everything into a more pythonic style, but it's a start.

@polaris-

This comment has been minimized.

Show comment
Hide comment
@polaris-

polaris- May 23, 2014

Owner

Alright, that sounds fine. I'll wait until you do that then.

I mostly programming in C, C++, and C#, so that influence is obviously going to come out. This was my first real Python project outside of a class I took in college. I originally chose Python so I could just quickly get something together without needing to always recompile, but I became too lazy to rewrite it in another language, so it stuck.

If you feel like you can improve those parts then by all means please do. If the functionality is the same then I would be more than happy to have cleaner code. If you wouldn't mind, please make sure to put some comments for what might be more the more trickier parts of the code so I will know what's going on and be able to learn from it.

Owner

polaris- commented May 23, 2014

Alright, that sounds fine. I'll wait until you do that then.

I mostly programming in C, C++, and C#, so that influence is obviously going to come out. This was my first real Python project outside of a class I took in college. I originally chose Python so I could just quickly get something together without needing to always recompile, but I became too lazy to rewrite it in another language, so it stuck.

If you feel like you can improve those parts then by all means please do. If the functionality is the same then I would be more than happy to have cleaner code. If you wouldn't mind, please make sure to put some comments for what might be more the more trickier parts of the code so I will know what's going on and be able to learn from it.

@msoucy

This comment has been minimized.

Show comment
Hide comment
@msoucy

msoucy May 24, 2014

Contributor

Due to some time constraints, I didn't have time to do the (admittedly sizeable) merge. I think it might be better to close this, and make a bunch of smaller pull requests over the next couple of days, each fixing one specific thing. That way there's less trouble merging all the fixes

Contributor

msoucy commented May 24, 2014

Due to some time constraints, I didn't have time to do the (admittedly sizeable) merge. I think it might be better to close this, and make a bunch of smaller pull requests over the next couple of days, each fixing one specific thing. That way there's less trouble merging all the fixes

@polaris-

This comment has been minimized.

Show comment
Hide comment
@polaris-

polaris- May 24, 2014

Owner

Alright, no problem. Take your time. I will close this pull request and wait for your smaller pull requests then. Thanks.

Owner

polaris- commented May 24, 2014

Alright, no problem. Take your time. I will close this pull request and wait for your smaller pull requests then. Thanks.

@polaris- polaris- closed this May 24, 2014

@msoucy msoucy deleted the msoucy:refactor/pythonic branch May 26, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment