New answers tagged

2

For now I'll mostly pick on how you do user input. In your first two functions, and then again in your main code, you build a complicated data structure with nested lists, only to ever need the elements in the list. Just, don't use that complicated structure. It might make sense to write a function that asks the user for input and evaluates it if needed: ...


2

Repetition You write: discriminant = b ** 2 - 4 * a * c followed by: if discriminant == 0: x_one = (-b + sqrt(b ** 2 - 4 * a * c)) / (2 * a) # x_one ... if discriminant > 0: x_one = (-b + sqrt(b ** 2 - 4 * a * c)) / (2 * a) # x_one ... x_two = (-b - sqrt(b ** 2 - 4 * a * c)) / (2 * a) # x_two ...


0

Welcome to Code Review, you are not closing the Scanner resource and this is a resource leak: to avoid this you can use from java 8 the construct try-with-resource like below: try (Scanner scn = new Scanner(System.in)) { //here your code } You can write your line: System.out.println((0 > a1 || 0 > a2 ? "ERROR-NEGATIVE NUMBER" : "OK")); using ...


3

I have some suggestion for you. Code duplication In your code, you have some code duplication that can be extracted in a method. By extracting the code, the code will become shorter, be less error-prone and easier to read. I suggest that you make a method to print a question and read the user input. public static void main(String[] args) { //[...] ...


2

Create a class Matrix Instead of having a vector of vectors, and have global functions that manipulate those, create a class Matrix and add member functions to it to manipulate matrices. You should probably also create overloads for arithmetic operations, so you can write things like auto matrix3 = matrix1 + matrix2; Have a look at existing C++ matrix ...


1

What I need is to optimize this code to be as fast as possible. That's premature. Start by making the code correct. For example, you do a multiplication of x*x in your primality check, but that can overflow. Once you have the code correct, then run the code through a profiler to find out where the slow parts are and attack those one at a time. What you ...


1

You can get a vast improvement in speed: If x is divisible by y, then x is also divisible by (x / y) with the result y. So if you try to get the divisors of 1,000,000,000,000 you only need to divide by the numbers 1 to 1,000,000 and add two divisors to your list at a time instead of 1. The exception is when x is a square and x/y = y since you don’t want to ...


1

I saw it would be better if I post an answer instead of using comments. For your IsPrime method, I see that you've covered most conditions, but you forgot to cover 0, 1, and -n cases, which you can just do with a small change on this line : if (n % 2 == 0) return false; to if (n < 2 || n % 2 == 0) return false; suggested : public static bool ...


4

Style As far as style goes, your code looks clean and readable, and follows the conventions, so good job for that. You may want to include documentation comments with \\\ four your class and methods. Although the method names are descriptive enough in this rather simple case, it is a good habit to take on. Tests It's nice you use a test framework to test ...


3

To add to the other answer(s): The parameters to both getDeterminant and printMatrix should be const-ref instead of passed by-value. int dimension = getDimension(); should be const int dimension = getDimension(); Note that str = str + ','; and number = number + str[k]; could be rewritten to str += ',' and number += str[k], respectively, for potentially ...


3

You need to check for std::cin::eof() before std::cin.fail() (or simply !std::cin), because there's no point repeating the read infinitely if we've reached EOF. To demonstrate, just run the program with closed standard input, e.g. make 237153 ./237153 <&- The reading of the matrix's elements fails in a different way at end of stream: ./237153 <&...


3

You deleted this before I could answer on SO: Option Explicit Sub Lead_Score() Dim shp As Shape Dim RelScore As Long Dim QualScore As Long For Each shp In ThisWorkbook.Sheets(1).Shapes 'change (1) for your sheet name. RelScore = MyRelScore(shp) QualScore = MyQualScore(shp) Next shp Dim Score As Long Score = ...


3

Though there is already an accepted answer, I would add that usually, you would want to only return values between 0 and 127 in main -- just replace return -1; with return 1;, following the convention that non-zero exit codes represent errors or exceptions. (https://stackoverflow.com/a/8083027/9870592)


8

A vector of vectors can be an inefficient representation, because the storage may be scattered across many pages of memory. We can keep the data close together by using a single vector, and linearising the rows within it, perhaps like this: #include <cstddef> #include <vector> class Matrix { std::size_t width; std::size_t height; ...


10

double getDeterminant(std::vector<std::vector<double>> vect, int dimension); This will create a copy of the std::vector. For small std::vector this is not a problem, but it's good to make it a habit to pass complex data structures as const&, so the copy will not be created: double getDeterminant(const std::vector<std::vector<double>...


14

if(dimension == 0) { return 0; } Mathematically, that is not correct. The determinant of an empty (i.e. zero-dimensional) matrix is one, see for example What is the determinant of []? on Mathematics Stack Exchange. With respect to efficiency: Your program computes the determinant recursively using the Laplace formula, which requires \$ O(n!) \$ ...


2

double matrix[dimension][dimension]; double array[dimension * dimension]; double matrix[dimension][dimension]; double subMatrix[dimension - 1][dimension -1]; double array2[(dimension - 1) * (dimension - 1)]; None of those are legal C++, because dimension isn't a constant-expression. We have a signed/unsigned ...


0

Some good points have been said, I'm not going over those again. Just wanted to say that your code is probably meant to be used by someone else. Even if it's not, your API should reflect what your code will be working on. I would not take 2 arrays for input, but an array of 2d coordinates. Then, if for your computation it's easier to split those coordinates ...


0

Consistency Your class does not seem consistent with itself. If you create a LinearReg object: x = [1, 2, 3, 4, 5] y = [2, 3, 2.5, 4, 3.5] lr = LinearReg(x, y) The constructor computes the mean of the x and y lists, and stores these for later computations. These means are fixed; you cannot add additional data points, as the mean will not be recomputed. ...


4

Toward optimized functionality and design since all crucial attributes x, y, x_mean, y_mean are initialized on LinearReg instance creation (within __init__ method) - no need to duplicate and define them as class attributes (x = [] ... y = []), those should be removed as redundant raise Error - Error is not Python exception class. Use Exception or ValueError ...


Top 50 recent answers are included