Skip to content

Conversation

@riking
Copy link

@riking riking commented Oct 11, 2012

String addition in python creates a new string object every time it happens. As a result, it is inefficient to add any more than two strings together, instead, the format operator should be used. I have converted all the send() calls in pug.py to use string formatting instead of addition, and also added three convenience functions to reduce string constant redundancy.

I did notice many more inefficiencies while looking through the code, and will get to those shortly.

All send() calls have been changed to one of sendNotice(msg, user),
sendMsg(msg, user), or sendChannel(msg). I recommend additional
refactoring in many parts of the program.
@jdecaron
Copy link
Owner

i will have a look at this, but so far the diff doesn't work since all the lines seems to be different. once i get the diff i will see if i can commit those changes.

@riking
Copy link
Author

riking commented Oct 14, 2012

Shouldn't you be able to push your state to github then hit "implement pull request" then pull on your end? That's how it's supposed to work.

@jdecaron
Copy link
Owner

indeed but before applying the change I want to review it but right now when I do the diff it doesn't show me which lines has been changed because every single of them changed. it's making the merge harder for me. I have been a little bit busy lately so I would gladly pull your changes if you would make sure the diff is only giving me the line that got changed.

@riking
Copy link
Author

riking commented Oct 16, 2012

Oh, I just noticed that the line difference count is huge. I'll go back over my changes, something must be wrong.

@riking
Copy link
Author

riking commented Oct 17, 2012

The diff appears to be fixed. I edited the file on Windows and forgot to setup the line ending configs. I fixed it on Linux :)

(Hit "files changed", https://github.com/550/tf2ib/pull/28/files , to see what I really changed.)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants