New answers tagged beginner
1
Don't Ignore Warning Messages
Please compile using the -Wall compiler switch that provides all warning messages.
This code in Board.h generates a warning message:
static enum Tile {
OPEN = 1,
WALL = 2,
SNAKE = 3,
FOOD = 4
};
The keyword static is ignored in this case, why are you trying to make an enum static?
Style and How it Effects ...
1
If you've only learned VBA a week ago, then congratulations and a good job getting a working solution! Hopefully the information and tips here can provide some help in refining not only your solution, but to help guide your code in the future.
You've avoided some basic traps because you're already using Option Explicit and you are assigning a type to each ...
2
I only got to the core classes, not the game implementation. Hopefully this will give you something to think about.
General
Classes that are not designed for extension should be explicitly marked as final. Variables that will not be reassigned should also be marked as final. This makes it easier for readers of your code.
I'm assuming this API is designed ...
1
I would use regular expressions to make a simple calculator that can only perform one binary operation. There is less overhead with tokenizing and casting. If you do decide to keep making calculator more and more complex, I suggest using the lexer-parser approach instead.
using System;
using System.Linq;
using System.Text;
using System.Collections.Generic;
...
3
Trying to avoid duplicating any comments you have already had
Why name the class "ST"? It's not very clear to another developer what a class called ST actually does
Why extend EQ? Generally inheritance is use to make another class that provides the same functionality but more specialised, e.g. A "Car" might extend a class "Vehicle". I think it would be a ...
0
This is all I can think of:
Option Explicit
Sub Analysis_ClientRating()
Dim lastrow As Long, i As Long, rowppt As Long, rowppt1 As Long, _
rowppt2 As Long, rowppt3 As Long
lastrow = ShNote.Range("C" & ShNote.Rows.Count).End(xlUp).Row 'you need to qualify the Rows.count
With shPPT 'you can use it so there is no need to write the sheet in ...
1
There is a lot of things done right in your program. The code is structured into functions, there are docstrings, you validate user input and you even have a main guard. Still there is room for improvement.
code duplication
This is a minor issue in your code but I do not want to skip it. In
def print_header():
print("-" * 40)
print("Welcome to the ...
2
Much of this review is about style. Style's important, especially in
joint projects. It's best if style matches what the community has come
to expect.
That said, it'd be natural to start off with an unconstrained array
type for Set:
type Set is array (Natural range <>) of Integer;
Naming is important. The function doesn't find a subset (clue: it
...
8
Your program is definitely improved over the last version. Good job! Here are some ideas for you about further improvements.
Make the object interface easy for the user
The app object has two public functions, start and end that are apparently intended to be called in that order. To me, it would make more sense to eliminate end and simply move the ...
1
The way to move things is extremely restricted. Instead of marking entities moveable (mobile?), encapsulate the behaviour of mobility behind an interface and inject a suitable implementation to each entity. When the time comes to move an entity, whoever is responsible for triggering it, fetch the mobility implementation from the entity and have it perform ...
4
Here are some things that may help you improve your code.
Don't declare enum const
In snake.h, the direction enum is declared as const but this is an error, since only functions and objects can be declared const.
Use const where practical
The Snake::getSnakeSize() doesn't alter the underlying Snake and so it should be declared const. Additionally, I'd ...
1
Style
Python comes with an official Style Guide often just called PEP8, and especially as a beginnger it's a good starting point to get going. Of course coding style is often a matter of choice, however, there are aspects you should definitely follow.
Whitespace
As you know, Python's code structure is build upon indentation, so some parts are already ...
5
Side note: this is a code review site, so I'm going to at least start by reviewing your actual code. I might try to add a few points about what you tried to investigate later, but for now this is mostly just about the code itself.
Don't Repeat Yourself
Right now, your code contains main repetitions of nearly identical code:
begin = clock();
for (int i = 0;...
2
Some observations:
Comments:
// Draws the objects
void drawWindow();
// Handles events
void handleEvents();
// Updates the window
void updateWindow();
These comments don t tell you anything new the code doesnt already
tell you so they should be removed. Ideally the code is well named
like in that example so you dont feel the need to ...
4
(setq letters (get-letters s))
You are setting the same letters variable from different functions; in Emacs Lisp, that sets a global variable which is bad style: (1) that pollutes the global namespace, (2) that could overwrite an existing variable, and (3) the functions are not reentrant and could lead to bad suprises with side-effects. Better introduce ...
2
General feedback
For a beginner the code is okay - for the most part it appears to function like a calculator, except for the fact that if I click the sequence4, +, C, 5, = then the result shows 5, whereas with your calculator I see 9 because clearing the form via the reset/cancel button (i.e. C) doesn't clear the value in the variable x.
This code is ...
2
In PrintData method you are assigning values to TaskData object but you don't seem to do anything with it besides just printing it out.
I would parse the file into a List at the beginning and then work with this collection in later code.
private static List<TaskData> ParseTaskData(string[] allData)
{
var tasksData = new List<TaskData>();
...
1
Performance
You create a new Scanner for every guess. this is unnecessary. the same scanner can be used to read all user input. the scanner should be closed only at the end of the program. and if you adopt that, you can also use Java 7 try-with-resources feature. it is a safe(r) way to open and process external resources.
JDK library features
in order to ...
2
A few minor things:
Imports should generally be at the module level, not inside functions.
Use more intention-revealing variable names. no_of_guess is borderline OK, but a, b and c don't tell me anything.
Think about control flow more carefully. For example:
you increment the guess counter for any numerical input, so you don't necessarily need to wait ...
1
I just spotted some random things not mentioned yet.
Make the wait portable
Consider using a modern portable way to replace
#include <windows.h>
Sleep(1000);
with
#include <chrono>
#include <thread>
std::this_thread::sleep_for(std::chrono::milliseconds{ 1000 });
This approach works on all platforms were c++11 is available.
...
0
it is a poor programming practice to include header files those contents are not used.
I.E. in the posted code, the contents of the header file: string.h are not being used. Suggest removing the statement:
#include <string.h>
5
At the end of their answer, @AJNeufeld has the right idea to improve the algorithm even further (in pure Python). You want to minimize the amount of numbers you have to mark off as being composite for every new prime you find. In order to do this, you can actually start at current_prime * current_prime, since you should already have marked up all smaller ...
5
AlexV has made some great points. I’ll try not to duplicate them.
while numbers[current_prime] <= sqrt_limit:
This seems like odd code. numbers[x] == x is true, or numbers[x] == -1 is true. You should save the indirect lookup in the numbers array, and just loop:
while current_prime <= sqrt_limit:
You take great pains to explain in a comment that ...
8
Four* letters to rule them all
The Sieve of Eratosthenes is an algorithm which heavily relies on loops. Unfortunately, Python's convenient scripty nature comes at a cost: it's not terribly fast when it comes to loops, so it's best to avoid them.
However, this is not always possible, or one, like me in this case, is not so much into algorithms to transform ...
-1
In reality, people just use third-party services for that. For instance, uptime website monitoring can be done StatusCake. If you own the target site, you install infrastructure tools like New Relic. Cloud services these days also provide uptime data and tools out of the box.
The only reason I would write my own uptime monitor is if you're doing this for a ...
3
You can get the value for ASCII_POS_OF_0 easily by using '0'.
Your code doesn't deal with malformed strings well.
If the string is only "123" it will attempt to read beyond the contents of the string. You can fix this by checking for '\0' at the correct places.
It also doesn't handle any type of whitespace in the string at all.
7
In general, placing constraints on your passwords, such as that they have x-many numerals and y-many letters, makes your password generation scheme slightly worse; there's no advantage.
But sometimes we're required to do silly things.
In this situation, it looks like the requirement is to have at least four of each, but that could change, as could the ...
7
Your passwords will be biased, with more letters appearing towards the front of the password and more digits towards the end.
A better approach would be to determine how many digits you will have in the password. Then, for each character, determine if it should be a digit based on the number of digits you want to have and the number of characters left to ...
1
Oh deer. Oh deer. oh deer.
Here you go friend, have a look at this:
https://jsfiddle.net/d3o2Lex4/56/
Should be easy enough to understand. One thing to keep in mind when making things responsive is using % for widths. You will also have to learn the css @media query to make things adapt at different screen sizes. That's how Bootstrap works. If you haven't ...
0
Don't make your Class members private if you offer public getters and setters without any functional logic. Just make your members public. This saves you 70 (i.e. 7*2*5) lines if I counted correctly. It makes your class more readable, the access of the variables less complicated and saves Bytecode.
0
Let me know if you ever see an Android activity class that is not spaghetti. I'm eager to learn if it is possible...
When UI code and business logic is combined into one class, the code becomes harder to read as the reader constantly needs to remember whether a field is related to UI or business logic. The code related to maintaining the payroll-related ...
5
A Python solution can be much more succinct than what you wrote.
Reading the file: The file is a degenerate CSV file with double-quoted values. You can use the csv module to split on commas and discard the double-quotes.
Opening the file: You can use the fileinput module to avoid hard-coding the filename. The program will either open the filename given as ...
5
Common File Names
There are 2 generally accepted extensions for C++ header files, they are .h and .hpp. A professional programmer will stick to the common extensions so that others can maintain his code.
Many editing tools will create these files for you, for instance in Visual Studio you can select Add from the solution explorer and then select class. ...
0
I'd like to add that a very important feature of C++ is templates. You can parameterize your vector by element types.
Example:
template <typename T>
struct Vector3 {
T x, y, z;
};
There are other suggestions here, they include:
Parameterize vector/matrix structs by element type and dimension.
Provide specializations and typedefs for the most ...
4
I am currently using the RK-4 method to update position
Firstly, the naming is unhelpful here. A name like RK4_1 implies that the method does the quadrature. I think that function is really dx_dt, RK4_2 is really dvx_dt, and there would be room to pull out a function like
def RK4_step(t, x, dx_dt, dt):
k1 = dt * dx_dt(x, t)
k2 = dt * dx_dt(x + k1 / ...
7
@Peilonrayz and @Carcigenicate have great feedback you should definitely follow.
Since you are using Python 3 features like f-string, #!/usr/bin/env python3 should be used as shebang line.
5
Your conversion functions have two notable things:
You're printing the result directly. Don't do this. For toy programs like this doing so doesn't create problems. In the real world though, you don't just print data to the screen, you use data. With how you have it now, the caller can't actually use the converted value for anything. What if you wanted to ...
8
You're not following PEP8 and you still have a spaghetti mind-set. Each function should have a single responsibility.
value_input however is in charge of:
Asking and validating user input.
Handling how to convert the input.
Convert and display the input.
This should instead only perform the first task I've said above. After this you should have the ...
3
First, check_replay has a few notable things.
Since you're only using valid_yes and valid_no to check for membership, they would be better as sets which have a much more efficient membership lookup. The performance difference won't be noticeable in this case, but it's a good thing to keep in mind.
The whole purpose of check_replay is to ask if they want to ...
4
One thing I notice is that, you use global variables. While they are not necessarily bad and work fine in this particular case, it might be beneficial to read these answers. It would be better to define these variables in main function and pass them as parameters for other functions.
def start_game(number, counter):
#Your code here
def main():
number = ...
2
In this self-mini-review, let it be clear, I just want to point out several things I do differently now. These were untouched by Toby Speight, so could be beneficial to someone. All of these are opinion-based!
Styling points
I add spaces around parentheses now, like in this case:
bool string_contains_integer ( const std::string & str )
I write ...
3
In your main game loop, rather than call Console.Clear() and repainting all your content (which can cause some flicker), just update the previous frame with any changes. This means that initially you'll want to draw everything, but after that you only need to update stuff when it changes. When the fruit moves, erase the old position (unless it is covered ...
0
Welcome to CodeReview! First, congratulations on writing a working Python program that is clean, straightforward, and easy to understand.
Now let's see if we can make it better!
Style
There are two things I think you could improve stylistically. First, your comments are a little too much. I know people encourage you to write comments. But you need to be ...
7
You include the modern random.
#include <random>
But in your code you use the old srand() and rand() functions. Also your usage of these functions is not correct.
time_t tim = time(NULL);
srand(tim + rand());
fX = rand() % (nFieldWidth - 2) + 1;
fY = rand() % (nFieldHeight - 2) + 1;
Here you are abusing the seeding ...
5
First on my Windows 10 computer in both Visual Studio 2015 and Visual Studio 2017 the console is killed by this line in the update function.
WriteConsoleOutputCharacter(hConsole, buf, nScreenWidth * nScreenHeight, { 0, 0 }, &dwBytesWritten);
This may have to do with settings on my computer.
Second I get this warning message in both VS 2015 and ...
1
In general, prefer to use the narrowest possible scope for variables. Old-school JavaScript is an exception for technical reasons to do with its scope rules being rather unusual and catching out people who were used to other languages: let was introduced to address that problem.
In short, product should be declared inside the loop over j.
Also, given the ...
9
First of all congratulations for this little entertaining console game.
It is simple but entertaining. I felt like I was back in the old mobile games era.
I don't have the time to rewrite all of the code but I still want to give some hints for improvements.
Here are some random observations:
Don't use global variables, they are a maintenance hazard.
...
1
This should be twice as fast:
dlr = wb1.Worksheets("INPUT_DATA").Cells(Rows.Count, 1).End(xlUp).Row
wb1.Worksheets("INPUT_DATA").Range("A2:S" & dlr).ClearFormats
'check duplicate values before import to TOTAL_DATA
Tlr = wb1.Worksheets("TOTAL_DATA").Cells(Rows.Count, 1).End(xlUp).Row
countMatch = 0
countUnmatch = 0
For cell = 2 To dlr
...
0
This is not a full answer, but it should lead you to the right way:
Option Explicit
Sub ImportData()
Dim wb2 As Workbook
Dim ws1 As Worksheet 'you can also reference sheets
Dim ws2 As Worksheet
Dim slr As Long
Dim dlr As Long
Dim Tlr As Long
Dim arrData 'working with arrays is always better
Dim i As Long
Dim ...
10
While there are a lot of things that could be fixed or improved in your program, the biggest problem with it is your use of functions and control flow.
One thing that was not at all obvious to me at first glance is that your program runs in a loop, requesting input and giving conversions until the user is done. Consider what another developer needs to read ...
Top 50 recent answers are included
