We’re rewarding the question askers & reputations are being recalculated! Read more.
64

The code leaves a bad first impression because of how it is formatted. In particular, the extra whitespace between the return type and the function name, and that between the variable's type and its name, looks odd. Unless you are writing code that needs to be compatible with pre-C99 compilers, you should break the habit of declaring variables at the top of ...


37

An alternative route is to use string.replace() and Regular Expressions to strip everything but the vowels from the string. Then count the length of the resulting string. This avoids iteration altogether. const vowelCount = s => s.replace(/[^aeiou]/gi, '').length console.log(vowelCount('The quick brown fox jumps over the lazy dog')) The regex ...


33

This is improper in a few ways: std::string *weekDays = new std::string[7] {"Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"}; Not only is this a global variable (no const), but it's being dynamically allocated without being deallocated with delete somewhere. Plus, it can be deallocated anywhere, and possibly more than once if ...


31

There are several small things that you can improve: double weight = 0; works fine, but double weight = 0.0; would be more pedantic since weight is a double. By the way, the following line is a good example to illustrate the benefits of literals pedantry: double volume = ((4/3) * M_PI * pow(obj.radius, 3)); Here, 4/3 performs an integer division and not a ...


30

Roughly 50% of all programming is about dealing with errors. You say your code works, but it doesn't handle errors and therefore doesn't work. Example 1: How many feet of steel was received today: four Program doesn't say "Error, try again". Instead it displays all prompts without waiting for any user input and exits. Example 2: How many feet of steel ...


29

Commenting every single line is excessive by most programmers' norms. There is a coding style called literate programming that heavily emphasizes comments, but even in literate programming, commenting every line would be frowned upon. The most important kind of comment in Python is the docstring, and you didn't write any. Many of your comments just repeat ...


28

First and foremost: Anyway, should performance be always the top priority? Definitely not. Don't write things that are needlessly unperformant, but only be overly concerned with performance when you have a reason to be. Most of the time JS execution time is irrelevant, because you're waiting for user interaction, or for a network request, or something ...


26

