29

Two general points about your code: Variable naming: res, res2, res3 are not descriptive, so it's harder to understand at first glance what their purpose is. Instead I would recommend something similiar to contains_digit, contains_lower_char, contains_upper_char. Variable naming is always up to a bit of personal preference, but explicit names are generally ...


18

Prefer C++ headers (such as <cstdio>) to the C compatibility headers (<stdio.h>). The C++ headers define identifiers in the std namespace where we want them. It seems that this header isn't even used here, so we can omit it completely. std::size_t is misspellt throughout as size_t. This commonly happens when writing on a platform that declares ...


17

Here are some observations that may help you improve your code. Don't abuse using namespace std Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. Use more whitespace to enhance readability of the code Instead of crowding things together like this: if(word[i]==word[a-i-1]) most people find it more easily ...


17

Make variables and functions const where appropriate You used const references in a few places, but you missed a lot of opportunities to make other things const. Consider a Book for example. Ideally, once constructed, the title, author and category should not change. So you could make this explicit and mark these member variables as const. This allows the ...


16

Return values def list_to_string(some_list): new_string = '' if not some_list: return '' ... Why are you returning '' here? You could return new_string instead, since you've initialized it. In fact, the last statement in the function is return new_string. Why not make that the only place you return from the function? def list_to_string(...


16

You're on the right track, but it can be simplified to the following. Note also that the step variable name could add confusion because Python ranges and slices have a step attribute -- but it's not what you are doing: start = 3 for x, y in zip(v, v[start:]): ... Also note that syntax like v[start:] creates a new list. If you are dealing with large ...


16

Confusing variable name. Is the tip an amount or percentage? I assumed the former based on variable name, but a commenter has corrected me. Keep calculation and presentation logically separate. You don't need to create a variable for the rounded amount. Just compute the per person cost in the natural way. When you are ready to display that value in a human-...


15

PEP8 The "PEP 8 -- Style Guide for Python Code" has many recommendations on how programs should be written, for maximum understandability between programmers. Variable Names The one that you violate the most relates to variable naming. PEP8 recommends snake_case for all variables. So customerOrder should be customer_order, sandwichTotal should be ...


15

Welcome to C++, and welcome to Code Review. C++ memory management is, as you probably have realized, tough and error-prone. There are many things that can easily go wrong. Assuming that no exception is thrown, I don't see obvious memory leaks in your code; however, there are still some issues worth discussing. You can take a look at my implementation of a ...


15

Avoid excessive memory usage At each step of the recursion, you allocate two arrays that together are as large as the input. That means that your algorithm uses \$\mathcal{O}(N \log N)\$ space. It should be possible to rewrite your code so you only use \$\mathcal{O}(N)\$ space without substantially changing the algorithm. But this also brings me to: In-place ...


15

void main is a really bad habit. It must be int main. Always check what scanf returns. For example, try to enter a non-numeric input, and see your program entering the infinite loop. Avoid conio.h (and hence getch). It is very non-portable.


15

One of the easiest ways to improve code is to reduce repetition. The most-repeated elements in your code are random.choice and outputpass. You can reduce bulk by importing what you need rather than importing a module containing what you need. from random import choice from string import digits, ascii_letters, punctuation Another bulk-reducer is to choose ...


15

Don't use random.choice() for generating passwords. As the documentation says: Warning: The pseudo-random generators of this module should not be used for security purposes. For security or cryptographic uses, see the secrets module. I'm not sure what you're expecting from these three lines: outputpass = random.choice(digits) outputpass = random.choice(...


14

Choosing good identifiers When I see a name like options I think of something a player can choose from like "single player" (i.e., play against the computer) or "multiplayer". A better name would be board because that's what it is. torf? Does it mean "true or false"? Every Boolean is true or false. This does not reflect the ...


14

The code is longer and more complex than necessary. Here is what it could look like instead: #include <iostream> #include <iomanip> #include <numeric> int main() { // Define the relevant constants static constexpr double cost_per_meter = 2.75; static constexpr double discount = 15.0 / 100.0; static constexpr double ...


14

