New answers tagged beginner
4
There's an implementation trick for linked lists which simplifies a lot of the logic. If your per-node data isn't too large that this is wasteful, consider
class linked_list
{
struct Node
{
int32_t data;
Node* next;
};
Node sentinel {0, nullptr};
Node *last {&sentinel};
// ...
};
Now: you don't need a special case for an empty list,...
6
General Observations
Welcome to Code Review. It is helpful when you provide the code that tested the class as well as the class so that we can do a better code review.
As noted by VNP modern C++ uses nullptr rather than NULL, this helps differentiate it from the C programming language.
Obvious additional possible methods are addNode(), and deleteNode() ...
7
struct Node would benefit from its own constructor. As a side note, if you don't want to expose struct Node to the client (and trust me, you don't), better make it private to class LinkedList.
tmp doesn't deserve to be a class member. It is strictly local to each method.
DRY. push_back could and should be streamlined:
struct Node * tmp = new Node(data);
...
1
First of all, welcome to Coding. This is pretty advanced for a person who just started to code.
Here are some tips:
Avoid importing stuff that you don't need:
I see you have imported random from random but you did not use that, try to avoid doing that because it complicates things
from random import randint # is better than from random import random, ...
2
Global variable
It appears the connection string is used as a global variable:
global $conn;
Global variables have more negative aspects than positives. The conn.php file could define a function that would return the connection represented by $conn And the function could be called where needed - e.g. in createCSV(). Then there would be no need to globally ...
16
#include <bits/stdc++.h>
it is a cheap hack, you're basically avoiding including the separate header file - string, vector, iostream. This doesn't even work on my compiler in visual studio ( msvc ).
why should I not #include <bits/stdc++.h>
using namespace std
This one is worse, it's making your program a lot more confusing. When there are so ...
2
You have quite a nice structure. And while it is a bit much for this size of project, it's good training for bigger things.
Still, static const is strictly inferior where constexpr is a choice. Enum constants are also a nice option.
Marking parameters const can be useful for definitions of lengthier functions, which you commendably avoid. But for forward-...
6
As has been mentioned, your code is generally pretty good.
Turn on warnings, and fix them.
computerOrPlayer is supposed to return a bool, yet it doesn't always.
Unfortunately, by default C++ compilers do not warn about this undesirable mistake, but they do generally can detect it -- if you have activate the corresponding warnings.
For gcc and clang, my ...
13
General observations
To be honest, your code is extremely clear and readable to me. I wouldn't guess that you were a beginner from reading your code. You have eliminated the use of magic numbers, and use global constants instead which is good!
Anonymous namespaces
The keyword static in this context means that it has internal linkage. An anonymous namespace ...
8
To add to the answers of Aryan Parekh and pacmaninbw, which I agree with:
Avoid repeating the name of a class in its member variables
For example, in class Event, all the member variable names are prefixed with event_, but that is redundant. I would just remove that prefix.
Avoid using std::string unless something is really text
Apart from date/time ...
4
public static double returnLogisticEquation(double x, double r) {
The name is not quite correct, as it does return the result of equation, not the equation itself, so more like calculateLogisticEquation.
public static double returnLogisticEquation(double x, double r) {
return r * x * (1 - x);
}
I'm torn on this one. Normally, I say that ...
10
General Observations
Welcome to the Code Review Site. Nice starting question, very good for a begining C++ programmer and a new member of the Code Review Community.
The functions follow the Single Responsibility Principle (SRP) which is excellent. The classes also follow the SRP which is very good as well. You aren't making a fairly common beginner mistake ...
5
Looking at your program, I would say that you have done a pretty good job considering that you are just a beginner.
private or public?
Let's have a look at your Event class
class Event {
private:
std::string event_type;
std::string event_priority;
std::string event_date;
std::string event_time;
public:
Event(std::string eventType, std::...
2
Here is my take on the problem, a function which accepts any string length and only allocates exactly enough memory:
//#include <malloc/_malloc.h>
#include <stdlib.h> // malloc, free
#include <assert.h> // assert
#include <stdio.h> // printf, fprintf
/**
* "Defangs" a string by replacing all `.` with `[.]`.
*
* @param ...
8
Consider using asprintf()
Just like you are using strdup() to simplify making a copy of a string, consider using asprintf() to print a string without having to worry about allocating memory yourself. This will greatly simplify your code:
char* defangIPaddr(const char* address)
{
char* defanged;
int ip[4];
if (sscanf(address, "%d.%d.%d.%d&...
12
Watch your memory allocations and deallocations. In both cases, you've got defangIPaddr returning a const char * to heap-allocated memory, which needs to be freed by the caller... but it can't be freed, because free expects a non-const void* as its argument.
Functions that return ownership-of-a-heap-allocation to the caller should (A) return char*, not const ...
2
I re-wrote your solution to use more typical Clojure features. When you are looping over data and need to keep track of accumulated state, it is hard to beat loop/recur. A first example:
(ns tst.demo.core
(:use clojure.test))
(defn breaking-records
[scores]
; this loop has 5 variables. Init all of them
(loop [low (first scores)
...
3
Others have made good points, but I'll add in a stylistic quibble.
return std::size(short_url) != kDomainTinySize ||
!decoded_url.count(short_url.substr(kDomainSize, kTinySize)) ? "" :
decoded_url[short_url.substr(kDomainSize, kTinySize)];
is a heck of a one-liner. The ternary operator is fun but speaking as someone who has ...
3
I agree with everything in Martin York's answer. Just one thing: you can avoid having two unordered_maps if you don't create a purely random URL, but instead create one by hashing the original URL. This way, you will always create the same tiny URL for the same long URL, so you don't need encoded_url anymore. Of course, you would still need to handle ...
4
You are doing the lookup twice.
if (!encoded_url.count(long_url)) {
.. stuff
} else {
tiny_encoded = encoded_url[long_url];
}
I know that it is O(1) for the lookup. But there is a real constant inside that. Avoid it if you can.
Use find(). Then if it is there you can simply use it.
...
1
Stream of consciousness review from top-to-bottom.
You should make sure you're adhering to PEP8 and general styling guides. I like to use Black to format my code
start_var just obfuscates datetime.datetime.now() and doesn't need to exist. duration is similar. I honestly would rather see this wrapped up in your info logger class (see below) - have it log ...
2
I've read answer by @user673679 and just want address a few issues.
I strongly disagree with disabling default constructors in classes like Member/Date/BookItem. If class has no default constructor then using it with std::vector, std::map and other template containers becomes very awkward in general. So it is a bad advise.
Instead you should make default ...
4
Date:
Date.hh is missing some includes (<iostream>, <string>).
Don't supply a default constructor. It doesn't make sense to have a default date.
Don't supply one- and two-argument constructors. Specifying a month and date in 1970 is unlikely to be very common.
We should support years before 1970. There were books back then!
year should be a ...
2
You define inplace_merge() inside the definition of inplace_merge_sort(), but it doesn't use any of the context of inplace_merge_sort(), so it isn't necessary.
If you defined it outside of the definition (perhaps with a leading underscore in the identifier to warn clients it was not meant to be used directly), you would get three advantages:
The definition ...
17
Merge is usually O(m) time, where m is the number of elements involved in the merge. Due to your insertions and deletions, it's rather O(mn), where n is the length of the entire list. That makes your whole sort O(n^2 log n) time instead of mergesort's usual O(n log n).
You call it inplace sort, which suggests it doesn't return anything, but you do return the ...
9
Welcome to Code Review!
PEP-8
Python has a style guide to help developers write clean, maintainable and readable code. It is referred to as PEP-8. A few points of note:
Avoid extraneous whitespace in the following situations:
Use 4 spaces per indentation level.
Type hinting
Yet another PEP (PEP-484) for putting in type hints for your variables and function ...
1
The stringLength leaf function (no prolog/epilog) could still be a bit simpler:
stringLength proc
mov rax, -1
_nextChar:
inc rax
cmp byte ptr [rcx + rax], 0
jne _nextChar
ret
stringLength endp
The comments on the stringPrint frame function suggest that a second parameter exists.
; Displays a null-terminated string.
; Parameters:
; ...
1
Welcome to Rust. Here's some suggestions to get you started:
Creating idiomatic use declarations
It is not common in Rust to bring a function into scope directly via a use declaration. Instead, io::stdin and io::stdout are preferred. See the section Creating Idiomatic use Paths in the book for more information.
Using structs to self-document code
You ...
8
About print()
If you are trying to simply print a new line, you should use '\n' compared to empty print() statements.
print("Hello, World! \n\n")
The code will print Hello, World! followed by two newlines as I have printed '\n' twice.
Converting input() to different data types
By default, the data taken by input() is in the string form, or str(). ...
6
Variable Name
From PEP 8 -- Style Guide for Python Code
Function names should be lowercase, with words separated by
underscores as necessary to improve readability.
Variable names follow the same convention as function names.
mixedCase is allowed only in contexts where that's already the
prevailing style (e.g. threading.py), to retain backwards
...
1
Congratulations on the correct use of the C++11 condition variables! As mentioned by vnp, there will be no difference functionally or performance wise in this specific case, as there is only ever one thread waiting while the other notifies. Some possible improvements though:
Unnecessary return statements
You don't need a return statement at the end of a void ...
4
Welcome to Code Review!
PEP-8
In python, it is common (and recommended) to follow the PEP-8 style guide for writing clean, maintainable and consistent code.
Functions and variables should be named in a lower_snake_case, classes as UpperCamelCase, and constants as UPPER_SNAKE_CASE.
You do not need to specify parentheses around the conditional checks in if-/...
2
Passing larger sized data in functions by const &data is a good idea since it does not make a copy. Note that when a parameter is passed by const&, the extra cost dereferencing and fewer opportunities for compile optimizing. You should do this typically when the data is large in size
From your next() function
int next() {
int curr_next =...
1
Redundant Code
pagetext = ""
pagetext = (page.text)
The first line assigns an empty string to pagetext. The second line ignores the contents already in pagetext and assigns a different value to the variable.
Why bother with the first statement? It simply makes the code longer, slower, and harder to understand.
Why bother with the (...) around ...
3
I think this is a theme for your code: const, where you've put it, has no benefit; and it's missing from other places that it should be there. Every single function in MyCircularDeque should drop the const out front, because those return values are scalar so marking them const has literally no effect. insertLast(const int value) has slightly more effect but ...
5
Using the same type for the deque contents and the sizes/indices (k, count, head, tail) feels wrong. At least, k and count should be std::vector::size_type.
Since you are backing up the deque with std::vector, making head and tail the std::vector::iterator looks more idiomatic.
k is not the most descriptive name. Consider capacity.
I am not sure that std::...
4
You can call stream.reserve(k) in the constructor to improve the efficiency of the vector because you know that you will only have k elements, so .reserve() will pre-allocate the memory.
Prefer using std::size_t over int
int k would be std::size_t k
You haven't declared a copy constructor nor a copy assignment operator. This can cause issues if you wanted to ...
4
On the whole, your Rust code looks good to me. You make appropriate use of the standard library and language features like closures. Nothing stands out as especially unidiomatic; even the formatting looks nice.
That said, of course there are always things that could be improved.
General observations
If you're not using rustfmt yet, start now. Your ...
7
I'm not sure any of the answers have yet really addressed the complexity. I'm going to do that by transforming your algorithm into one that is simpler without changing the time complexity. This both proves the time complexity and also gives you a version of the algorithm that might be easier to read and reason about.
Let's start with your solution
void ...
3
Just to be explicit: You are \$O(n)\$ in time and \$O(n)\$ in memory. I don't believe you can easily do better in integer arithmetic (when actually calculating it) in time, but memory could be \$O(1)\$.
As has been pointed out by Peter Cordes, there is a "Closed form" for the Fibonacci sequence, which means that if you have a constant time ...
0
Thank you all for your input. I was able to dramatically make it more efficient by creating models and using CSVhelper. https://joshclose.github.io/CsvHelper/ to export all the historical records to disk, only once during transitioning to realtime. Much faster.
var trendchars = new[] // array of arrays
{
input1_1_trendchar,
input1_2_trendchar,
...
2
I don’t have enough reputation to comment, so I must say this in an answer.
It looks clunky to use
while x == 4:
and then do
x = 3
whenever you want to break out of the loop.
It looks better to do
while True:
and when you want to break out of the loop do
break
Cheers!
7
The most obvious way to write fib:
int fib(int n)
{
if (n < 2) // 0 or 1
{
return 1;
}
return fib(n-1) + fib(n-2);
}
You can see it explodes because for every call to fib() you get 2 subsequent calls which both get 2 calls with all get 2 calls etc.
Level Calls Calls This Level Total Calls
...
14
About using namespace std
You should try to avoid this statement as it is considered bad practice. It is best to avoid it whenever you can. Here is a simple example why.
#include <iostream>
#inlcude <list>
using namespace std;
class list // uh-hoh, list is already defined, or is that std::list?
{
...
};
Another problem is that if you ...
2
My try to prettify the code. Some comments inside.
private void WriteToCSV2()
{
if (CurrentBar <= BarsRequiredToPlot + LookBack) return;
StringBuilder logEntry = new StringBuilder();
string fullPath = Path.Combine(filePath, fileName);
// don't need a Stringbuilder to build a constant string
const string header = "Index Key,...
6
There is no v-while, no. You can however use v-for over a computed property (which you can compute using while if you so wish).
Although I don't see a reason to use any while-loop. What you have is a data structure with topics and subtopics of any arbitrary length. Using a recursive component for this, which you have done here, is a good choice.
About the ...
4
This answer is only about mergesort
I don't see why you don't want mergesort to take &[u32] as an argument - it makes the function more generic! I read in the rust book that rustaceans prefer to use &str instead of &String as an argument for that reason ;)
I'm new to Rust as well - sorry! - and decided to try my hand at this.
Edge cases
My first ...
1
Avoid converting data structures to string unnecessarily
Just store tuples of values in indegrees instead of strings:
if i == a:
indegrees[i].append((i, b))
indegrees[i].append((b, i))
Iterate over the keys in indegrees directly
In this piece of code:
for i in range(n):
for j in range(n):
if f'{i}-{j}' or f'{j}-{i}' in indegrees[i]:
...
0
Zeros
This:
a_mat = np.zeros
is not the right call for your purpose. You want np.empty instead, because you don't actually care what the initial values are since you do a comprehensive initialization loop right after.
Further to that: since you're adding random() to every element of a_mat, simply initialize a_mat to a single call of random() with the ...
1
@Duck from Stack overflow provided the answer. Pasting it here. Its is essentially writing a function:
library(r2excel)
#Data
sample_prime1 <- 1:4
sample_prime2 <- 10:14
sample_prime3 <- 22:26
#Store in a list
List <- list(sample_prime1,sample_prime2,sample_prime3)
#Function
myfun <- function(wb,name,df) {
# Create object
sheet <- ...
Top 50 recent answers are included
