New answers tagged beginner
0
This code needs compiling with more warnings enabled:
gcc -std=c17 -fPIC -g -Wall -Wextra -Wwrite-strings -Wno-parentheses -Wpedantic -Warray-bounds -Wstrict-prototypes -Wconversion 254841.c -o 254841
254841.c: In function ‘main’:
254841.c:14:13: warning: conversion from ‘int’ to ‘float’ may change value [-Wconversion]
money = dollars_to_cents(...
3
The code can be structured better, so the code is more self-documenting.
Self-documenting code doesn't need as many comments, which makes it easier on everyone. Comments that aren't there can't be obsolete either.
Take input (height)
Output pyramid
Those are the only lines I want to see in main. The rest should be in functions.
You started out nicely by ...
-1
printf is overkill for printing a single character - you could use putc instead.
However, printf provides some useful facilities that we can use to our advantage. Look at this for inspiration:
#include <stdio.h>
int main(void)
{
const int width = 8;
for (int i = 0; i < width; ++i) {
printf("%*s%.*s\n", width-i, "&...
2
A pyramid in geometry is a certain polyhedron i.e. a 3D structure. Your empty stripe in the middle turns it into a stepped pyramid.
This can be split into two triangles, as you do with print_hashes(). But while the function is declared as:
void print_hashes(int width, int empty_space)
it is called with height, not width
print_hashes(height, height - row);
...
0
BUG: q_init() doesn't return a value. Your compiler should have caught this; perhaps you're running without enough diagnostics turned on? (I think this requires a diagnostic, so perhaps you need a better compiler).
BUG: We forgot to copy the terminating null character!
for (int i = 0; i < numtest; i++) {
q_push(nameq, test[i], strlen(test[i]));
}
...
2
UI
I noticed that when I add a new task I tried to click on the text "New task" to replace that with the text of the task but then I didn't get a cursor, so then I clicked on the text labeled "Task" and started typing - I could barely tell my text was added but it was black so it was difficult to see it against the black background. ...
3
"Utility" libraries
It's time to take the training wheels off, so to speak. You need to stop using cs50.h and replace it with standard calls to the C libraries. The implementation for get_string has a careful, dynamically-allocated buffer algorithm that is really not necessary for most purposes and can be replaced with simpler calls that use a ...
1
Most of the code in this class just makes no sense and written only because it has been shown in some tutorial as an example for some other class.
In case of a database connection class, there is no use for the database credentials' getters. You never need a database host anywhere outside of the database class. Hence, getServername() method will never be ...
3
I don't see any problems with the code functionally, and the layout follows good practices for the most part. You chose meaningful names for the signals and parameters, and the parameters make your design scalable to different data widths. It also follows good practices using nonblocking assignments for the sequential logic. This is excellent for a ...
2
Global variables
one major suggestion - don't (over)use global variables, it's generally not a good practice and it will make more difficult to test the code. For example if you'd like to add a functionality for downloading multiple items at the same time you'll have a hard time with it.
So instead of this:
def av_select():
global av_switch # this ...
0
The solution is OK, if you know that you have to perform a time propagation of exactly \$t\$ with that same Hamiltonian and you have to do so countless times.
As someone else has suggested, ditch all the matmul and conjg and use only BLAS calls.
If you apply the propagator to a small number of vectors, however, computing the full propagator is not efficient, ...
3
It's hard to tell if the compiler would be able to eliminate duplicate calls to delta. It may do that, by inlining the x1 and x2 functions and then eliminating common subexpressions. Personally though, I prefer to do this myself. The following modification is guaranteed to call delta only once:
solve (Quadratic a b c) = solutions
where
both = [x1,...
3
Overview
Your code produces the exact same set of samples each time you random_shuffle. So the output is always
0 1 2 3 4 5 6 7 8 9
4 3 7 8 0 5 2 1 6 9
0 5 7 8 4 3 9 2 1 6
5 7 6 3 8 4 2 0 1 9
4 6 0 2 8 1 3 9 5 7
4 0 5 1 7 9 6 2 8 3
3 8 5 6 1 7 2 4 0 9
0 4 6 2 9 5 8 3 1 7
1 3 6 8 2 7 5 9 0 4
5 1 7 9 8 0 6 4 2 3
You should initialize the seed. You ...
5
Over all, if this is your first time coding, you've really done a stellar job. I'm having a hard time believing this was a first attempt without replicating someone else's code, just because of how cleanly you've approached it. Very well done.
The below suggestions are more further improvements than they are corrections on what you were doing. Some of the ...
3
Dividing work
The job of your calculator is to take some input, process it, and show the output to the user. If you split those 3 steps into functions of their own you add a lot of clarity to the person who reads your code. Moreover, you can re-use those functions if you want to perform the same step again.
Something along the lines of
var numbers = ...
4
Building on Toby's excellent answer, this offers 2 alternate ways to parse the options.
#!/usr/bin/env bash
readonly PROGRAM=${0##*/}
readonly USAGE=$(cat <<END_USAGE
Usage: $PROGRAM [FILE]...
By default file(s) will be extracted to the working directory.
OPTIONS
-d, --directory, eg. nzip [file] [-d directory]
END_USAGE
)
die() { printf '%s\n' ...
5
I'm not sure why we have #!/bin/bash - it looks like plain, portable #!/bin/sh would be fine here.
Things I immediately like include the good error handling, using &2 for error messages and exiting non-zero.
All the tar commands can be combined, as tar is able (with -a) to automatically identify any compression scheme it handles.
As there's no need to ...
3
General Observations
Interesting project. The code looks a little too much like C and not enough like C++. You might want to convert the structs to classes to take advatage of data encapsulation.
I suggest that you follow more of the advice given in the answer on your first question:
Make your code as platform-independent as possible
You should not need two ...
2
Let's raise the bar a bit here.
Function Declaration
def convert(s):
This function converts ... something. Inches to feet? Celsius to Fahrenheit? The function name doesn't give us any clue. The function argument s is similarly non-descriptive.
What does the function do? It truncates a string 3 characters after the first period character, if one is ...
1
I decided to take a crack at simplifying this problem myself as it sounded like a fun challenge.
@SᴀᴍOnᴇᴌᴀ already offered a bunch of good advice. The main thing I want to add is how important it is to be able to split up the logic into well-named and easily understandable helper functions. You did have one small helper function in there, but one or two more ...
3
In fact this would be sufficient:
return '{0:1.3f}'.format(s)
(provided that the argument passed is a float).
There is no need for a function.
The above will return a string, if you want to return a float then use the float function:
return float('{0:1.3f}'.format(s))
Or even shorter, taking advantage of F-strings. I prefer this option personally:
return f'...
0
You can just use the format() function to get the result.
def convert(s):
""" Convert 1234.12345678 to 1234.123"""
s = print("{:.3f}".format(float(s)))
s = "1234.12345678"
convert(s)
Refer to this link to know more about String Formatting.
2
Your option1 and option2 functions are a bit confusing in that they use val_am, but up until that point, val_am hasn't been assigned yet. This forces me to read ahead a bit
then scroll back a bit if I'm trying to follow the data, and also opens up the possibility of accidentally calling one of those two function before val_am has been set if you refactor ...
1
I've managed to join the links and the todo list in my modular 'compiler server'. I've added some more features too and I've had some growing pains with the code.
Components have a minimum size for good interactions. Changing the todo component to the width of the links module makes the todo component horrible to use. Whilst I've dropped the width from ...
2
Consistency
Sometimes you are using 4 spaces for indentation, sometimes 2. Sometimes you are using vertical whitespace (an empty line) before a method definition, sometimes you don't. Sometimes you have whitespace at the end of a line, sometimes you don't. In one case, you indent a couple of lines just for no discernible reason.
You should choose one style ...
4
There'sa bug in your square and square_root functions:
def square(self):
self.set_prev_expr()
if True: # If the last number is in paren applies to entire paren block
match = re.findall('\[[^\]]*\]|\([^\)]*\)|\"[^\"]*\"|\S+', self.expression)
print(match)
try:
last = float(...
7
Separation of business from presentation
It's mostly good; Calculator is effectively your business layer. The one thing that creeps me out a little is using a tk.StringVar in it. One way to have a "pure" Calculator that has zero requirements on tk is to accept bound function references to answer.set and equation.set as arguments to your constructor....
3
Overall this isn't a bad effort. I can't see any problems, aside from clobbering the EOF, mostly it's stylistic choices. So good job 👍.
Comments go above
It's much more common for the comment in a header to be ABOVE the function, rather than below it. This is ok for now as there's only one function, but when you have more it will confuse people.
If you're ...
4
Overall, a good and thoughtful effort.
Bug: Treats character as EOF
When EOF == -1 and code reads a character with the value of 255, the c = getc(fp); likely converts that 255 to -1. Later, the loop incorrectly exits.
signed char c;
c = getc(fp);
if (c == '\n' || c == EOF) return NULL; // Maybe true for wrong reason
Instead use an int c to well ...
5
Excellent comment in the header. It's very important to be clear who owns the returned memory and how to release it. It ought to mention that both newline and \0 are considered as end-of-line characters.
Good realloc() usage - we don't leak when allocation fails.
Some small improvements:
#define EXPAND_RATIO 2
Why do we need a preprocessor macro for this?...
5
When structuring your code to components (classes) you should think about what the application tries to accomplish and what steps it needs to do to achieve that. In this example, the steps are quite clear:
Read lines
Split line to words
Normalize words to lower case
Filter interesting words
Gather statistics
Print statistics
These are the responsibilities ...
2
I'll start out by doing some a general code-quality, just with your app.js. Then I'll go over some of the other questions you had.
app.js code quality suggestions
Let's start with this chunk:
var words = [];
fs.readFile('dico.txt', 'utf8', (err, data) => {
words = data.split('\n');
});
While often asynchronous versions of a function is preferred, ...
0
It seems odd to me to use an algorithm that relies on knowing exactly how many URLs will need to be fetched.
You use len(fetcher) to determine your starting depth as well as the buffer size for the urlChan. So basically you're using what you know about the mock data to write code that would need to be deployed on the unknowable internet.
Thanks for sharing ...
6
In general
Whenever you try to implement an application try to use sentences to describe the desired behavior. After that ask yourself: "Can it be further refined?" If so, then write new sentences to detail your plan in a more fine-grained level.
Let me show you what I mean:
The view from 10 000 feet
The application can perform basic operations ...
2
An excellent attempt to writing your Calculator, I find the variable names and syntax following well accepted conventions for nomenclature in C#
One suggestion: anticipate invalid inputs from user. I would recommend using int.TryParse in place of Convert.ToInt32().
6
Make use of the standard library
You are reinventing lots of wheels in your programs. There is already a lot that the standard library you are using will do for you. Even in C on Linux, you can use POSIX functions for managing hash tables: hcreate() and hsearch(). But even better is probably to write both the Linux and Windows versions in C++, and make use ...
1
This answer is meant to be for beginners from a beginner. I try to incorporate best practices as explained by Lev M. while not changing the original structure of the code too much. I guess the most important take away is to follow the DRY principle as much as possible.
I changed the structure of the shop items to lists for easier access. The shop level then ...
5
Unnecessary conditional
if (args.length > 1) {
for (int i = 1; i < args.length; i++) {
option.addWords(args[i]);
}
}
If args.length is not greater than one, then for (int i=0; i < args.length; i++) will execute zero times. There is no need to protect the for loop with an if statement. The ...
6
Perspective
The answers that were already provided are great for solving the particular problem you have. I will provide feedback based on my current workflow. I implement a tool for those who would like to make quick C++ prototypes without much regard to engineering, which I then fix and make more of a complete solution.
Underlying problem
What you want to ...
17
Here are some things that may help you improve your program.
Separate interface from implementation
The interface goes into a header file and the implementation (that is, everything that actually emits bytes including all functions and data) should be in a separate .cpp file. The reason is that you might have multiple source files including the .h file but ...
5
Design:
Data is not a descriptive name, and also misrepresents what the class does. It's really a generator function that's triggered by calling get().
As such, it would be better to replace it with a function, called something like generate_spirals().
We should use the C++ standard library random header for random number generation. This gives us various ...
8
This is a nice, readable piece of code. I would just highlight a few things you might want to consider.
Unnecessary Class
Let's take a close look at Random class. It holds no state, just a member function rand. In addition to having just one member function, it is only used in one place, in member function generate_data. Classes are meant to be reusable and ...
0
Why reinvent the wheel? Why not use Get-Date cmdlet, and simplify the code?
$StartDate = Get-Date -Day 13 -Month 1 -Year 2019
$EndDate = Get-Date -Day 13 -Month 12 -Year 2020
$IntStartDate = $StartDate.TofileTime()
$IntEndDate = $EndDate.TofileTime()
do {
if ($StartDate.DayOfWeek -eq 'Friday') {
Write-Host ($StartDate.tostring("dd-MMMM-yyyy&...
5
As you are as you say "brand new" to coding this will be less like a review and more like some pointers and advice..
Some pointers
Rather than define the functions as arrow functions within the class verticalScore = (score, col) => { Use the shorter syntax verticalScore(score, col) {
There is a technical reason due to the way this is associated ...
3
File storage
Text files are not convenient for lookups. So your next logical step should be to learn about databases and SQL. With Python you can use SQLite to build a small, self-contained database file.
Security
Entering a user name is not enough, there should be a password too. I am assuming this is for a family business or a small circle of very trusting ...
0
You write
from termcolor import colored
but then ignore that and write non-portable escape sequences directly in the strings:
print("\033[1;34m"+"....", "\033[0;30m")
Doesn't that defeat the point of termcolor, which is supposed to adapt to the connected terminal?
4
Functions
Study functions. Your code would greatly benefit from breaking up into functions.
Bugs
User Management
If you add the name "Anne", you cannot login with the name "Anne" until you quit and restart the program.
If you remove the name "Anne", you can still login with the name "Anne" until you quit and restart ...
2
Hey here a few little things I saw. I hope this helps a bit.
Overall comments
A good coding practice is to use Access modfiers
class BattleSystem should maybe become public class BattleSystem depends on how you layer your projects what access modifier to use https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/access-...
4
This function is quite complicated. You should consider writing tests for it. For example:
If the code you gave is in a file named shop.py. Put the below code in a file called test_shop.py:
from shop import shop
from unittest import TestCase
from mock import patch
class TestBuyingThings(TestCase):
@patch('shop.input', return_value='3')
def ...
Top 50 recent answers are included
