Printable View
Hi
I've written a console program in C++ and I'm wondering if the standard of the code is acceptable on a professional level. Now that I've finished studying, I'd like to use the program as my 'calling card' to get commercial work. Is my programming good enough for that level?
Your feedback would be much appreciated. I've attached a text document containing the code to this message.
Panther2
I donot know really but DO YOU THINK THAT YOU ARE GOOD ENOUGH ?
:)
(1) You mix stdio and iostreamQuote:
Originally posted by panther2
Hi
I've written a console program in C++ and I'm wondering if the standard of the code is acceptable on a professional level. Now that I've finished studying, I'd like to use the program as my 'calling card' to get commercial work. Is my programming good enough for that level?
Your feedback would be much appreciated. I've attached a text document containing the code to this message.
Panther2
(2) You use <iostream.h> -- deprecated; use <iostream>
(3) The code is hard to read and hard to maintain
(4) The code is not well structured -- very repetitive
(5) You use "magic numbers"
(6) You don't check the user input
The same program could be written in half the lines you needed.
You asked for critique and oppinions, so here is my oppinion: the code is beginner level -- very far away from the professional level. Don't show this code in a job interview.
Sorry for being so direct, but I'm trying to be honest.
I'm not going to critique your style or anything, but rather, the
choice of programming material for the example of your skills.
The program itself doesn't really have any object-oriented
design concepts. It's basically a "dumb" program that just inputs
stuff from the user and runs conversions on it. If you want to
have a "calling card" application, make something that will do
something that will demonstrate your skills. What you showed
may have taken you a while, but it's very easy to sum it up in a
few seconds. So, try to take on something a little more
challenging; you want to dazzle these people with your ambition!
--Paul
I can't agree more with Gabriel.
Furthermore, you choice of variable names and function names are cryptic. Your code has to be self-explanatory, I can't understand what the functions do, which brings me to another point, you have no comments at all. You have to comment each function and variable, each input and output from each function.
While this code will do exactly what you want right now, if you or another programmer look at it in 6 months, it will be unreadable and unmaintainable.
Start by giving yourself good practises and discipline over HOW you write code. This is not only true for just C++ but for any programming language. So instead of focusing on C++ features, spend time on style and readability.
I've had in the past the unfortunate opportunity to have to maintain legacy software written by another programmer, for each 2000 lines of code, there are maybe if you're lucky 1 line of comment and a lot of the code is written using cut and paste, magic numbers, mix of C and C++, memcpy vs new, etc.
All I can tell you is that this programmer is not in the good hearts of anybody who touch his crap.
So the code you show us is not what other programmers would expect from you.
Hope you find our comments useful.
This program doesn't show any programming skills.
It contains primitives functions which are the same by structure. Nevertheless this program contains some disign errors. Look at Gabrial warnings.
May be it shows some interesting things from economic side, but not programming.
panther2,
All forum users,
Don't be so hard on the guy!
Let's look upon the good side. You can actually write a few subroutines and evidently crank up some kind of compiler and linker to get the console program up and running. This is a good thing.
There are definitely some aspects of the code which are in poor style. Many forum comments have addressed these topics quite well. Most blatant is the repeating of similar code sequences in individual subroutines which would be better unified in some coherent design scheme. The lack of a meaningful and uniform naming convention is also a sign of weakness.
My advice:
Do not try to improve your chances of gaining an entry-level programming position by showing any sample codes. Usually, when hiring for a permanent position, we try to build a complete picture of the candidate. This includes many aspects such as emotional stability, positive energy and a record of a consistent and sucessful performance in education. With practice and a healthy development climate, new engineers learn from the more experienced developers. Older developers benefit from the creativity and youthful energy of the new dudes.
Assemble your resume and do not worry about your prowess in coding. Everyone sucks at programming in the beginning. And even experienced engineers must adapt to the type of programming within a new technical environment. You are good enough if you can see yourself as an entry-level professional engineer awaiting the opportunity to learn from others and share your positive energy in a professional climate.
Good luck.
Chris.
:)
What are "magic numbers"?Quote:
Originally posted by Gabriel Fleseriu
(5) You use "magic numbers"
:confused:
Panter,
I have to agree with most of the posts here-- I would not show off this code, but rather focus on what dude_1967 said. I would recommned reading some books on code design and software development techniques. One really good book is "Code Complete" by Steve McConnel -- granted most of its examples are not C++ examples, but there is a lot of information in there about general software design as well as how to program as part of a development team -- something that in my opinion is of great value in industry. I would also recommend the reading the book "The Mythical Man Month" -- I don't know how much this will really help your skills as an entry level programmer, but the information about software management, management pitfalls, and other info is extremely interesting and will help prepare you for some of the types of situations you are likely to encounter.
Anyone else with ideas on what other books should Panther read?
- Kevin
Magic numbers are nontrivial numbers hard coded inmiddle of the code.Quote:
Originally posted by Doctor Luz
What are "magic numbers"?
:confused:
"The C++ Programming Language", 3rd Ed., by Bjarne StroustrupQuote:
Originally posted by KevinHall:
Anyone else with ideas on what other books should Panther read?
"Exceptional C++" and "More Exceptional C++" by Herb Sutter
"Effective C++" and "More Effective C++" by Scott Meyers
I know that many people would call these books "heavy artilery" but that's what I'd go with.
For a beginner, I'd recomment "Thinking in C++" by Bruce Eckel.
He does a thorough job of explaining things so that beginners can
understand what's going on [and why]. After that, I'd read the
books Gabriel just mentioned. The good thing about Thinking in
C++ is that you can physically buy it ... OR you can download it
for free on the internet. You can get links to download locations
at http://www.eckelobjects.com.
--Paul
Another book is "Accelerated C++" by Koenig & Moo. It goes right into teaching proper C++ techniques to a beginner in C++, and assumes you already know how to program in another language (so you don't get too much "baby talk" about what is a compiler, what is a variable, etc.).
Since panther2 shows that he/she shows competence in C++ but needs guidance, I highly reccommend this book.
Regards,
Paul McKenzie
First of all, your first 7 lines of output end the line with the '\n' constant. Then you chose to use endl (as you should). There is no reason for this inconsistancy, or explaination.
You ask "Enter an integer on a scale of 1 - 100", yet you never validate this input. What if i enter -10? or 0? That could very possibly give you a division by 0 error. If you are going to be using a number from 1 to 100, you should use an unsigned short, not an int.
Later on you ask "Enter the National GDP(in billions)", and a few others which could very possibly be fractional, so a float should be used. The same goes for most of your calculations.
I can't tell for sure, I haven't compiled it but it looks to me like most your output won't find on one screen, making it useless. If you need so much output, you should page it so the user can at least see it.
Most of your functions and variables have names that do not make sense to anyone, probably not even you. eg. GlaxoSmKlisp. How is anyone supposed to interpert that?
Taking the below function as an example:
int ServiceEconomy(int GDP)
{
int ServiceIt;
ServiceIt = GDP * 25/100;
return ServiceIt;
}
For one, you could simply divide by 4, or comment it so I know why you decided not to just simply divide by 4. And not every number is divisible by 4, so the output of this calculation is very likely a float, so the function should be declared as such. Now, I know why you didn't make it a float, you don't want to have to deal with setting the percision and such. They already touched on magic numbers, but just one more point from me. You should define a macro not only because this value is likely to change, but because it makes your code easier to read. You don't modify the input parameter GDP, therefore it should be declared const. This whole function can and should be written in one line, the temporary storage only slows things down and does not add anything to the program. Take the function below as an example of what this function should look like:
/// I don't really know what this is, so your name should be
/// more desciptive
#define SERVICE_ECONOMY_CONVERSION_RATE 4
float ServiceEconomy(const float GDP)
{
return GDP / SERVICE_ECONOMY_CONVERSION_RATE;
}
Besides the afformentioned problems, this is a C program, plain and simple. If you changed every cout to printf and every cin to scanf, you would have a C program. Which is fine, but you are attempting to demonstrate a knowledge of C++, with out actually writting and C++ code. C++ is an object oriented langauge, so you should use an OOP approach to this problem.
Well...since we are using C++ we should get rid of the '#define' and use a constant with proper type information instead...additionally the usage of 'const' for the argument is discussable. 'GDP' gets passed by value so the 'const' might add more overhead than necessary...nevertheless disregarding this we would end with...Quote:
Originally posted by mwilliamson
You should define a macro not only because this value is likely to change, but because it makes your code easier to read. You don't modify the input parameter GDP, therefore it should be declared const. This whole function can and should be written in one line, the temporary storage only slows things down and does not add anything to the program. Take the function below as an example of what this function should look like:
/// I don't really know what this is, so your name should be
/// more desciptive
#define SERVICE_ECONOMY_CONVERSION_RATE 4
float ServiceEconomy(const float GDP)
{
return GDP / SERVICE_ECONOMY_CONVERSION_RATE;
}
Code:const float cfServiceEconomyConversionRate = 4.0;
float ServiceEconomy(const float GDP)
{
return GDP / cfServiceEconomyConversionRate;
}
To expand in a sense...while it's a bit much to tell you why not to use #define in C++ vs const, you should read up on the subject as it's off a missed subject/usage.Quote:
Originally posted by Andreas Masur
Well...since we are using C++ we should get rid of the '#define' and use a constant with proper type information instead...[/code]
It doesn't quite work that way in this field. You never finish studying in technological fields, because there is always new stuff to learn...Quote:
Originally posted by panther2
Now that I've finished studying...
The guys asks us to criticize his piece of code, I think we are giving him a fair evaluation. I think we've all been fair with our evaluation and to be honest with you, I think it is better to be harsh at first then find out the hard way we had too many illusions. If the guy can't take criticism, then programming is not his path, as we all know our designs gets criticized from time to time by newcomers with new experiences. This doesn't mean that his code didn't serve any purpose, certainly the guy learned a lot from this exercise, maybe now he just needs to build a real challenging application. That would be the next step in my opinion.Quote:
Originally posted by dude_1967
panther2,
All forum users,
Don't be so hard on the guy!
And as others pointed out, employers do not necessarely seek gurus all the time, they will want to give formation to the newbies to mold them to their philosophy of work. Being honest about your capabilities is the key to a sane work relationship. But I honestly do not think that showing this code we saw would be giving the guy any edge whatsoever and can even hinder his chances for hiring as the boss would think the guy has not come to ground with reality.
Exactly.Quote:
Usually, when hiring for a permanent position, we try to build a complete picture of the candidate. This includes many aspects such as emotional stability, positive energy and a record of a consistent and sucessful performance in education. With practice and a healthy development climate, new engineers learn from the more experienced developers. Older developers benefit from the creativity and youthful energy of the new dudes.
Finding compatible workers (programmers) is also a challenging task and sometimes clashes can occur, that is the most important thing to be careful about in my opinion, as it can drive down productivity a great deal when people do not share the same programming philosophy.
[/QUOTE]
Good luck.
Indeed. I've seen one too many senior programmers who relied on "old" technologies (DB2, Mainframes...) realize they were now incompetent in new technologies (Internet, Desktop applications...) that they went into deep depression.Quote:
Originally posted by galathaea
It doesn't quite work that way in this field. You never finish studying in technological fields, because there is always new stuff to learn...
It's never a good news to learn you have become useless part of the workforce. Be careful guys. We didn't choose to program because it was easy, but because it was hard. :)