Skip to main content
51 votes
Accepted

"How are you spending your time on the computer?"

Performance [..] winforms is not powerful so it takes 1-2 seconds to refresh the content [..] It's not WinForms because it's actually very fast and I have never had any issues with it. There is a ...
t3chb0t's user avatar
  • 44.3k
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 ...
Heslacher's user avatar
  • 50.4k
32 votes
Accepted

Use of Async/Await for EventHandlers

The things to consider when it comes to async event handlers are: Exceptions thrown for the handler might be rethrown on the UI ...
svick's user avatar
  • 24.3k
23 votes

House plan design (Head First C#)

I have a general rule: if I see string concatenation with a loop I assume it's not the best way of doing it. Let's look at this: ...
RobH's user avatar
  • 17k
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 ...
motosubatsu's user avatar
16 votes
Accepted

2D Collision Detection

Collision detection You can calculate the collision much easier with the Rectangle structure. Use the Contains method if you ...
t3chb0t's user avatar
  • 44.3k
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 ...
Geek's user avatar
  • 439
15 votes

Handling keyboard shortcuts in C# software

Also I may want to implement shortcut customization in the future, and there is no easy way to find out what keys are mapped to which method so it will have to be rewritten. Stop right here! Extract ...
Mathieu Guindon's user avatar
15 votes

"How are you spending your time on the computer?"

First thing I would do is change if (mainString.Length == desiredLength) to if (mainString.Length >= desiredLength), since if ...
Der Kommissar's user avatar
14 votes
Accepted

Calculate subtotals and totals from a form with many decimal values

Two words: loops and arrays. There a bunch of variable names that differ only by a number. Whenever you see code like this, you can probably clean it up by creating a collection of those things and ...
Greg Burghardt's user avatar
12 votes

Checking whether textboxes have data

Why so complicated? One possible approach is: ...
tym32167's user avatar
  • 229
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 ...
oerkelens's user avatar
  • 396
12 votes
Accepted

Clearing all controls

I think you're rather over-complicated this for yourself! A much better solution is to use the pattern matched results (which you are already creating (t)) (this ...
VisualMelon's user avatar
  • 7,431
11 votes
Accepted

C# OOP Pong game in WinForms

First thing's first: Don't use public fields. (What is a field? It's a member of a class/struct that looks like a regular variable.) ...
Der Kommissar's user avatar
11 votes
Accepted

House plan design (Head First C#)

RobH's review covers syntax and style well so I won't go into that. Instead I'd like to give my take on feedback given by Svek and BittermanAndy. Separation Of Concerns I think Svek's commentary ...
Mr.Mindor's user avatar
  • 346
10 votes
Accepted

Converting string to control

Unless you're working with an ancient version of .NET, Convert.ToString(i); is superfluous. You should be able to just do ...
BCdotWEB's user avatar
  • 11.2k
10 votes

Login form C# SQL

using as already covered SQL injection as already covered For getting a single row Reader is more efficient than ...
paparazzo's user avatar
  • 6,016
10 votes

House plan design (Head First C#)

You've already made a decent effort to make your types immutable by setting its properties in a constructor and providing get-only access, not set - like you have with ...
BittermanAndy's user avatar
9 votes

"How are you spending your time on the computer?"

I added a few dummy rows in the listview and tried to refresh it a few times, to see how it looks. And guess what it was flickering and it was taking around 100ms maybe to refresh the list view. Which ...
t3chb0t's user avatar
  • 44.3k
9 votes
Accepted

Windows Forms Clock

Timer The clock-face and label are 'out of sync' because Invalidate() does not cause the window to draw immediately, so ...
VisualMelon's user avatar
  • 7,431
9 votes
Accepted

GUI Interface for a command line program in C# Windows Forms

meaningful names These are not terrific identifiers. ...
J_H's user avatar
  • 30.7k
8 votes
Accepted

PDF script tool to convert documents to PDF and add fields and scripts

...
Mathieu Guindon's user avatar
8 votes
Accepted

Refactoring Calculations through a series of textboxes

Honestly, it looks like you want to use a domain object to model your providers and perform the pay calculations. Rather than using a bunch of text fields and drop-down controls, you can then data ...
Dan Lyons's user avatar
  • 3,618
8 votes
Accepted

Alternate checked between buttons

Yes, this can easily be achieved with a few non-redundant lines. You can hook up all button clicks to this common MouseDown event: ...
TaW's user avatar
  • 326
8 votes
Accepted

Creating multiple threads to process items in a listview

We actually don't use the BackgroundWorker anymore. The same thing can be done much easier with the newer Task Parallel Library (TPL). Briefly you need to (ok, you ...
t3chb0t's user avatar
  • 44.3k
8 votes
Accepted

Winform, with a workaround to avoid dependencies

If you start thinking with code, you will almost always end up doing bad things. Why? Because it is not natural. OK, no more preaching and let's get to work. Review of Your Code I apologize for ...
CodingYoshi's user avatar
8 votes
Accepted

ROT13 encoder/decoder

The number one problem here is that you concatenate translated letters on to your string one at a time. Since strings are immutable in .NET (and most right thinking languages), this means for the ...
Rafe's user avatar
  • 1,360
7 votes
Accepted

Guitar Hero III Bot

That huge comment block at the top should be an actual comment block: ...
Mathieu Guindon's user avatar
7 votes

Dynamically adding controls to a form in a WinForms project

In my eyes it is an appropriated way to build up your form that way. The *.Designer file is for designer generated code - but if you need to add custom logic to ...
JanDotNet's user avatar
  • 8,538
7 votes

2D Collision Detection

Just a quick answer. You are calculating s.Width / 2 for each Picturebox two times. Just calculate it once, store the result in ...
Heslacher's user avatar
  • 50.4k

Only top scored, non community-wiki answers of a minimum length are eligible