I'm just wondering if there's an easier way to solve this problem without the big blocky code in the if statement. Well, you could put all of those in an array: const vowels = ['A', 'E', 'I', 'O', 'U', 'a', 'e', 'i', 'o', 'u']; And then the if condition can be simplified using Array.prototype.includes(): if( vowels.includes(v)) { The fact that I have ...


25

To measure the time taken, you should use System.nanoTime. That should only be used for "stopwatch" type operations - never for taking the "current system time" in a wall-clock way. Conversely, System.currentTimeMillis should not be used for "stopwatch" type operations, as it can be affected by system clock changes etc. From the docs for nanoTime: This ...


25

I'm afraid you aren't actually reading the entire file into either Set. In fact, you're quite lucky this code isn't throwing an IOException. while(in.readLine() != null) { String line = in.readLine(); set.add(line); } This loop, present in both fileToHashSet and fileToTreeSet, is actually reading two lines every iteration. You're only placing every ...


22

You don't need fancy logic. Good old closure is good enough here. function addg ( left ) { if ( left === undefined ) return undefined; return function addg_chain ( right ) { return right === undefined ? left : ( left += right, addg_chain ); }; } This simplistic approach creates an execution context, a closure, on the first call, with the ...


22

Error handling STR(const char* ptr): sz(strlen(ptr)+1),m_data(new (std::nothrow) char[sz]) { if (m_data) { memcpy(m_data, ptr, sz); std::cout << "STR() - "<< getData() << ":" << getSize() << " chars" << '\n'; } else { std::cerr << "...


21

Some minor changes I would recommend: Replace the magic numbers with symbolic constants. It will be easier to modify your program when these rates change. constexpr long double ANNUAL_MORTGAGE_COST = 0.03; constexpr long double LOAN_PRINCIPAL_PLUS = 0.06; constexpr long double MARGINAL_TAX_RATE = 0.35; Instead of doing the calculations inside main() I ...


21

I'll just focus on getMonthValue(): int getMonthValue(const size_t month, const size_t year) { switch (month) { case January: if (isLeapYear(year)) return 6; case October: return 0; case May: return 1; … The month and year parameters aren't sizes. Don't use size_t when you just mean ...


21

Overall, this is really well done. You've missed the usual traps of using magic numbers, not creating structures for related items, and other common things. So nice work! I think it could be improved with the following changes. Types vs. Variables You've created a type for Location which is great. Looking at the types for Player, Trap, Bandit, and Treasure,...


20

This looks pretty decent for a beginner. Some suggestions: Making a variable space instead of using the literal directly is actually a kind of nice idea. You can improve the code by marking the local const. Why is pos an integer? it only has two values, and you are using it to test a condition. Use a bool instead. For that matter, why have pos in the first ...


20

It looks good and readable IMHO. For brownie points, you could: modify the structure a bit. Not every LinkedList will contain books. Similarly, not every Node in a LinkedList will have an AuthorName. You could define 4 separate classes : Node, Book, LinkedList, and Bookstore(LinkedList). define a data attribute for Node. define __str__ for Node, which just ...


17

A few thoughts which come to my mind, without having actually profiled your code: Try passing std::string as reference-to-const to avoid a copy (in case your std::string implementation is not Copy-On-Write). Reserve space in the std::string by calling reserve. Avoid calling std::string::length repeatedly, memorize the value. Avoid indexing the string ...


17

Try not to use using namespace std and system("PAUSE"). The former is okay for shorter programs or toy programs, or when used in local scope as opposed to global (such as within a function). The latter is best avoided as it is problematic while safer and more portable alternatives are in existence, such as std::cin.get(). It's also non-portable and only ...


17

Just a quick glance. But: std::cout << XXXX while(!(std::cin >> age)) { std::cin.clear(); std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); std::cout << "Invalid input. Please enter a number: "; } That could be a function. This would reduce code and make the application easier to read. Remember code ...


17

Bugs You calculate the volume of the sphere incorrectly! Style You misspelled the variable bouyantForce. The constant waterBuoyancy is a misnomer. It should be waterDensity. There's not much point to struct Sphere. You haven't defined any methods in it, nor do you use it to facilitate parameter passing. You might as well have two independent variables ...


17

I see some things that may help you improve your code. Don't abuse using namespace std Putting using namespace std within your program is generally a bad habit that you'd do well to avoid. The comment in the code seems to indicate you know this and can't change it for some reason, but others who may read this question and answer may benefit Simplify ...


16

I'm not sure how far you are in your class, but here are a few notes I didn't see in other answers: It is possible your CalculateWhistles() method could fail the if condition tests and reach the end of the non-void method. Your methods should ALWAYS have a return statement for every case, even if you have to return nothing. For example, what will your ...


16

There are two huge improvements you can make to increase readability of your code: Choose better names for some variables – skill1 is better than skl1, and strength2 is better than str2 (str usually means “string”, which adds even more confusion). play1 should be play or play_on. Put whitespace around operators rather than trying to write your code as ...


16

char again = '\0'; No need for setting a value. You overwrite it before using anyway. do { // ... if ('\n' == again) again = 'n'; } while (again == 'y'); The if here seems pointless. If again is not 'y', you will exit the loop anyway, so setting again = 'n' seems pointless, together with the if. Although you created a struct for Sphere, it doesn'...


16

That's an awesome little game! User experience Before we dive into the code, let's talk about the game itself. Random hangs Sometimes, the executable hangs when I launch it. But only sometimes. It's almost as if it happens randomly! We might find the source of this bug later. What am I supposed to do? When presented with this: ........... ........... ....


16

Simple does not mean fast, so you cannot judge performance based on how simple the implementation looks. Usually the most efficient way to perform a non-trivial task is not also the simplest way to do it. In this case though, there is a much more efficient solution that is about equally simple, and is probably sufficient. There is a serious problem with ...


15

_maxSpeed and _horsePower should be private. You have (confusingly named) setter methods for public variables, but no getters. That makes for an odd interface. In C# you by convention use properties for getters and setters. So instead of public int _maxSpeed = 0; public int _horsepower = 0; public void maxSpeed(int newmaxSpeed) { _maxSpeed = ...


15

Things you could improve: Compilation: I am led to believe that you are compiling your code as C++ code. tmp = (struct node *)malloc(sizeof(struct node)); That line right there would be unacceptable when compiling as C code. And considering that your question originally had c++ on it, I am further convinced that you are compiling your C code as C++ code....


15

bool isLeapYear(const int year) { if (0 == (year % 400) || (0 == (year % 4) && (year % 100))) return true; else return false; } This is nitpicky, but this is a bit unreadable to me. The inconsistent use of parentheses makes me think hard about order of evaluation. Also, you use 0 == to express a false value, but you don't use 0 != to ...


Only top voted, non community-wiki answers of a minimum length are eligible