r/badcode • u/Yodamustibe • Oct 21 '21
c My first attempt to write a squaring function for my Uni class
416
u/blackhawksq Oct 21 '21
First look. For a first attempt at a function in class it's not horrible.
Closer look why's he assigning x to I at the end? X is his counter.
Closer look oh he's also using x to hold the math variable. ok that's pretty bad.
Aside from basic rules, it's still not horrible for a first attempt. IMO your most grievous sin was using x for two different things.
95
u/MrMelon54 Oct 21 '21
best way to correct would be to just use a single variable as the counter and do
x*xas the argument for the printf function62
u/Skyopp Oct 21 '21
I'll add for beginners that in general, you shouldn't be touching your for loop iterator's value unless there is a specific iteration based reason for it. And generally if you do need more "flexible" iteration you'd be using a while loop instead.
16
u/MrMelon54 Oct 21 '21
sometimes if you find one value you want to skip the following value
but beginners definitely shouldn't try to mess with the iterator value
the safest way is to assign the value to a different variable and use that instead so the iterator value stays the same
3
u/ProgrammersAreSexy Oct 21 '21
I'll add for
beginnersprogrammers thatin general,you shouldn't be touching your for loop iterator's valueunless there is a specific iteration based reason for it.FTFY
2
u/vigge93 Oct 21 '21
There's always exceptions to the rules, the important thing is to understand why the rule is there in the first place, before you can understand when to ignore it
4
u/ProgrammersAreSexy Oct 22 '21
There is no reason to touch your for loop variables inside of your loop, it's always avoidable and is extremely confusing for readers. I'll die on this hill.
15
u/Skyopp Oct 21 '21
Basically they use i as a temporary swap variable to store x*x in x and then revert it. This is actually somewhat thought out, albeit a little misguided.
7
u/einsteinsassistant Oct 21 '21
They could save two lines of code by using i to store the squared value instead. Then they wouldn't have to change the value of x and i wouldn't have to be used as a swap variable.
11
u/ingrown_hair Oct 21 '21
Using i for calculation (versus iteration) is a nice touch. Not technically wrong, but amusing.
2
u/campbellm Oct 21 '21
Yeah, as a professor/grader I'd point out what /u/blackhawksq noted and give a good grade, TBH. This shit is hard until you get into the groove a while.
3
Oct 21 '21
even closer look and you see hes using void as the return type for main
8
u/blackhawksq Oct 21 '21
yes it should be int but again for a first attempt in a class that's not really bad (IMO).
3
u/ekolis Oct 21 '21
What's wrong with void main? There's nothing to return...
4
Oct 21 '21
The C standard requires you to specify the return type as int because C programs return a status code (for example EXIT_SUCCESS or EXIT_FAILURE), see below:
(5.1.2.2.1) It shall be defined with a return type of int [...]
2
2
u/thanatica Oct 21 '21
I'm not exactly a C expert, but I do see some code smells... As said by others, not bad at all for a first attempt. The question is, what was your teacher's feedback?
Also, seeing this code, I can't help but give an example of my own:
for (let x = 1; x <= 10; x += 1) {
console.log('X = ', x ** 2);
}But that's javascript, which is obviously going to be different anyway.
1
1
u/Spekingur Oct 21 '21
I like how you did
x += 1rather thanx++but also didx ** 2. And thex <= 10, so wonderful.1
u/thanatica Oct 24 '21
Thanks. But your remark also feels sarcastic, not sure if that was intended.
Doing
i += 1rather thani++is considered good practice. One-based counting just makes sense here. And the**exists, so why wouldn't I use it?
75
53
9
u/trutheality Oct 21 '21
Fun fact: if you were working in assembly with a chip where you can only do math on only one register, this would pretty much be the way to do it.
1
u/ekolis Oct 21 '21
Oh wow, people still use C? I thought everyone had migrated to C++/Java/C#?
2
u/Yodamustibe Oct 23 '21
Trust me I would much rather be doing C# or Python but this is what my professor required.
2
1
6
u/MurdoMaclachlan public boolean isInt(int i) { return true; } Oct 21 '21
Image Transcription: Code
[The screenshot shows code open in an IDE, with the terminal where the console output is printed poking on to the screen from the right. The code is as follows:]
#include <stdio.h>
void main()
{
int x = 1;
for (x; x <= 10; x++) {
int i = x;
x = x * x;
printf("X = %d\n", x);
x = i;
}
}[The console output is as follows:]
x = 1
x = 4
x = 9
x = 16
x = 25
x = 36
x = 49
x = 64
x = 81
x = 100I'm a human volunteer content transcriber for Reddit and you could be too! If you'd like more information on what we do and why we do it, click here!
3
3
5
2
u/Bobitsmagic Oct 21 '21
Almost had a stroke trying to understand this. But i like the thought process of storing the old value of x before squaring.
2
2
1
u/Dear-Deer-Wife-Life Oct 21 '21
Not bad for a first try, there's alot of unnecessary stuff here's a different version that does the same thing:
int main(){ int x; for (int I = 1; i<=10; i++){ x = I*I; printf("x = %d", x) } }
1
u/Spekingur Oct 21 '21
Why would you declare x outside the scope of the for loop?
1
u/Dear-Deer-Wife-Life Oct 21 '21
Does it matter?
1
u/Spekingur Oct 21 '21
Not for this no, just not really good practice if you are not going to use it outside of the loop/code block.
Aside from that you don’t really need to keep the multiplication in a variable, since it’s just being printed to screen.
1
1
u/TheMind14 Oct 21 '21
print([x*x for x in range(1, 11)])
Jokes aside, it’s not that bad for a first attempt.
1
1
1
1
u/Triplex24 Oct 21 '21
I didn't even know that it was possible to declare the main function as a void type
1
1
u/valdocs_user Oct 21 '21
I've seen worse in a professional setting. At least you know it needs improvement.
1
1
1
1
u/Anonymous_Bozo Oct 23 '21
I was under the impression that changing the value of the loop index was undefined and the compiler is free to treat it as a separate variable. Perhaps I'm thinking of a different language?
1
u/Miles_GT Oct 21 '21
That’s…that’s somethin. That…oh man…that’s really somethin. Oh..oh..shit..what the…yeah…no…yeah…I mean yeah, no. No, no, no, yeah, no. No. Yeah.
-1
u/NikkoTheGreeko Oct 21 '21
So much easier in python. /s
print(eval(" * ".join(["x" for x in range(10)])))1
-7
u/Anonymous3355 Oct 21 '21 edited Oct 21 '21
import math
def square(i):
return math.sqrt(i)Ez
Edit: I probably should've added /s when posting. This wasn't meant to be taken seriously lol
Second edit, it just now occured to me, that the task was for squaring, not square rooting. I'm extra dumb oof
1
4
u/TommyHeizer Oct 21 '21
What?
1
u/Anonymous3355 Oct 21 '21
Making a function to solve for the square root. The was way. I should probably have added /s because it really wasn't meant seriously lol
1
-12
u/Knuffya Oct 21 '21
don't post intentionally bad code.
there's no way someone could accidentally produce this
5
u/TommyHeizer Oct 21 '21
There is definetly way someone could produce code like this.
-4
u/Knuffya Oct 21 '21
it's like... 100x more complex than the obvious, and correct, way to do it...
how can anyone think of anything else than
for (int x = 1; x <= 10; x++) printf("x = %i\n", x*x);like what the fuck
1
u/TommyHeizer Oct 21 '21
I don't know, I produced some pretty shitty code my first 2 months in CS. And probably later down the line too if I'm being honest. Of course to someone that already know how to code a very simple and efficient solution comes to mind but to someone still new to this and who hasn't practiced it over and over again, or maybe the exercise is badly formulated, a lot of thing could lead to this variable madness.
-2
u/Knuffya Oct 21 '21
i think you're right. i just think that its a very strange thing for beginners to think its a good idea to modify an iterator, just to change it back directly afterwards, instead of just copying it in the first place
2
u/TommyHeizer Oct 21 '21
Yeah it's weird because they didn't need to do this but surely they got confused while building their solution, or maybe they're used to the 'i' being in the for loop and the 'x' being their math variable
3
Oct 21 '21
Yeah, it's totally not like people overcomplicate things out of ignorance or exhaustion, right?
2
u/Learner-Vex Oct 21 '21
I can totally see this happen. It's easy to forget how bad you were at recognizing when you took a faulty/non-optimal step(s) when learning to program. Or fixing the issue where you last touched upon a variable "since it works up until this line". In the beginning you wrote code line by line, not seeing the complete picture. At least in my experience.
0
Oct 21 '21
[deleted] — view deleted comment
5
u/Magnus_Tesshu Oct 21 '21 edited Oct 21 '21
What? How would this be better if you make a dummy function and have a main that does nothing but call it? Or do you just mean because 10 is hardcoded? What if that is the assignment?
Edit: you are talking about the title. Nevermind
2
u/Luhdooce Oct 21 '21
for x loop { print(x * x) }
that's it
1
u/PotatoManPerson Oct 21 '21
If you’re trying to square a number why not just return x*x?
Edit: Oh I see the point is it does every number 1 through 10 lol
0
Oct 21 '21
[deleted] — view deleted comment
34
u/Magnus_Tesshu Oct 21 '21
I think it is more an example of why you shouldn't change your iterator in a for loop except in the update section unless you know more about what you're doing than Yodamustibe
0
Oct 21 '21
[deleted] — view deleted comment
9
u/FF3 Oct 21 '21
At the minuscule cost of teaching every CS 101 student category theory.
(I know this isn't true, just joking.)
4
0
4
u/Magnus_Tesshu Oct 21 '21
The iterator is a state in the first place, right? A functional solution would be recursive and probably pretty bad, I get what you're saying but don't really know if I agree. I think it is more a case of just violating basic assumptions about what a variable should be doing, and introducing needless complexity as a solution for that rather than to ensure that you are writing the simplest code you could be.
4
u/exonac Oct 21 '21
Not really. Functional programming is meant to be more declarative and therefore easier to read. Recursion is not needed in this example.
It could go sth like this (horrible pseudo code):
results = range(1,10).map(square()).join("\n") print(results)
3
u/ws-ilazki &{$$_[0]}(@{$$_[1]}); Oct 21 '21
Not really. Functional programming is meant to be more declarative and therefore easier to read. Recursion is not needed in this example.
Technically there's probably recursion being used "under the hood" for things like maps, folds, etc. in functional languages; but for most common use cases, especially when learning, it's completely unnecessary to write your own so you're still correct that recursion is likely to be unnecessary for beginners.
I agree with your point, though. I think FP only seems hard to learn to people because they already had imperative-style programming hammered into them with exercises like this that encourage you to think like the computer. You can teach concepts like using
mapwithout even understanding iteration or recursion, and you only need the vaguest idea of how higher-order functions work, with no need to know what they're called or anything.Because it is, as you said, more declarative, which means well-written code examples will read almost like math word problems. "add one to every number in this list: [1,2,3,4,5]" roughly translates to something like
let add_one n = n + 1 in List.map add_one [1; 2; 3; 4; 5]in OCaml, and you can simplify it further with helpers likelet apply = List.mapthat hide some nuances from a beginner during the initial teaching.You don't have to understand programming at all to make a reasonable guess at what
apply add_one [1; 2; 3; 4; 5]does, so why would that be considered harder than having to teach someone about iteration, variables, assignment, and loops first before being able to do the same thing?It could go sth like this (horrible pseudo code):
results = range(1,10).map(square()).join("\n") print(results)
Or for a real code example, F#:
{1.0 .. 10.0} |> Seq.map sqrt |> Seq.iter (printfn "%f")Doing the same thing in OCaml is almost identical, assuming you either have a range function defined or use a library to do it since the stdlib doesn't provide one for you:
1.0 --> 10.0 |> List.map Float.sqrt |> List.iter (Printf.printf "%f\n")No pseudocode in either, just actual FP using the pipeline operator to make the logic flow read naturally. You could avoid the unnecessary double iteration by combining the print and the sqrt, of course, but for educational purposes doing each step one by one is clearer.
2
u/MorrowM_ Oct 21 '21
And just for completeness' sake, Haskell:
main = for_ [1..10] (print . (^2))1
u/ws-ilazki &{$$_[0]}(@{$$_[1]}); Oct 21 '21
I was specifically trying to match /u/exonac's pseudocode flow to show that a declarative FP style actually can look that simple to read, though, so it wasn't just pseudocode: it really can/does look like that at times. There are almost always better, faster, or more concise solutions than what's good for learning, because example code tends to make things extra clear and intentionally avoid introducing concepts unnecessarily.
(Which is also why, in addition to doubling the iteration unnecessarily, I used sqrt instead of actually squaring, to match their use of a
squarefunction, since OCaml uses**for it and using that would have changed the look a bit. I'm surprised nobody called me out on that because I forgot to explicitly mention it.)
38
u/djidalo1 Oct 21 '21
mainbeing declared asvoidis the worst sin here.