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.
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
(1) You mix stdio and iostream
(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.
Gabriel, CodeGuru moderator
Forever trusting who we are
And nothing else matters - Metallica
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!
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.
Martin Breton
3D vision software developer and system integrator.
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.
"UNIX is simple; it just takes a genius to understand its simplicity!"
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.
You're gonna go blind staring into that box all day.
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?
Magic numbers are nontrivial numbers hard coded inmiddle of the code.
Originally posted by KevinHall:
Anyone else with ideas on what other books should Panther read?
"The C++ Programming Language", 3rd Ed., by Bjarne Stroustrup
"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.
Gabriel, CodeGuru moderator
Forever trusting who we are
And nothing else matters - Metallica
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.
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.
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.
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;
}
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...
Well...since we are using C++ we should get rid of the '#define' and use a constant with proper type information instead...[/code]
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.
* The Best Reasons to Target Windows 8
Learn some of the best reasons why you should seriously consider bringing your Android mobile development expertise to bear on the Windows 8 platform.
Reply With Quote
Hi
