New answers tagged beginner
1
I'd recommend having a look at Python's "official" Style Guide aka PEP 8, which is widely accepted in the community as a guideline on how Python code should be formatted.
But you can also learn a lot from looking at other peoples code, e.g. here on Code Review. E.g. it's very common to have imports at the top of the file, followed by constants, class and ...
4
Recovery from Errors
The safe_malloc() function looks good, however, you could use setjmp and longjmp rather than exit(EXIT_FAILURE) to attempt to recover enough to clean up after errors occur and to only exit the program from main. According to an answer on this stackoverflow question they can also be used for co-routines. The additional information that ...
0
You could build your final string using ternary operators and avoid all the ifs. I think this is much more readable:
String firstChar = str.charAt(0) == 'a' ? "a" : "";
String secondChar = str.charAt(1) == 'b' ? "b" : "";
String remainingChars = str.substring(2, str.length());
String finalString = firstChar + secondChar + remainingChars;
1
A few subjective points...
Isolation
Going forward, you're going to want to isolate the UI from the algorithm as much as possible. So, using methods to break up the algorithm as suggested by @Doi9t is definitely worth considering. I think it's also worth considering making the output stream a class field for the moment. This is going to be closer to the ...
-1
you can try this as well.
public static String deFront(String str) {
if (str.length() >= 2) {
if (str.charAt(0) == 'a' && str.charAt(1) == 'b') {
return str;
} else if (str.charAt(0) == 'a') {
return str.substring(0, 1) + str.substring(2);
} else if (str.charAt(1) == 'b') {
return ...
1
After some research I found that tempfile.NamedTemporaryFile is broken on Windows (you could have a temporary file, or you could get a filename - but you cannot have both).
This is just flat out wrong. Maybe you're misunderstanding something and conflating two things. But this is just factually incorrect. And can be seen in your code:
temp_pptx.close() # ...
2
It seems this could be simplified with a small lookup table and then concatenating strings. Using a table as simple as:
const lookup = {
blue: 'b',
green: 'f',
purple: 'g',
yellow: 'p'
};
we can then lookup the color values and concatenate them together. Combined with a simple check to make sure the color is valid and the 2 colors aren't the same, ...
2
There's a subtle issue with this line in the get_random_card function:
srand(time(NULL));
You're seeding the random number generator with the current time, each time get_random_card is called, which means multiple calls within the same second will have the same sequence of random numbers. In this specific case, it doesn't affect anything (other than a ...
5
This question is tagged C++ but I would say that the code looks more like C with classes.
STL offers you a lot of functionality you could use.
Use STL instead of providing your own implementation
Trust me, folks who wrote the standard library have a lot of experience, it was tested really hard and it's (almost always) more efficient than what you'd write ...
1
Spelling
"Suite" is not the same as "Suit". A suit would be Hearts, Clubs, Spades and Diamonds.
Logic Checking
Why is MAXCARDS 25? In Blackjack, the most cards a player is allowed is 5. Even then, the target is 21; with a standard deck of cards, that would be 4 Twos (8), 3 Threes (9) and 4 Aces (4). If you are planning on playing with more than 1 deck, ...
1
A potential issue is related to the static Scanner object:
private static Scanner scanner = new Scanner(System.in);
This scenario can bring potential problems due to the closing of the Scanner resource left to the developer. From Java 8 it is possible to use the try-with-resources statement to ensure closing of resources (in this case the Scanner object to ...
6
Good first effort. Coming from Java, you'll find memory management in C new and frustrating:
Memory management
You can't just call malloc and directly use the result like this:
Hand player_hand = malloc(sizeof(Card*) * MAX_CARDS);
for (int i = 0; i < MAX_CARDS; i++) {
player_hand[i] = NULL;
}
If malloc() fails, it will return a null pointer, and ...
3
Just a few possible improvements.
Variable Naming
According to PEP 8, variable names, including parameters, should be in snake_case, not camelCase. This will apply to all the improvements made below.
Type Hints
You can use type hints to display what types of parameters are accepted, if any, and what types, if any, are returned by the function. Observe:
...
3
I have some suggestion for you.
1) Constant name should always be uppercase.
private static final Scanner SCANNER = new Scanner(System.in);
2) When creating arrays, I suggest that you use the java style instead of the c style, since it's the more used and less error-prone, in my opinion.
Before
public static void main(String args[]) {
//[...]
}
...
4
There is room for improvement in your code, I'll give a few pointers of things that stand out to me.
DRY (Don't Repeat Yourself)
There is a lot of copied and pasted code in your work. It makes it harder to read and much harder to maintain.
To initialize the board, you manually put in 49 instances of a class with some parameters changing. What if you want ...
2
First off a typical bad habit many beginners get into is, putting everything in main. If the code is only doing one thing and can be self contained a function will do. Otherwise, use a class. In this case I would suggest a class with static fields and functions.
Try to avoid using magic numbers, strings, etc.. Use final variables so that they can be ...
1
I found a little inconsistency in the following part of the code:
// Minuten berechnen
int minutes = seconds / 60;
seconds = seconds - minutes * 60;
System.out.println("Minuten: " + minutes);
In all the other paragraphs you use named constants. You should follow that pattern here as well. MINUTE_IN_SECONDS is a perfect name for the 60.
The following two ...
3
I have some suggestion for you.
1) The naming convention of the variables HOUR_IN_SECONDS, DAY_IN_SECONDS and YEAR_IN_SECONDS is not correct, since they are not constant; missing the static keyword. I suggest that you extract them in the class level.
public static final int HOUR_IN_SECONDS = 60 * 60;
public static final int DAY_IN_SECONDS = HOUR_IN_SECONDS ...
0
There is already a good answer from @Doi9t. I was too slow to publish mine but can still be useful.
"My issue is that each color tee has its own "final" data for course rating and course slope, and this makes the calculation differ". Okay, do you know the the strategy pattern ?
In computer programming, the strategy pattern is a behavioral software design ...
1
First off, the second loop should be a for loop:
for (i = 0; i < len; ++i) /* what i think is unnecessary */
{
s[i] = rev[i]; /* storing back to initial array */
}
But to eliminate that, try:
while(i < l)
{
char tmp = s[i]; /* swapping */
s[i] = s[l]; /* swapping */
s[l] = tmp; /* swapping */
++i;
--l;
}
If there is a true ...
1
Standard C requires that main() return int:
int main(void)
There's no need (for 20 years now) to write all local declarations before the code. Introduce variables as you need them, so they can be initialised immediately:
{
char line[MAXLEN];
int i = 0;
There's no need for the extra storage rev in reverse(). We can reverse the string in place, ...
2
Here is an attempt to leverage Java's BigInteger class to implement the RSA algorithm, as well as md5 and sha512 hashing functions to generate keys for what I hope to be strong cryptography. My prayer is that some of my ideas are novel and not just wrong.
If you start at cryptography, you should not start by making up your own schemes and hoping that they ...
1
Firstly, main() must return an int:
int main(void)
We can make the array initialization easier to read with judicious use of whitespace:
static const int N = 5;
const int pixel_array[5][5] =
{
{1, 3, 6, 10, 15},
{2, 5, 9, 14, 19},
{4, 8, 13, 18, 22},
{7, 12, 17, 21, 24},
{11, 16, 20, 23, 25}
};
Instead of the if/...
4
How can I decrease code length?
You could split your big for loop into 2 smaller ones. The first one would print all diagonals up to the main one, and the other one the remaining ones.
For instance, let's say there's a matrix like:
1 2 3
4 5 6
7 8 9
Then, your first loop would print 1 4 2 7 5 3 and the second one 8 6 9. The code would look conciser and ...
2
I would turn around the logic. The design you have has a lot of the general logic in the main program, and a lot of game-specific logic in the questions module. A better design for reusability would be to put the logic in a module, and the game-specific stuff (basically just the questions and perhaps some very superficial user interface) in the main program.
...
5
I have some suggestions for your code.
1) The input variable is unused.
2) Since the tees variables are hard-coded and have two values, I suggest that you use an Enum to hold the values. It will make the code easier to read and remove the variables from the method.
Tees.java
public enum Tees {
BLACK(146, 74.6F),
BLUE(139, 72.4F),
WHITE(129, ...
2
user_choice is your main loop, and so it would be better described as main. Whilst fairly undescriptive on what it does you can add a docstring to add information on what it performs.
It is more idiomatic to use loops in Python rather than recursion. This is partly due to the recursion limit, and partially that the iterator pattern has a lot of support in ...
1
Conversion of a comment to an answer.
Definitely avoid the recursion in user_choice. It will run out of stack if you keep exercising it for long enough. A simple while True: around the rest of the function body should fix it (obviously take out the calls where it calls itself).
- tripleee
2
It may not be what you are supposed to be learning, but if you use brute force to solve Sudoku, you'll do fine with easy ones (30 clues, asymmetric), but you'll struggle with difficult ones (fewer than 20 clues, symmetrical diagonally) - so why not use a SAT solver library like Google OR Tools to do the solving? Learning to use 3rd party libraries in your ...
0
After reviewing some of the answers I rewrited the code and added some descriptions
This does not work because I have a index error in pandas...
'''
This code intends to do an excel SUMIF between two tables with different indexes.
The first tables has GPS data with timestamp, vehicle ID and distance
The second table has vehicle ID and timestamps of events
...
5
while True:
if solve_check():
break
would be a lot more obvious as:
while not solve_check():
And I'd be tempted to change conditions like this:
if not (row + 1) % GRID_ROWS and counter < 2:
to one of:
if (not (row + 1) % GRID_ROWS) and counter < 2:
if not ((row + 1) % GRID_ROWS and counter < 2):
to make it more obvious which you ...
3
The really big suggestion (which I'm sure you're going to hear a lot) is to not use global variables. Declaring global constants is fine, but when you have state that any function might modify at any time it rapidly gets hard to figure out what state your app is in at any given point.
So with that in mind, your board_update function should not exist. I ...
9
I think you're over-commenting, but you're a student, so your professor probably is requiring more than needed.
I think your code has some pretty big problems even after my answer. Over use of globals, lack of SRP, and I don't think your code works with all Sodoku boards. But my answer is long enough.
In board_filler:
I'd prefer the name create_board, it's ...
2
Working from @JoopEgen answer, i wrote a numpy version that will usualy speed up the whole thing by a huge factor (but since no data are given, i cant test it...)
Well, while doing it, i remarked that you use :
for i in range(len(lista[1])):
...
lista[1][i-1]
which is wierd. I then consider that you intended that the last value will be used as ...
6
The code does not look appetizing, readable.
I reduced the conditionals which indeed brought some structure into the whole:
red = 0
green = 0
black = 0
c1 = listc[1][j]
if c1 <= lista[3][i] or c1 >= lista[2][i]:
if lista[0][i] == listc[0][j]:
c2 = listc[2][i]
if lista[8][i] == 'intended value':
...
7
For me, the problem starts with the nested loops showing no specification of what is to be achieved, not even a suggested abstraction (being the body of a function given a name).
Observations:
the output does seem to depend on the order of elements of lista
(even beyond its order: lista[3][i-1])
• hope lista[2][i] >= listc[1][j] is never True for i 0
...
2
[1] By Java convention curly brackets belong in the same line as the preceding statement/expression.
Instead of this:
if
()
{}
else
{}
for
()
{}
do this:
if () {
...
} else {
...
}
for () {
...
}
I suggest you read the Google Java style guide for more tips: https://google.github.io/styleguide/javaguide.html
[2] You don't have to introduce ...
1
Avoid many duplicate calculations.
The code will be shorter and easy to read.
i.e. in towerMoves
void towerMoves(){
for(int dir = -1; dir < 2; dir+=2){
for(int tx = x+dir; tx>=0 && tx<=7; tx+=dir) {
if(board[y][tx] != 0) {
if(blackWhite[y][tx] != color) {
moves[y][tx] = 3;
...
2
I did not bother to read your test code in detail, as that code looks totally unexpected to me.
In the test for a sudoku solver there must be at least one full example sudoku that is solved and validated.
The test must not depend on external files and it must not run external programs since a sudoku solver gets an incomplete sudoku board as input and ...
0
Using pandas you can do it in much simpler way.
Steps:
1. Load the data in pandas dataframe from your excel sheet - https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.read_excel.html
2. Filter out the columns which you want to keep (you can specify the column names to be loaded while importing in step 1 also)
3. Transform the columns based ...
-3
Being new to C, you probably do not know that there is C++, and moreover, all of such things, that you are looking for, like python's split(), etcetera - already exist and are implemented in the boost library.
#include <boost/algorithm/string.hpp>
using namespace std;
using namespace boost;
std::vector< std::string > ...
5
As an industry professional, here's what immediately comes to my mind:
class SudokuSolver: package private class? Why?
inconsistent spacing (two spaces before the int[][] solveSudoku, sometimes spaces before opening braces, sometimes not, ... - yes, we notice these details!)
Comments repeating the code.
Use of Stack class (see opening comment in API-doc, e....
6
Minor typos / grammar
Note: most IDEs have a spellchecker and will highlight wrongly spelled words
You have an extra space between static and int on line 16. - Most code bases have checkstyle or similair processes to prevent this sort of error, so I wouldn't worry about these sort of things. I just wanted to include it for completeness.
I suggest changing ...
3
First of all we should describe a user-story for your ID-generator. This description should contain use-cases (including examples), requirements, and related technical context.
Use-Story
As creator of a new entity (system) I would like to make sure that this entity can be identified within the system by an Identifier (ID). So the ID should be automatically ...
6
It's hard to make small tweaks to this code to make it do what you want, so I'm going to give some pointers on how to do a wholesale rewrite.
Before you can write a card game, you need to write a data model for a deck of cards. Picking the right data model makes everything else easy (part of why your program was hard to write and is hard to fix that the ...
1
Your original code is simple and straightforward, easy to follow. There are a couple things that might trip you up when maintaining the code and that I would suggest could change.
You have a solid list of Private Const declarations at the top of your code. Abstracting the strings into consts is very good practice, but eventually you'll find that it's even ...
3
Using static methods only for your IDGenerator class is antipattern. It's better to make those instance methods and create instance of your class when you need it. One reason may be extendability, another can be thread safety. I have a feeling, that static local variable lastTicketDate will probably cause trouble with race conditions.
If you really want to ...
1
Using plain addition is going to create identical ID's. For instance, if for 1 ID the dayIDValue is 6 and the monthIDValue is 5, you'll get the same ID if next time the dayIDValue is 5 and the monthIDValue is 6.
I would suggest using the UUID class. It allows you to choose different versions depending on how unique you need the ID's to be.
2
User input shouldn't require the full case-sensitive text.
If there isn't already one in a library somewhere, write a function that compares two strings to see if one is a valid truncation of the other.
Then use that function rather than != for comparing the response to the flip.
E.g. any of these should match "True": "t", "tr", "tru", and "true", ...
11
After getting user input, you check it by doing decision == "Yes". This is pretty exact text to expect from the user. I'd at least upper-case their input, and unless you really needed them to be specific, only keep the first letter:
decision = input("You guessed wrong, do you want to continue? Yes or No ")
std_decision = decision[:1].upper() # [:1] ...
Top 50 recent answers are included
