25
In short programs it can be OK, but in general avoid writing using namespace std. You'll find plenty of material here and elsewhere on why this is so.
To promote proper encapsulation of data, both real and imaginary should be declared under private, i.e., not be visible to the outside.
All of the four member functions that perform arithmetic take on too ...
15
Use field initialization lists:
So your constructor
ComplexNumber(double real = 0.0, double imaginary = 0.0) {
this->real = real;
this->imaginary = imaginary;
}
Can become:
ComplexNumber(double real = 0.0, double imaginary = 0.0)
: real(real), imaginary(imaginary) { }
Simplify your returns
I could see an argument for making an extra ...
12
Since the goal is the best possible implementation of this algorithm, I'd suggest the following. However, faster algorithms do exist.
To conform to PEP8 make sure you have two blank lines after your imports and surrounding function definitions.
Since, you aren't editing each item, but rather adding and removing items until the list is sorted, I'd use the ...
12
To answer your comment, yes there are equivalent of PEP8 in java. I suggest checkstyle, this plugin works in lots of IDE / text editors and can be configured with your code style.
Code review
reverse_string method
In java, we try to use the upper-case version of Snake case only on constants / enums. For the methods and variables, I suggest that you use ...
10
In java is the preferred way of curly brace placing on the same line like you can see in code snippets in Oracles Style Guide.
reverse_string has a type embedded in its name which leads to a code smell.
The disadvantage is that if you want to change the type of the parameter you have to change the method name too.
Additional the invocation looks messy and ...
9
Your code looks well-formatted, it's easy to read and to follow, and the explanation you gave matches the code exactly. Well done. :)
for item in my_list:
This statement looks strange since in the body of this for loop, you neither use item nor my_list. You can express the idea of that code more directly:
for _ in range(len(my_list)):
The variable _ is ...
9
Doi9t's answer is very good, but even with their improvements, there is still a problem: your code does not produce the correct answer in all cases.
Java strings use UTF-16 encoding. This means that a Java char is not large enough to store all Unicode characters. Instead some characters (for example, 😂) are stored as a pair of chars, and reversing the pair ...
9
First, note that the complex class is unnecessary because we have std::complex in the standard library, which is provided in the header <complex>. If you want to design your own class, std::complex is a good reference. Now, for two complex numbers x and y, we can use x + y, x - y, x * y, and x / y directly.
Next, notice that this pattern comes up a ...
8
Member Access
In the real world, people often care about being able to look at the real and imaginary parts of a complex number individually. As such, you will want to provide an interface to them. While contrary to some of the advice you revived in your last review, I'd advise giving these members variables public access. These components are not an ...
8
Building off the the already excellent points made by Juho,
Redundant arguments
Within your member function, you never make reference to the object being called on. Take for instance your ComplexNumber::add function. A more sound object-oriented implementation might resemble
void add(ComplexNumber other) {
//Just add real- and ...
7
Operator Consistency
You provide operators for +, -, etc., but as it is some things I would expect to do are illegal, such as
ComplexNumber c(1, 2);
ComplexNumber d(3, 4);
d += c;
Generally the recommendation with these forms of operators is to implement the += form in your class, and then define + as a non-member in terms of +=. For example:
class ...
6
As in your previous question, your interface is still awkward. That is, if there's an add method, I fully expect that calling a.add(b) will mean that a results in a plus b. So in particular, the state of a will be changed.
A user of your class will also find void printNumber(char mathOperator) weird. Indeed, why as a user of the class do I need to worry ...
6
The CustomArrayList API is inconsistent because I can declare a list of type E to add elements of type E, but when I remove an element from it, I get an Object element:
public Object remove(int index) { /* ... */ }
Inside CustomArrayList you work with Object[] elementData. From the book Effective Java; Item 26 you could change your code to:
class ...
5
Dangling Reference
If you add 10 elements into the array, and then you remove the last item, the array capacity will be 10, the array size will be 9, and the last element, self.elementData[9] will still point to the item that was removed. This means the item can never be garbage collected.
Fix: When you remove an element from the array, you should null ...
5
People have talked about simplifying the return temporary, but not the interior temporaries:
ComplexNumber operator+(ComplexNumber a, ComplexNumber b) {
//Just add real- and imaginary-parts
return ComplexNumber(a.real + b.real,
a.imaginary + b.imaginary);
}
Conversely, sometimes you should make a temporary. Notably, the ...
3
String parsing
You are right that C++ slightly lacks some convenient features for string parsing. If you were not trying to count lines as well as find words, then a single while loop with:
inFile >> s;
would have sufficed, because streaming will terminate on whitespace by default. But because you want to count lines, you need the 2 loops and ...
3
Clean and readable code! Nice work.
Here's some nitpicks.
swap_together
template<class... Its>
void swap_together(size_t i, size_t j, Its... its)
{
using std::swap;
int dummy[] = {
[&](){
auto it = its + i;
auto jt = its + j;
using std::swap;
swap(*it, *jt);
return 0;
...
3
Elephant in the room
Let's first try and identify the elephant in the room. There is one big problem with using GCM and file decryption: tag validation. The problem is that for GCM there are two choices: either you output unvalidated chunks of data, or you buffer all data until the tag is validated. The first option should be followed by any reasonable low ...
3
Employee
If you're not going to change the fields for Employee, then they should be final. As it stands, there doesn't seem to be anyway to access the values...
List interface
Your list implementation doesn't really make sense to me. I think part of that is because you've implemented your display method as part of the list. This really doesn't belong ...
3
This code doesn't work, as can be shown by including <algorithm> and changing the output:
std::cout << std::is_sorted(tobesorted.begin(), tobesorted.end()) << std::endl;
Other things that are surprising:
We pass pointers but always assume they are not null. That suggests we should be using references instead.
Misspelt std::srand and std:...
2
Do you actually need to create the reversed string. Why not just go over the string from both directions.
String palindrome = "radar";
isPalindrome(palindrome, 0, palindrome.length() -1);
// Could be converted to loop if wanted.
private static boolean isPalindrome(String candidate, int startIndex, int endIndex) {
if(startIndex >= ...
2
For reverse_string, it would probably be most efficient to create an array of chars and then construct a string from it. Note that I have not tested this code.
private static String reverse_string(String my_string)
{
char[] chars = new char[my_string.length()];
for(int i = 0; i < chars.length; ++I)
{
chars[i] = my_string.charAt(chars....
2
First I got same approach than previous answers (@Roland Illig & Luapulu) that are very similar.
But then I remembered Item 72 of Effective Python (Brett Slatkin's book):
72: Consider Searching Sorted Sequences with bisect. Python’s built-in bisect module provides better ways to accomplish these types of searches through ordered lists. You can use ...
2
This isn't really a code review so much as help on how to get this to compile.
Array::Array(int *p = NULL, int s = 0){ default parameters go on the declaration, not the definition.*
Array::Array& operator=(Array other){ Qualify the operator with Array::, not the return type. You're not returning an Array::Array**, you're defining the operator= member ...
1
Don't using namespace std; - especially not in a header, where it inflicts the harm on every source that includes the header.
Prefer nullptr to NULL, because the former is more strongly typed.
Use std::size_t for indexing, rather than int.
When overloading operator[], it's usually necessary to provide two versions:
int& operator[](std::size_t);
int ...
1
The Custom implementation is good.
Improvement/Suggestions-
Integer Overflow is not handled for the size
private static final int HUGE_CAPACITY = Integer.MAX_VALUE - 3;
private void ensureCapacity() {
// Here newIncreasedCapacity can be negative value
int newIncreasedCapacity = elementData.length > 0 ? elementData.length * 2 : INITIAL_CAPACITY;
/...
1
Naming
class Rosa tells me nothing. The name should scream out that it is a dependency injection container.
PSR
There is a standard interface for DI containers.
https://github.com/php-fig/container/blob/master/src/ContainerInterface.php
You might want to have you container implement it.
Exit vs. Exception
Implementing the PSR interface would ...
Only top voted, non community-wiki answers of a minimum length are eligible
