Code Review

Let’s have a code review thread.

Post a snippet of your code and have other people rate, give advice, and criticisms of your style, algorithms, form, etc.

Other urls found in this thread:

blog.calyptus.eu/seb/2011/01/javascript-call-performance-just-inline-it/
api.Sup
stackoverflow.com/questions/20957693/fizzbuzz-in-python-using-random-how-does-it-work#20957817
twitter.com/SFWRedditGifs

for (var y = from_y; y < to_y; y++) {
for (var x = from_x; x < to_x; x++) {
var wall = scene.map.get(x, y)[1];
var owall = scene.map.get(x, y)[2];
if (wall > -1) {
//if (wall > 0 || (owall > 0 && direction === MM_DOWN)) {
var wall_rx = x * MM_TILESIZE;
var wall_ry = y * MM_TILESIZE;
//if (aabb.intersects(new pf.AABB(wall_rx, wall_ry, wall_rx + MM_TILESIZE, wall_ry + MM_TILESIZE))) {
if (new mm.AABB(wall_rx, wall_ry, wall_rx + MM_TILESIZE, wall_ry + MM_TILESIZE).intersects(aabb)) {
// collision detected
// fix player position
switch (direction) {
case MM_UP:
this.y = wall_ry + MM_TILESIZE - this.bbox.y;
break;
case MM_DOWN:
this.y = wall_ry - this.bbox.height - this.bbox.y;
break;
case MM_LEFT:
this.x = wall_rx + MM_TILESIZE - this.bbox.x;
break;
case MM_RIGHT:
this.x = wall_rx - this.bbox.width - this.bbox.x;
break;
}
// return with collision type
return MM_COLLISION_WALL;
}
}
else if (!allowFallthrough && owall > -1 && direction === MM_DOWN) {
var wall_rx = x * MM_TILESIZE;
var wall_ry = y * MM_TILESIZE;
if (new mm.AABB(wall_rx, wall_ry, wall_rx + MM_TILESIZE, wall_ry + MM_TILESIZE / 2).intersects(f_aabb)) {
// down direction implied
this.y = wall_ry - this.bbox.height - this.bbox.y;
// return with collision type
return MM_COLLISION_WALL;
}
}
}
}

Where is your God now?

Absolutely disgusting.

I realize, I'm in the process of rewriting it

Any hints?

what does this do?

Pls post the implementation of any of delete_* function

Checks for wall collision and allows falling through a wall below the player

It's only one part of a huge function, I'm in the process of rewriting it but it's painful

>It's only one part of a huge function
Then your function is getting a bit big. Any chance of you splitting it into smaller, private ones?

Function calls are expensive

You don't know what you are doing

No u

blog.calyptus.eu/seb/2011/01/javascript-call-performance-just-inline-it/