I assume this your first project of such size, and if so, that's a great work. It's really cool you've done all this. Bad Recursion This is the worst problem in your code. You see, calling the function is not free, it uses space in a call stack. The space is released on function return or program exit; but because you never return from functions like game(), ...


14

There's a bug in the logic: If I select 0% tip, it is quite possible that adding the rounded amounts each person pays don't add up to the bill total. You should probably use ceil instead of round, to ensure you round up, never down. Of course giving a negative tip percentage also makes you not pay the bill total, but that would clearly be user error. It's ...


13

Not much more to add here, but you'll quickly find that this routine is too basic for the need and regular expressions are the way to go. This routine does not guarantee that the resulting password will be balanced, that is sufficiently "random" and hard to guess. For example AAAbbb123 or Abcdef123 will pass your test. These are not strong ...


13

We're missing some required headers: #include <climits> #include <stdexcept> #include <string> #include <gtest/gtest.h> We can reduce the number of division operations here: if ((ULLONG_MAX / i) >= fact) If we use ULLONG_MAX / n instead, we get the exception in the same cases, but this doesn't change during the loop and ...


12

Design review Trying to roll your own shared_ptr is actually a really good practice project. This is not true for many other standard library components, but it is true for shared_ptr. You could try to duplicate the behaviour of std::shared_ptr exactly… but you don’t need to, because there are actually many, many different smart pointer designs. The ones in ...


12

Using itertools.tee to zip an iterable with itself Since you are iterating twice on the same iterable, one possibility is to use itertools.tee: from itertools import tee def zip_offset(iterable, offset): a, b = tee(iterable) for _ in range(offset): next(b, None) return zip(a, b) print(list( zip_offset(range(10), 3) )) # [(0, 3), (1, 4), ...


11

I'm not a C++ developer, and can't comment on the details of the code. But I can comment on the test cases and the general style of the code. A good test should be enough to recreate the code from reading just the test cases. Especially when you get the task to make sure the code works under any circumstance. Successful test cases Let's have a look at your ...


11

Looks good, just a few ideas: Since the function only returns two Series, you can save some overhead by removing the copy() and just working with Series. Combining logical_and() with reduce() allows you to build the shifted Series in a comprehension (and also seems slightly faster than chaining & & &, based on some rough testing). Instead of ...


11

using namespace std; is a bad habit to get into, as it can lead to name-collisions and other issues. It's best to explicitly qualify the names we need where necessary, e.g. std::cout. A few more line-breaks would be useful in the code. Think of code with a specific purpose as a paragraph when writing text. For example, this could be considered one "...


11

Don’t write using namespace std;. You can, however, in a CPP file (not H file) or inside a function put individual using std::string; etc. (See SF.7.) double cost_per_meter = 2.75; Use const where you can! In this case, it should be constexpr since it is a compile-time constant. double length, extra_length, extra_length_cost, discount, ...


11

In addition to AJNeufeld's comprehensive answer there are a couple of additions that you could make to make your function more useful in the future. Parameters At the moment you have: def list_to_string(some_list): ... and you're always joining on a comma and using "and". But in the future you might need to use a semicolon and "or" ...


10

When you have a function where not every possible input value can be mapped to one of the possible output values, you have three options: Allow fewer inputs, allow more outputs, or declare that your function is partial rather than total and just isn't well-behaved sometimes. The first is arguably the most flexible, but often involves some awkward re-shaping ...


10

Disclaimer: I know nothing whatsoever about the rules of cricket. Useless check Because either info['toss_decision'] equals 'field' or it does not, you do not need to check things twice in: if info['toss_decision'] == 'field': first_innings: object = toss_loser elif info['toss_decision'] != 'field': first_innings = info['toss_winner'] which could be ...


10

For a beginner this is actually quite good. For future reference, we can provide better reviews when the entire program is included. This code does not include the Using statements nor does it provide the class declaration. You might also want to consider some of the heuristics of Sudoku, such as solve for the number that appears the most in the puzzle first....


10

Header The header file includes a couple of headers that it doesn't need: #include <fstream> #include <iostream> That said, I would consider including <iosfwd> and declaring functions that work on streams (probably using >> and <<) rather than files, to make unit-testing possible. It also has using namespace std;, which is ...


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