Simplify random_salt_generator
#22
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ekultek
reviewed
Jun 29, 2017
The speed difference is negligible, only the first test is slower than the original (the second test is essentially tied). These tests are also not checking different lengths (feel free to write those on your own though). Having simple and maintainable code should be more important. The original version repeats work and is harder to reason about.
In[3]: %timeit original(True, False)
10000 loops, best of 3: 19.2 µs per loop
In[4]: %timeit new_version(True, False)
The slowest run took 5.45 times longer than the fastest. This could mean that an intermediate result is being cached.
10000 loops, best of 3: 25 µs per loop
In[5]: %timeit original(True, True)
10000 loops, best of 3: 24.1 µs per loop
In[6]: %timeit new_version(True, True)
10000 loops, best of 3: 25.1 µs per loop
In[7]: %timeit original(False, True)
The slowest run took 4.19 times longer than the fastest. This could mean that an intermediate result is being cached.
10000 loops, best of 3: 35 µs per loop
In[8]: %timeit new_version(False, True)
10000 loops, best of 3: 25.3 µs per loop
In[9]: %timeit original(False, False)
10000 loops, best of 3: 33.3 µs per loop
In[10]: %timeit new_version(False, False)
The slowest run took 4.24 times longer than the fastest. This could mean that an intermediate result is being cached.
10000 loops, best of 3: 25.5 µs per loop |
|
Your version with ascii letters and numbers: |
|
My version with ascii letters and numbers: |
|
Your version random bytes: |
|
My version random bytes: |
|
Is this a joke? How am I supposed to read 1000+ lines of code hahaha |
|
So as you can see, your version is faster, so this is accepted. |
|
Wait one second though |
|
Did you want me to make that change though (to handle if somehow a string like try:
salt_length = int(length)
except (TypeError, ValueError):
salt_length = 8 |
|
Nah, the error is good, people should know the difference between a number and a letter |
|
Ok, sounds good. Just wanted to double check! I'm actually just starting to rewrite the |
|
I really really appreciate that, and I don't want to sound like an asshole, but we're not ready for that yet. We can slowly convert it over time, but I really want to get these algorithms done before we start on the conversion
- Thomas Perkins
… On Jun 29, 2017, at 9:15 PM, delirious-lettuce ***@***.***> wrote:
Ok, sounds good. Just wanted to double check!
I'm actually just starting to rewrite the optparse section, maybe by tomorrow I'll have something to show you.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
I'm just doing it for fun and learning. If you like it at the end, great, but I'm really not too worried if you don't. I enjoy trying to figure out how to solve problems. The cli part is a couple hundred line puzzle for me, that's all. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since
int(None)raises aTypeError, we can use atry/exceptand if it is raised, setsalt_length = 8. If something else likeint('a')was entered, it will raise aValueErrorbut, if you want it to default to 8 as well, I could change it to something like:Instead of having a separate loop for each character set combination, I used a
char_setstring variable. It is built at the same time assalt_typeto get rid of repeated checks below (if not use_string...,elif use_string is True..., etc).There is no need anymore to create the intermediate list
saltto then turn into a string. Thesaltstring is created immediately using whatever character set is inchar_setand thesalt_length.These changes make generating salts more efficient.