nokonoko / Pomf

Simple file uploading and sharing, source for the now shut down site Pomf.se
MIT License
465 stars 98 forks source link

Changed name generater to use for loop. #19

Closed mrjkuhl closed 9 years ago

mrjkuhl commented 9 years ago

Using a for loop reduces LOC and induces fewer autism attacks.

NuckChorris commented 9 years ago

This is less clear and will be unrolled anyways. I'm afraid I don't see the point?

mrjkuhl commented 9 years ago

@NuckChorris , Most people would consider it good programming to use a for loop in place of simply calling a single function several times, and I honestly doubt that most people would say that a simple for loop is unclear.

SEAPUNK commented 9 years ago

@NuckChorris It's not unclear to me. It (except the comment) explicitly says to generate 6 extra random letters. Less code copying.

You know, kinda like instead of using

var a = 1;
a = a + 1; //add one to variable a
a = a + 1; //add one to variable a
a = a + 1; //add one to variable a 
a = a + 1; //add one to variable a
a = a + 1; //add one to variable a
a = a + 1; //add one to variable a
a = a + 1; //add one to variable a

and replacing it with

var a = 1;
for (var i=0;i<7;i++){
  a = a + 1; //add one to variable a 7 times
}
zebracanevra commented 9 years ago

it's just a ridiculous thing to pick out, and would literally be a 10 second change if it's needed sometime later

SEAPUNK commented 9 years ago

It still makes the code better (at least prettier), nitpicking or not. ;^)

NuckChorris commented 9 years ago

Personally I think both are fine but unrolled is better. My main reasoning is that I want it fully clear how goofy the algorithm to pick a name is

NuckChorris commented 9 years ago

Side note: I'm probably gonna switch it to base36 of an integer when I do the node stuff. Might also be doable in PHP, anyone know? I'd rather not shove my head up PHP's docs. It's like drowning in sewage.

SEAPUNK commented 9 years ago

I want it fully clear how goofy the algorithm to pick a name is

You can further make it look goofy by using very long variable names, and obfuscating the code so a + 1 turns into a - ((2/10)*-5) and etc

or, you can simply make your code look good, and then add a big-ass comment prepending the code, e.g.

///////////////////////
//WARNING: THIS CODE IS FUCKING STUPID DON'T EVER DO SOMETHING LIKE THIS
//////////////////////
//START

//[code]

//END
ghost commented 9 years ago

The reason that it's like it is currently is because it wasn't just 6 random characters previously - it had the file's crc32 in there too. And it's going to be changed in the future so there is literally no point in this. Besides the fact it looks better when unrolled anyway.

NuckChorris commented 9 years ago

I also feel the current code is far more direct. The thing is, programmers have to maintain a mental model of the program at all times, and this is replacing a more obvious (but perhaps uglier and harder to change) structure with a less obvious version :/

I can look at the current code and see it generates 6 random letters and shoves them into a string. This loop mildly obfuscates it.

I guess what I'm trying to say is that it seems like it has a lot of negatives and not very many positives, except slightly better code metrics and perhaps an optimization in memory usage (of a few bytes)

I think aki summed up my thoughts fairly well

ghost commented 9 years ago

:hand: high five :hand:

NuckChorris commented 9 years ago

No I will not get high with you, stop trying to make me into a criminal :(

SEAPUNK commented 9 years ago

Well, to each their own, then. :^)