while (S_HP > 0 & B_HP > 0)
{
B_HP += 10;

//samurai attack
if (s_chance_ignore_def.Next(0, 100) 0 & B_HP 0 & S_HP

Premature optimization is even more expensive.

I smell unity

not really, it's just a snippet from a simulation of something similar to pic related

for(int x = 700; x < 700; x--){
int start = 700;
int increment = 13;
double answer;
answer = start - increment;
if(answer < 0){
break;
}
}
[\code]

answer is a double because it needs to store the value taken from two variables

>[\code]
huehue

>answer is a double because it needs to store the value taken from two variables
dude wat?

double means two and it's taking the integers start and increment so it has to be a double otherwise the code will segmentation fault

I don;t believe anyone can be that stupid

1/10 troll bait

>not understanding the purpose of something as basic as a double
Get off Sup Forums retard

This guy gets it. They need to make a triple so that we can do something like
answer = start - increment + answer
or something

this. if you are combining 2 variables it is a double, 3 variables a triple, etc

>double means two and it's taking the integers start and increment so it has to be a double otherwise the code will segmentation fault
le cs grad meme

Did you read the code? It actually has comments.

...

>not using an ORM

enjoy your SQL injections

I thought prepared statements were safe.
That's what the hordes of Pajeets told me at least.

> an additional layer of abstraction enhances security

>i don't know what i'm talking about

...

> i use angular with a shitty orm and talk about stuff i don't understand
your shitty orm maybe enhances comfort and maintainability but it's not a security layer by itself.

>i use angular
where did i say i use Angular?

I've use Java ORM with Servelts and Stateful/Stateless beans

In general, ORMs use parametrized SQL, which will protect you from a direct SQL injection attack.

I get it that ORM makes things easier but I can't find info on how it would be safer.

This nigger posting his latest lecture slides

>2011
>javascript
If your language of choice can't inline simple calls and you need performance, you chose the wrong language.

NB. Day 14.

NB. Builds disk of 1s and 0s

disk =: 3 : 0
in =. y , '-'
row =. 0
dsk =. ,.i.0
while. row < 128 do.
dsk =. dsk, (,@:((4#2)&#:"0)@:((".`((-&87)@:(a.&i.))@.(e.&'abcdef'))"0)@:(2&day10)) (in,":row)
row =. >:row
end.
dsk
)
NB. Part 1.

day14p1 =: +/@:(+/@:disk)

NB. Flood fill algorithm, for part 2.

flood =: 4 : 0
node =. 2{.y
if. (*./ node < $x) *. (*./ node >: 0 0) do.
color =. (

Tell me about your videogame user

#! /usr/bin/python3
# downloads all images from Sup Forums thread url in format:
# [http(s)://]boards.Sup Forums.org/g/thread/threadnum[/thread-name]

import re
import os
import sys
import bs4
import requests

if len(sys.argv) == 1:
print("No arguments given.")
sys.exit()
url = sys.argv[1]

# Parse URL and quit if invalid
chanRegex = re.compile(r'''
(http(s)?:\/\/)?
boards.Sup Forums.org\/
(\w+)\/
thread\/
(\d+)(\/)?
(.*)?''', re.VERBOSE | re.IGNORECASE)
# group 3 = board, 4 = threadNum, 6 = threadName

threadInfo = chanRegex.match(url)
if threadInfo is None:
print("Invalid URL: %s" % url)
sys.exit()

res = requests.get(url)
res.raise_for_status()

board = threadInfo.group(3)
threadNum = threadInfo.group(4)

print("Downloading images from /" + board + "/")
print("Thread: " + threadNum)
print("...")

soup = bs4.BeautifulSoup(res.text, "lxml")
imageElems = soup.select('.fileText')

# make image directories if don't exist
homeDir = os.path.expanduser('~')
os.makedirs(os.path.join(homeDir, 'Pictures', 'Sup Forums', board), exist_ok=True)

# save all images in thread
for image in imageElems:
imageUrl = "https:" + image.select('a')[0].get('href')

imageFileName = image.getText().split(' ')[1]
# check if title has file extension, else add one from link
fileExtRegex = re.compile(r'^.*\.(.*)$')
if fileExtRegex.match(imageFileName) is None:
imageFileName = imageFileName + "." + imageUrl.split('.')[-1]

imageFile = os.path.join(homeDir, 'Pictures', 'Sup Forums', board, imageFileName)

if os.path.exists(imageFile):
print("File already exists: " + imageFile)
else:
print("Saving: " + imageFile)

res = requests.get(imageUrl)
res.raise_for_status()

imageFile = open(imageFile, 'wb')
for chunk in res.iter_content(100000):
imageFile.write(chunk)
imageFile.close()


print('Done')

this is stupid; not the code, but because the api exists and makes this stupidly easy with just jq & xargs, you can literally do this with one line

...

here:

get_thread () {
# e.g.: board=$(echo $1 | cut -d/ -f4)
thread=$(echo $1 | cut -d/ -f6)
curl "api.Sup Forums.org/$board/thread/$thread.json" | \
jq -r '.posts[] | select(.tim != null) | \
"i.4cdn.org/$board/\(.tim)\(.ext)"' | \
xargs -L1 -P4 wget
}
export -f get_thread


something to this effect is much better (not tested)

just realized that line of jq wasn't supposed to be split into two lines, oh well

# prep offset reset request
current_offsets = {c[0]: c[1] for c in consumer.fetch_offsets()}
fresh_offsets = []
for p in topic.partitions.values():
if offset >= 0:
if offset > current_offsets[p.id].offset:
diff = abs(offset - current_offsets[p.id].offset)
click.secho(
f'Skipping partition {p.id:2d}, offset is too new (+{diff})',
fg='yellow')
continue
new_offset = current_offsets[p.id].offset - abs(
offset) if offset < 0 else offset
fresh_offsets.append((p, new_offset))

#!/usr/bin/env python2
import random

for i in range(0, 100):
if not i % 15:
random.seed(1178741599)
print [i+1, "Fizz", "Buzz", "FizzBuzz"][random.randint(0,3)]

wait, how does that work?

Carefully chosen seed.

Self-answer
stackoverflow.com/questions/20957693/fizzbuzz-in-python-using-random-how-does-it-work#20957817

put some of the loop innards into blocks with descriptive names and call those. It will be more readable.

Question, why would you post your code here? Someone will steal it.

Are you mentally retarded or just a fucking idiot?

No worries senpai nobody will steal your trash code.

this.getPlayers().putAll(players.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, entry ->
Streams.stream(entry.getValue().getAsJsonArray())
.map(JsonElement::getAsString)
.collect(Collectors.toList())
)
)
);

this is actually java

Made with Unity

i enjoyed that reference my man

def _add_opts(regex,option_dict,offset):
options=filter(regex.match,sys.argv)

a_n = len(sys.argv)
options,list_options=tee(options)
list_options=list(list_options)
for option in options:
i =sys.argv.index(option)+1
if len(sys.argv) == i:
break
option_dict[option[offset:]]=sys.argv[i]


def get_options(short_alias):
option_dict={}
short_dict={}
short_re=re.compile("-[^-]+")
long_re=re.compile("--w+")
_add_opts(short_re,short_dict,1)
_add_opts(long_re,option_dict,2)
for key in short_dict:
option_dict[short_alias[key]]=short_dict[key]
return option_dict

short_alias={
'p' : 'profile',
't' : 'type',
'u' : 'user'
}

#read parameters from config file and argv
_config=json.load(open('./worklist.conf','r'))
options=get_options(short_alias)

#TODO check that combination of configs makes sense

def github(params):
endpoint="api.github.com"
if 'user' in params:
endpoint+="/users/"+params["user"]+"/repos"
r=requests.get(endpoint)
tty=open('/dev/tty','w')
for repo in r.json():
tty.write(repo["name"])
tty.write('\n')
return r.json()

def gitlab(params):
endpoint=params["endpoint"]
r.requests.get(endpoint+"/projects")
tty=open('/dev/tty','w')
for repo in r.json():
tty.write(repo["name"])
tty.write('\n')
return r.json()

No it doesn't, retard.

>In general, ORMs use parametrized SQL, which will protect you from a direct SQL injection attack.
>not sanitising inputs

you deserve everything you get

this triggers me

Thanks, user. I've never used JQ. I figured there would be a better way to do this. It seems anytime I automate a task with python there is a simpler way to do it in bash.

ReSharper would blue screen.

Is this Sup Forums source ?

Everything can be “fixed” with another layer of abstraction. It just costs cycles & performance.

I'm not sure what's wrong with my code.

class Pipeline < BasicObject:

func initialize(this: Pipeline, stages: List(Atom)):

bless this, "Pipeline"

for i, stage_name := range stages:
stage_name ! {:init_pipeline, self()}

nb_procs := stages.length

while nb_procs > 0:
receive:
{:ok, name, priority, type} ->
nb_procs--
define_method ("proc_" + name.to_s).to_sym, args:
name ! {:pipeline, args, this.next}
after 10:
panic(Error.new('timeout'))

return :ok, this

func next(this: Pipeline):
return this.send :priority, @stage_num

np, but that shouldn't stop you from using python or whatever you're comfortable with

jq is super handy, learning to use it along with sed/grep/awk/xargs is really helpful