53
Your inner loop appears to be unnecessary. Why not create a lookup:
var knownAccountNumbers = new HashSet<string>(
dataStructure.Tables["AccountData"].Rows
.Cast<DataRow>()
.Select(row => row[0].ToString()));
Now your loop is simply:
foreach (DataRow row in dt.Rows)
{
var accountNumber = row[0].ToString().Replace("\""...
20
You shouldn't be sharing Connection instances, because ADO is already doing the job for you, it uses object pooling behind the scenes. Sharing the Connection object will make it very hard to clean up your resources after you are done with it.
Use interfaces instead of concrete implementation.Use IDbConnection instead of MySQLConnection
You should use ...
17
Strains() {
System.out.println("Select a Strain.");
System.out.println("1. Blue Dream");
System.out.println("2. Sour Diesel");
System.out.println("3. OG Kush");
System.out.println("4. Girl Scout Cookies");
System.out.println("5. Green Crack");
System.out.println("6. Pineapple Express");
System.out.println("7. GrandDaddy ...
14
Note: I don't think this is what you want to hear. But I have to write "the truth". This is my personal opinions about your code.
Give me parameterized SQL or give me death!
tell me if there are any bugs or improvements to be made.
Don't suggest prepared statements.
I'm sorry, but those two requests really don't fit together in the same Code ...
14
The code appears to be broken, you're not break;ing after finding a match so all records will likely have row[12] == "N".
You should really be doing a join on accountNumber:
var matchingRows =
from DataRow row in dt.Rows
let rowKey = row[0].ToString().Replace("\"", "")
join DataRow queryRow in dataStructure.Tables["AccountData"].Rows
on ...
13
for (int i = 0; i < type.length; i++) {
if (ch == 7) {
System.out.println("What type of cannabis? " + type[6]);
System.out.println();
break;
}
if (i == ch) {
System.out.println("What type of cannabis? " + type[i - 1]);
System.out.println();
}
}
This loop is completely confusing. And as it turns out,...
13
I really think this is a bad idea. A function called any returning True if there not only isn't anything true but even not anything at all... that's just wrong. It sure isn't a "generalization". Shadowing the built-in is not a good idea, either. Maybe just call yours any_or_empty (without a default)?
For the emptiness check, the counting seems like ...
12
I see a number of things that may help you improve your code.
Eliminate function prototypes by ordering
Since you put the acadd and menu implementations above main in the source code, you don't need the function prototypes. Generally, I try to order functions from smallest and simplest to most complex so that one can read from the top to bottom. Your ...
11
Security
This code is not secure.
SQL Injection
You are trusting the user input completely (except in the login function where you do use mysqli_real_escape_string, which is not secure enough). Use prepared statements instead. You should also read up on SQL injections in general.
Other
This is all minor in comparison to the SQL injection.
don't ...
11
Reduce duplicated logic by using helper methods
Instead of repeating this condition:
if (file.getName().endsWith(mime)) {
Use a helper method:
private static boolean isValidFile(File file) {
return file.getName().endsWith(mime);
}
The advantage is that this hides the implementation details of what is a valid file to you. Without this, all you ...
11
Just commenting on ... the comments in the source (sorry): I think they add a lot of noise, and don't provide any improved clarity (in this case). They're mostly restating what the following line of code obviously does, or repeating what a variable name already describes.
Such comments "by habbit" are actually harmful, as they have no benefit and could ...
11
The most terse and declarative way to write this is to use C#'s ?? operator. Your line becomes:
new SqlParameter("@Price", (object)items.Price ?? DbNull.Value),
The expression here will either return the non-null value of the left operand (items.Price), or return the operand to the right instead.
If you hadn't guessed, the ?? operator is basically ...
10
Let's start with your priorities: optimization. You should be primarily optimizing for the most expensive and limited resource, which is almost always developer brain cycles. You optimize for that resource by writing clean, readable, well-organized code with few surprises. Your code can be improved on all of these fronts.
As far as CPU or hard-drive ...
10
It looks like passwords are stored in plain text form in the pswd.txt file. Never ever store passwords in plain text form. Store passwords salted and cryptographically hashed. That way, if an attacker gains access to the file, he still has to crack the password, which can be extremely difficult if the password is strong enough.
To verify a password, apply ...
10
Reflection is slow. That line probably doesn't help your performance :
var model = (T)Activator.CreateInstance(typeof(T));
I notice that you always create your instance with parameterless constructors. So you could add the type constraint new() in your class. That means you could do :
var model = new T();
That'd be faster and well... clearer.
You also ...
10
All you have to do is to use the ?: (ternary operator):
new SqlParameter("@Price", items.Price == null ? DBNull.Value : (object)items.Price)
Alternatively you could write an extension for it:
public static object PriceOrDefault(this ItemsType items)
{
return items.Price == null ? DBNull.Value : (object)items.Price;
}
and use it instead:
new ...
9
Also said before, but I think it worth mentioning again: there is not need for prefix_ for all columns. As far as I know, Hungarian and Leszynski notations are not exactly recommended. Since column names are almost always used in conjunction with their table, the table alias will make the reader known the table for a column.
It's a matter of taste, but ...
9
Self-documenting code
char h1 = { (e.hour / 10) + '0' }; // Extract the first digit and convert to char (if any, else 0)
As you considered the division by ten and the 0 too enigmatic you added a comment, this is much better than leaving the reader wondering so good job.
The even better option though is stating your intention in code:
char h1 = ...
9
Both impressive and ambitious!
Passwords
the absolute requirement of a 32 character long password ensures it will take a considerable amount of time to break the encryption
It will also ensure that some users will be writing that password down or saving it to a text file, defeating the entire purpose of a password. A softer approach would be, during the ...
9
Any and All
If you are going to override any(...) to return a custom result when given an empty iterable, you should also override all(...) to return a custom result as well, for consistency.
Detecting the Empty Iterable
This code:
count = 0
for count, item in enumerate(iterable, start=1):
if item:
return True
return count ==...
9
We're missing an include of <stdint.h> in the header, and <stdbool.h> in the implementation.
Some parts of the code are hard to read. In particular, I think this comment should be wrapped at a more conventional line length:
/*
* Description: This module is used to accumulate data when the network is down or unable to transmit data due to poor ...
8
You have a static dictionary as backing store which means that all cache instances share it. Not saying that this is wrong and it might be intended but you should be aware of it.
Your cache apparently is not thread-safe. You need to protect access to the dictionary with locks or if you are using .NET 4 or later use ConcurrentDictionary. If you make the ...
8
First, I don't like the concept of mixing "Models" with "Repository".
One principle that, from my point of view, should be followed, is the Single Responsibility (the S in SOLID). It means that one class should have one responsibility, and only one. Therefore yours would be against this principle, simply because you are holding your data, and loading data ...
8
Nitpicks
There is a slight amount of inconsistency in your capitalization of keywords. SUM() / sum() and WHERE / where. Otherwise, it's pretty consistent throughout!
Aliases
You use the following aliases: B C T. There is no meaning or explanation of what those mean. You should use meaningful aliases, especially where your table names don't really help, such ...
8
I can understand why you want to remove the clutter from your code - but sharing the instance of MySqlConnection isn't a good idea. As soon as multiple threads are trying to use the connection at the same time, things are going to go wrong.
The connections are already pooled for you (See here) so you creating and disposing IDbConnections is cheap and easy.
...
8
In your function pick_password, you should close the file at the end of the file because this frees up any system resources that were being devoted to working with that file.
For some reason, at the end of that function, you commented out the line
f.close()
You should un-comment this.
In python, it is good practice to open a file using the with keyword....
8
Everything is a String
Welcome to Java and object-oriented programming! You are doing something really strange in Java in your Effects() method (which really should be called effects() or getEffects()):
String BD = "Happy: 10 " + "Euphoric: 9 " + "Releaxed: 8.5 " + "Uplifted: 8 " + "Creative: 6.5 ";
String BDM = "Stress: 10 " + "Depression: 7 " + "Pain: 6....
8
There are some things to improve in this code. Especially when it's underlined that "if it can be maintained later on".
SQL query
The columns where the data will be inserted should always be defined, for example: INSERT INTO TASK(id_task, title, creation_time... etc) VALUES(... . It's the same problem as for SELECT *, that should never be used in serious ...
8
Your code will work, of course, but you go to a lot of trouble to grind the result set into an array when that capability is already available. Mysqli would provide the fetch_all() function if you had the mysqlnd drivers available; fortunately, PDO has provided it since PHP 5.1.
You will need to create a PDO connection with
$dsn = "mysql:host=" . $hostname ...
Only top voted, non community-wiki answers of a minimum length are eligible
