Code critique. Post your code and I'll tell you how to do it better

Code critique. Post your code and I'll tell you how to do it better.

> prefer C, Python and Java, someone else can review C++ and other.

Other urls found in this thread:

codereview.stackexchange.com/questions/176798
pastebin.com/CC4dCEVa
twitter.com/NSFWRedditVideo

print('OP is a faggot')

codereview.stackexchange.com/questions/176798

In python 2, you can remove the parentheses. its less verbose and easier to read code.

python 2 is deprecated and should not be used

>easier to read
personally i think parenthesis can increase readability

No

avoid macros, they make static analysis harder, which makes auto finding bugs harder

> for (int i = 0; i < MAZE_A; ++i)
Always user curly braces. While the code might be "correct" now, adding another statement in the future would be formatted right, but not run right.

> bridges[n] = EAST(i);
> paths[n++] = EAST(EAST(i));
DRY, or dont repeat yourself

n is updated in all of the conditions above. It could be extracted to its own statement at the end.

#include
#include

int main(int argc, char const *argv[]) {

int minutes;
std::cin >> minutes;

int m{minutes % 60}, h{minutes < 60 ? 0 : minutes / 60};

std::cout

python 3 is an unholy abomination. Slower and more verbose.

>adding another statement in the future would be formatted right, but not run right
One line statements without braces is a valid brace style and is entirely opinion based.
>n++ It could be extracted to its own statement at the end.
Good point.
If you're expecting speed from an interpreted language you're braindead to begin with.

No

Is there any other way to do this function:

pastebin.com/CC4dCEVa

Aside from little nitpickings and code cleanup I have to do later, I'm mostly concerned if there is any more readable way to replace all those else if's.

doesn't account for negative minutes :^)

> char const *argv[]
most professional programmers defined this as
> char const **argv

> std::cin >> minutes;
for a small program this is okay, but for a larger one you should extract this to a function, and make the input an argument
> why?
because it makes it easier to write unit tests for this. (note: if you make video games you can ignore this advice). The reason is becuase programs that are easier to test tend to be better structured.

> int m{minutes % 60}, h{minutes < 60 ? 0 : minutes / 60};

Don't *ever* do your own time conversions. Always use a library. There are days with 25 and 23 and 23.5 hours in them. There are years with less than 350 days. Someone else has figured out all this insane shit; benefit from their work.

> return 0;
Good. Always return a value from main(). People who write void main() are shit tier programmers, and laughed at on their first day at a new job.

>Good. Always return a value from main(). People who write void main() are shit tier programmers, and laughed at on their first day at a new job.
People who care about other's laugh are pathetic to begin with. The correct argument is that void main is not a standard main prototype.
The only standard main prototypes are int main(void) and int main(int argc, char **argv), all other shit, including yours, is bad.
Also, you can omit return statement, return 0 is default.
>Don't *ever* do your own time conversions. Always use a library
Depends on program use-case.

>pastebin.com/CC4dCEVa

Personally I would extract all the cases out into an enum (enums in C# can take strings in their constructor, right?)

Long if-else chains are a code smell. See if you can't use a map or table to do quick lookup. This turns a O(n) algorithm into an O(1) one.

> while(n-- > 0)
Personally I would make this a for loop (they are easier imo to understand.

> waitForReadyRead(1000)
This is controversial, but generally its considered a bad idea to block on your thread. Event driven IO is popular nowadays because OSes make it really expensive to do multithreading.

I know its a small time project, but if you care about performance you can poll the network for events and hand them off to the appropriate thread. I think JS got this right, but most of the other things wrong. Pick and choose the best of other languages and keep them. (Do the map shit first tho)

>laughed at on their first day at a new job.
Don't be a sad cunt, you had a good post going.

> People who care about other's laugh are pathetic to begin with.
While I generally agree with you about caring what other people think, often times multiple people make the code better, not worse. Being spec compliant is an easy way to show you aren't a green horn. There are social benefits.

> Depends on program use-case.
So far, in my exp., it has always been a good idea to use an external library for time stuff.

For example, we had a cron job that would run once a day. When should it run? "At midnight, user". Midnight can change for different people, and has happened before, causing weird intermediary results. Better let the pros do this, so you can focus on what you actually care about.

If(x=1){
If(y=1){
Do stuf;
Return;
}
}


Been doing c++ for 7 years as my job now so gl in trying to correct me!

> promoting being a dunce
good luck, user

You can combined it into a single if statement.
Add vertical lines of code unnecessarily makes people have to scroll more to see the full logic of your function.

>generally its considered a bad idea to block on your thread
>if you care about performance you can poll the network for events and hand them off to the appropriate thread
True, I thought about that and tried to initially, but keeping it all synchronous just simplifies the code so much with no noticeable difference.

>Long if-else chains are a code smell.
Yeah I know. I just couldn't think of any solution that's much better. This function is most of the file and it's honestly not hard to read, just very ugly.

A map might actually work if I could just use function pointers. I'd still need the if's for setting mouse position and a few other weird ones, but most of the functions called simply pass message substring'd to the first space as the only argument. Thanks. I'll give it a try.

> One line statements without braces is a valid brace style and is entirely opinion based.

It is opinion, based, but it is based on the experience of hundreds of thousands of other programmers. Don't ignore their advice lightly.
(A notable exception are the kernel programmers, but they have internalized this brain damage by used 8-width tabs)

> If you're expecting speed from an interpreted language you're braindead to begin with.

Best to make speed improvements if they are easy. Python may not be the fastest, but let's not make it slower than it has to be.

How would i combine those into 1line????????

Well, first of all
> if( x =1)
probably doesn't do the right thing.
Second,
if (x == 1 && y == 1) {
doStuff();
return;
}

O shit it does compile i tried that a few years back and it gave me an error so i havent used it since. How do u into code bubble thong?

Why is void main() bad user? What should I be using instead? int main() didn't seem to make much sense to me because it's main, why would I be returning an int? I'm new to C/C++ so pls no bully

>not just x == y == 1
what a meme

because this is the return code passed to your shell. Try this:

$ cc main.c
$ ./a.out
$ echo $?
0

This code is what comes back from main(). It tells if your program completed successfully.


that will fail if comparing with 2. Don't try to be tricky user, be explicit.

it is either
int main()

or
int main(int argc, char* argv[])

The program returns the error code. On most systems, 0 is success.
If there is arguments, they are counted (spaces separate arguments) in argc and then you can access them in argv.
The first argument (argv[0]) is the application name.

oh oops i was thinking of chaining assignment not comparison, muh bad
still any amount of replication triggers my jimmies, what if you decide to check x and y for 1.5

so assign both x and y to 1.5? Then theres no need for the if.

Remember, C programs have an optimizing compiler that is very good at taking your code and simplifying it down to the best version. Code is meant for humans, and only incidentally for computers.

See Single Static Assignment for how compilers do this.

No it's

auto main(int argc, char const *argv[]) -> int

>People who write void main() are shit tier programmers, and laughed at on their first day at a new job.
t. Userspace programmer
void main is the way to go with embedded devices, there is no OS to return to.

That is literally the stupidest argument for violating the C ABI. Fuck yourself with a raspberry pi.

#include
#include
char c,
f = 0;
int main(void){
while(read(0, &c, 1) > 0)
if(c == ' ')
f = 1;
else{
c = (f)?toupper(c):tolower(c);
f = 0;
write(1, &c, 1);
}
return 0;
}

Yeab right gramps

Use significant variable names so people don’t have to waste their time figuring out what your shit does.

> Don't define global variables. They are an anti pattern.

> while(read(0, &c, 1) > 0)
nanonit: reading a -1 indicates a problem, something you might want to check errno for.

reading from 0 will probably work for ever, but it isn't obvious. For example, if you close(0), the next file you open will have a filedescriptor of 0. It won't make you code more correct, but it will make it more clean. Use stdin to make the code clear that you mean stdin, and not just the filedescriptor in slot 0

write(1, &c, 1);
fyi this won't work for unicode characters. You might consider usingf utf8 or multibyte encoding.

> If you think this doesn't apply to you consider the following:
> Many vidya developers translate their games to other languages to sell to more markets
> if you don't deal with this shit now, it will be much harder later

You should leave them off if you're printing multiple values though.

> has obviously never benchmarked python 2 vs. 3
> obivously doesn't pay the powerbill for the computers running his code.

Next time some code is running slow, you can think "wow, Python 3 was such a great choice. I love waiting for it to complete."

Performance doesn't matter, Python2 is a terrible language compared to Python3.
A sane person wouldn't write code that has to run fast in Python to beginn with.

What are your deal breakers for python2?

>> obivously doesn't pay the powerbill for the computers running his code.
Developers are more expensive than computers.
Developer time > Processing time.

Why is python2 adding dev time?

So using void gets rid of that run was successful code? Why is that bad though, wouldn't you know if a program ran or not? Couldn't you just use a printf to output "run completed"

#include

double getResult(double &a, char &op, double &b) {

switch (op) {
case '+':
return a + b;
case '-':
return a - b;
case '*':
return a * b;
case '/':
if (!b) {
std::cerr

Why

It will print tuple of items instead of each item.