43
votes
Accepted
Extensible code to support different HR rules
I coded keeping SOLID principle in mind and here is my code. What did I miss?
Let's see...
SRP - Single Responsibility Principle
A class should have only one reason to change.
Violated. The name ...
- 44.1k
33
votes
Login form C# SQL
SQL Injection alarm
Don't use user passed text in your sql statements if you aren't using parameters.
From: https://stackoverflow.com/a/7505842/2655508
Using parameters helps prevent SQL Injection ...
- 50.1k
31
votes
Accepted
Performing and timing loops with SQL queries
In the case of both if your while loops, you could easily make them for loops instead, which tends to be the best practice.
<...
- 20.1k
27
votes
Accepted
Simplifying constructing native SQL query with nested if-else's
Let's look for some patterns in your code:
First of all, we notice that you have three parameters with similar names. condition, ...
- 58.7k
22
votes
Accepted
Excuse me, how many of you are new here? Mind if I JOIN you?
Technically, the full outer join is insufficient. If during a certain week there was not a single user who posted their first question or answer, the plot for that week needs to be 0. It would be ...
- 143k
20
votes
Accepted
Where are my downvotes?
If this is actually your first SQL you're quite good in learning new languages.
Some remarks:
The Group By is not weird, it's the ...
- 1,172
18
votes
Accepted
Preventing SQL injection by converting all characters to their ASCII values
There's a common credo believed when evaluating the security of software:
If it's homemade, it's unlikely secure.
Sec.SE has a Q&A about homemade algorithms, which is somewhat germane to your ...
- 5,717
18
votes
Login form C# SQL
Cleartext passwords
Not a code-specific issue exactly but it would appear that you are storing the user's passwords in clear text. This means that anyone gaining access to the LOGIN_Tab would be ...
- 291
17
votes
Extensible code to support different HR rules
public class EmployeeLeave : ILeaveRequest
...
var leaveRequest = new EmployeeLeaveDetail();
This is confusing. Going by name, ...
- 2,296
16
votes
Accepted
Top users under age 30, sorted by age, with rank
A few things, some nitpicks, some UX, some probably-bugs. What Lyle's Mug has already stated is also part of my answer, but first things first:
UX
This query is quite unwieldy to use, because all ...
- 25.2k
16
votes
Login form C# SQL
In addition to Heslacher's answer I would also look at the using statement.
It is very important to keep a handle on resources like Sql Connections to ensure they ...
- 439
15
votes
Math quiz in PHP
That may sound a bit harsh, but have you ever heard of the for loop ?
More on the SQL side of your code :
SELECT * is BAD ! And that's really obvious in your code : the only field that you use is ...
- 829
15
votes
Accepted
Searching for bad Minecraft questions on Gaming.SE
The use of the Tags field on the Post is a poor choice for the query. It relies on a table scan, which is slow, because it has to check each question. Note, you can use the ...
- 97.1k
14
votes
Accepted
"How can I make this SEDE query better?" (bad title finder)
Your query is well structured, and consistent, but there is one issue you have failed to incorporate.... not all bad questions have a user. When questions are migrated, or there's a user deleted, the ...
- 97.1k
14
votes
Excuse me, how many of you are new here? Mind if I JOIN you?
'2011-Jan-01' --There was very little activity prior to this date. Including it skews the graph.
And then
...
- 74.9k
14
votes
Accepted
Oh my goodness: How many selfies have I taken?
One by one, then:
Is it good that I stored the posts in tables, or is it unnecessary?
It is unnecessary, in this case. What your table variables are doing for you here really is simulating a self-...
- 20.3k
14
votes
Extensible code to support different HR rules
catch (Exception)
{
throw;
}
Well this won't buy you anything but a few more lines of code. Just remove the try..catch ...
- 50.1k
14
votes
Accepted
A wounded Zombie finder, or: how to find the almost dead undead?
Not utterly wrong, but
q.PostTypeId = 1
and
AND a.PostTypeId = 2
are superfluous. The only post types with a non-null ...
- 1,089
13
votes
Accepted
Sonar remarks make SQL unreadable?
First things first, I assume you are already convinced that using literal SQL query strings is the way to go in this project/class, and that there are no comparable solutions from using an established ...
- 19.2k
13
votes
Accepted
Query to find users from Australia
I don't like how you're specifying the column aliases. I expect a whitespace between the column and the alias.
I like that you're not specifying the optional AS ...
- 74.9k
13
votes
Top users under age 30, sorted by age, with rank
If you write the query where you first order by rank and then by age, you will get a better result, and you should be able to get rid of part of the where statement, which I would think would make the ...
- 28.6k
13
votes
Accepted
Stack Exchange User Activity Score
TOP 1
When selecting values that correspond to Unique fields referenced by constants (OwnerUserId), you're only really ...
- 11.9k
13
votes
Accepted
Wrapper for commonly used database classes
First of all, it's unclear to me what exactly you want to achieve with this code. Database access, yes, but what benefits should your code provide? Why don't you want to use ...
- 8,199
12
votes
Accepted
Validating user supplied input
There's only so much a code analysis tool can see.
using (SqlCommand command = CreateCommand(query, parameters))
You're using a parameterized ...
- 74.9k
12
votes
Oh my goodness: How many selfies have I taken?
Is it good that I stored the posts in tables, or is it unnecessary?
I would say that it is unnecessary.
Instead of using those temporary tables, you can run a ...
- 58.7k
12
votes
Oh my goodness: How many selfies have I taken?
This can be solved with a much simpler join. The process of loading data in to table variables actually makes the process a lot more complicated.
Consider the following:
...
- 97.1k
12
votes
Login form C# SQL
Security issues are covered, so just a bit on the code itself.
You have three nearly identical pieces of code, inside a switch-statement. That seems to be a code-smell. Apart from that, I'm missing a ...
- 396
11
votes
Performing and timing loops with SQL queries
Why isn't using (SqlConnection con1 = new SqlConnection()) inside your while loop, right before ...
- 11.1k
11
votes
Searching for bad Minecraft questions on Gaming.SE
This doesn't do anything at all.
ClosedDate IS NULL
OR ClosedDate IS NOT NULL
Saying "where is or not" results in the same thing as not including the statement ...
- 30.8k
10
votes
Top users under age 30, sorted by age, with rank
It's not obvious what this query does without reading the code. You should add a title and description in Stack Exchange Data Explorer. I also find the columns illogically arranged: I would expect
...
- 143k
Only top scored, non community-wiki answers of a minimum length are eligible