New answers tagged beginner
0
Review
You should take advantage of the bi-directional nature of a doubly linked list. It's a pitty to let it use navigation of a normal linked list just for the sake of enabling inheritance. And why should a normal linked list be able to branch between normal and doubly mode? This is a code smell.
A practical way to create a doubly linked list, is to ...
1
First this is probably C++ rather than C, the C programming language does not support function overloading (blink as an example).
Magic Numbers
In programming a Magic Number is a numeric constant in code that doesn't identify what is is. Magic numbers make the code harder to understand, debug and maintain. When magic numbers are used as a basis for ...
2
You wrote several for loops without any spaces.
Please don't do that, as it makes the code harder to read.
Instead of
for(;*colors;colors++) {
blink(*colors);
}
you might prefer
while (*colors) {
blink(*colors++);
}
Moving loop() to the bottom of the file would save you
some forward declarations.
Your Serial.print() statements might ...
0
Just a heads up!
You can't re-declare an Id. They have to be unique. Use Classes instead.
Css Units that have A value of 0 do not need a unit, So you can remove the px part.
for file src's it should use / instead of \ &
background-color can be declared with just background:ChosenColor;
also img tags are self closing so it doesn't need the closing......
1
Structure and flow
Add levels of abstraction
Your overall program structure is hard to read. You have a bunch of different functions calling each other in a loop/chain, which means you have to look at all of the code just to understand the basic sequence of events.
One way to fix this problem is to add a main() function that defines the core sequence of ...
1
Good job separating concerns
Your tetris game-board model, piece and UI code is nicely separated in classes, so each one has its own responibility. Well done!
Don't use magic values, use Enum
public Piece(int f)
This switches on a magic value of f. Piece is a excellent candidate for Enum!
public enum Piece { L, J, T, Z, I, ... }
Enum works as class ...
1
In your code you often use the expressions len(m) and len(m[0]). The code would become clearer if you defined proper names for these expressions, like:
func (m Matrix) Rows() int { return len(m) }
func (m Matrix) Cols() int { return len(m[0]) }
This way, you can write zeros := matrix.Zeros(m.Rows(), m.Cols()), which make the code operate in mathematical ...
2
I think the most important things to which you should pay attention are:
Avoid code duplication. Every time you find yourself writing the same or similar code even twice, think about using loops (hint: your load method) or extracting the code into a function (hint: dead method). The two most evident out of many existing reasons to avoid code duplication: ...
3
Your code is quite short, which is good. On the other hand, it is equally unreadable as short, which makes it worse.
You chose really bad variable names. Most of them are one-letter variables and do not tell the reader anything about what they contain or what their purpose is.
Your code looks inconsistent. Sometimes you write a space around operators, like ...
2
I would use a Closure to avoid excessive memory usage and allow for reusability of the method. Try also to adhere to styling and naming conventions. I don't mind variable names a, i, j, ... Make sure you document them well. Replace var with respectively let and const.
Fiddle
(function() {
"use strict";
function toLongNumber() {
return ...
1
The following code with revised variable names and less coding
function NumToWordsInt(NumIn) {
//---------------------------------------
//Convert Integer Number to English Words
//Using a Loop String Triplets
//Mohsen Alyafei 10 July 2019
//Call for whole and for fractional parts
//---------------------------------------
if (NumIn==0) return "Zero";
...
4
Most of your variables should be static readonly
There is no reason to new up a brand new copy of the arrays of characters each time you run this function. Instead:
public static readonly ReadOnlyCollection<char> AllowedLower = ReadOnlyCollection<char>("abcdefghijklmnopqrstuvwxyz");
public static readonly ReadOnlyCollection<char> ...
1
In your game, which I've played, you have 3 main warriors: knight, mage, and healer. These warriors all have similar behaviors, health, attacks, and heals - they are essentially objects of a class, Warrior.
Tip 1: Let's create a Warrior class:
You will be able to create new Warriors (ie Archers, Brutes, Zombies) later with ease.
You can interface your AI ...
7
Fully Refactored Code: see this Gist
Example Usage:
static void Main(string[] args) {
var generator = new PasswordGenerator(new PasswordGeneratorOptions {
MinimumNumberOfLowerCaseCharacters = 1,
MinimumNumberOfNumericCharacters = 1,
MinimumNumberOfUpperCaseCharacters = 1,
OutputLength = 8,
SpecialCharacters = new ...
8
When calling your method like this:
for (int i = 0; i < 10; i++)
{
Console.WriteLine(GenerateSimplePassword());
}
I get this result:
88yuQg7H
u5UqU36o
u5UqU36o
u5UqU36o
u5UqU36o
u5UqU36o
u5UqU36o
u5UqU36o
u5UqU36o
u5UqU36o
which is not so random.
The reason is the Random object being instantiated ...
2
This is going to be relatively long, but I don't have a TL;DR section
if debug: print
Whenever you see code repeated like this, it's quite often you can refactor it into either a function, or there's a builtin to support it. Fortunately, the logging module makes this quite simple. You do lose a bit of speed over the if statement, but it's a much easier ...
0
package com.company;
import java.util.ArrayList;
import java.util.List;
public class Main {
static void palindromesChecker(String inpPalindromeNo) {
String s = inpPalindromeNo;
boolean flag = true;
List<String> revCombination = new ArrayList<>();
String[] arrSplit = s.split("");
for (int i = arrSplit....
-1
The problem with your code is that you reset the number of factors counter count, every time you get a new factor. Instead, use the following code where you reset the number of factors counter at the beginning of each function call:
n = int(input("Enter a positive integer n: "))
def factor(e):
count = 0 # number of factors
for a in range(1, n+1):
...
1
The logic can be moved out to an external process, if you're able to install jq (command-line JSON parsing tool).
Command: apt install jq
Then the code can be reduced to just:
import os
resp = os.popen('lsblk -J | jq -c .').readline().strip()
4
You have a memory leak. Well several.
All of the static char * functions allocate memory using malloc() which is never freed.
Now let's take a look at how they are used, for example
strcpy(sweepSpeed, getSweepSpeed(currentLine, i));
So getSweepSpeed() is returning a pointer to a string, which is immediately copied into another string sweepSpeed, then the ...
6
Functions should be very short: as a maximum, 2 or 3 screens (considering a screen size of 24 lines), but much less if possible; and shouldn't indent more than 2 or 3 levels of indentation normally. You should try to break that big fat function into a lot of small functions.
0
View it as a matrix; meaning, X can go from 0 to 2, and so can Y - free your mind of the actual content. It is likely easier to build a new array and copy values into it using loops on X and Y positions.
Assuming you view mat[][] as mat[X][Y], you filled it as mat[left-to-right][top-to-bottom]. In order to perform what you consider as a 90-degree ...
12
These declarations are not prototypes:
void getDataFromRapidSrcFile();
int getTotalPositions();
These declare functions that can be called with any number of arguments. It appears that they should take no arguments; we indicate that like this:
void getDataFromRapidSrcFile(void);
int getTotalPositions(void);
It's a good idea to make the same change ...
5
Regarding:
char* speed = malloc(sizeof(char) * 6);
The expression sizeof(char) is defined in the C standard as 1. Multiplying anything by 1 has no effect and just clutters the code, making it more difficult to understand, debug, etc.
When calling any of the heap allocation functions: malloc() calloc() realloc(), always check (!=NULL) the returned value ...
2
Requirements?
Please state a short bit about what the requirements are, so we can more easily check how your code might be improved accordingly.
Java Naming Conventions
Please stick to the Java Naming Conventions! (Classes with capital, variables in lowerCamelCase, etc)
Don't use String to store information
Prefer using a custom class to store ...
-3
Player vs computer
import random
player_name = input("Enter your name:")
player = input("Do you want to play rock ,paper or scissors?:")
computer = ("rock", "paper", "scissors")
def comparing(player, computer):
computer = random.choice(computer)
print("Computer played: ", computer)
if player == computer:
return "It is a tie"
...
2
Apart from that I don't quite get the point of your game, there are a few things that caught my eye:
1. User input
Your user input system is not particularly intuitive to use. Without the source code the user has virtually no way to see what inputs are acceptable at the moment or what your game does.
2. Documentation
Your functions are documented like this ...
2
This already has a good answer, but I noticed some other things, that you should be aware of.
save_to_file
input_file_path is never used
In simple loops it can be a valid choice to use single-letter names for iteration items. The usage of a, b, c here however is confusing, as it is not clear, that they are what you intend them to be, especially since the ...
0
With unsanitized user input, re.compile(char) is dangerous. You should use re.compile(re.escape(char)), which will allow you to strip the asterisks from "***Winner***", instead of crashing with an invalid regular expression.
See also this question and related answers for a different interpretation of the question’s intent for stripping other characters.
3
Initially, I thought maybe the process is slower as it uses two memory containers, as opposed to simply comparing two halves of a single string.
I think you hit on an excellent idea right there. I'd read a line of input into a string, the compare the first half of the string to the second half in reverse order.
You can use std::getline to read the string.
...
1
Pre-compilation
In theory, you're going to want to call this function more than once. That means that you only want to pay the cost of regex compilation once, and you should move your re.compile calls out of the function, setting your regex variables in the module's global scope.
Type-hinting
s is s: str, and char is (I think) also char: str.
'Removed ...
4
Performance
The problem was there were a large number of PDF files (over 1000) which were scattered over a drive.
This is the perfect scenario for a parallel application. Spin up a few workers and have them run through the files, perhaps sorted by size and evenly distributed so that the workload is also evenly distributed.
File paths
input_file_path = ...
11
While it is not quite definitive, it looks like you use using namespace std;.
That namespace is not designed for wholesale inclusion, being vast and subject to change at the whim of the implementation, aside from providing what is standardised.
Read "Why is “using namespace std” considered bad practice?" for more detail.
Synchronizing C++ iostreams with C ...
1
Do not forget to close the scanner
No need for obvious comments, try to comment why not what, we understand what already
Why do you have the empty else; ?
Be consistent using println, no need for + "\n"
Make use of printf
Use loops or methods instead of copy / paste
Your last condition is always true, add tests to your apps
if (bottles == 1) but no sad face ...
4
First off, this is quite good for beginner code. You'll do well if you keep the good habits you've started following.
A few critiques:
class Card : IComparable<Card>
Do you intend this class to be subclassed? If not, seal it.
Are cards comparable by reference? That is, suppose we have two different instances of the queen of spades. Is it possible ...
4
I see two improvement points in the code.
It is better to use getLine() and store the input in char* instead of reading each char and appending to a stack
It is more than enough to iterate till half of the string as the remaining half is checked in the first half iteration cstack.top() != cqueue.front()
2
Splitting the input, parsing the parts, figuring the bits and filling the hosts list should all be seperate methods that return the results to main.
Every method should perform a simple task. If you list the steps necessary to perform an action, each step should be a separate method.
Commenting a methods purpose above it will help you and others understand ...
0
And here is an example of approaching it from the "other" side.
Algorithm here would be:
Input: binary numbers: [1, 0], [1, 1, 1]
Output: Sum of inputs as binary number: [1, 0, 1]
Algorithm:
Convert binary to base 10 digit, ex: [1, 1, 1] -> 7, [1, 0] -> 2
Add them: 7 + 2 -> 9
Convert to binary: [1, 0, 0, 1]
Implementation:
def binary_sum(*numbers):
...
1
getHeight and getWidth should be const.
int getWidth() const { return width; }
int getHeight() const { return height; }
Your destructor does not properly delete the pixels pointer. Since you allocate it with new[] you need to use delete[].
delete [] pixels;
You're storing your textures in column-major order. Depending on how you access them, this can ...
8
I'm submitting this as a different answer because it goes in a different direction from my previous one, eliminating the bool cast as well as individual functions. You can simply define a tuple of regular expressions and apply all.
rexes = ('[A-Z]', '[a-z]', '[0-9]')
# ...
if len(password) >= 8 and all(re.search(r, password) for r in rexes)):
print('...
4
Firstly: what @l0b0 said. This is fundamentally misguided, and you're better off doing an actual entropy measurement.
As for your check functions, you can rewrite them as:
def uppercase_check(password: str) -> bool:
return bool(re.search('[A-Z]', password))
Note that it uses proper type-hinting, and doesn't need an if.
2
Use standard docstrings
This comment:
to get input from user
is best placed in a docstring:
def get_input():
"""
get input from user
"""
Consider using type hints
You're best to google this, because there's a wealth of information about it, but as an example: the idx argument would be idx: int.
Operator precedence
(2 * idx) + 1
doesn't ...
5
Just a few things not already mentioned by other answers:
Are you sure that you will need DrawRandomCard() ? How many card games have you played where you just draw from a random position in the deck? The only times I can think of this is useful is when you do a magic trick, which doesn't really work the same in code as in real life :) Normally in card ...
5
Encapsulation and Mutability
Your Card class is a perfect candidate for being immutable: make Kind and Suit readonly fields or getter-only properties (prefer the later in most cases):
public Kind Kind { get; }
public Suit Suit { get; }
You should also hide away the Deck list in CardDeck: making it readonly and private (or protected, if you are happy to ...
7
enum Kind
{
...
Queen,
King,
Joker // ???
}
Jokers is the joker. You're not considering jokers
class Card : IComparable<Card>
{
...
I'm not convinced that there is a default comparison for cards? It could be misunderstood by consumers. On the other hand a deck of cards is always sorted by Suit and then Kind, but the ...
5
List<T>
You have just learned about List<T> and are eager to use it. Perfectly understandable, but think about what it is designed for and what alternatives are available for this use case. Here is an overview of common Collections in C#
To be fair, there is no common collection available designed for a deck of cards. What you need, is ...
2
You preform multiple function calls which are all very related. You can cram each of the regex checks into one if statement.
You should use a main guard to ensure that the code is only running if this file is the main one, so that any programs importing this file don't have any issues.
You should also use docstrings. These allow potential documentation to ...
5
First, please note that this security measure has been thoroughly debunked (although the reasons why are complex and psychological). Any code resulting from this exercise is therefore by the standards of the security community following an anti-pattern, and should not be used no matter how polished.
That said:
Rather than the
if something:
return True
...
3
Consider a functional approach
I have been coding Java for a long time before I started with Javascript so I understand the urge to write Object Oriented code, create classes etc. However, why don't you try a functional approach for a change. I bet that once you get the hang of it, you will not want to go back. I know I don't.
Javascript already had some ...
3
timing
Put the code to time the run time of the function outside of the function
augmented assignment
value = (value // i) can be expressed as value //= i
divisors
Why do you need to keep a list of divisors. They only as for the largest one. The largest one will always be the last one, so you just need to remember this
pep-8
your variable names are ...
Top 50 recent answers are included
