IEatBagels
  • Member for 7 years, 9 months
  • Last seen this week
  • Canada
Generate random numbers without repetitions
22 votes

Instead of using a List<int>, you should use an HashSet<int>. The HashSet<> prohibites multiple identical values. And the Add method returns a bool that indicates if the element was ...

View answer
Simplifying a series of type checks and casts in a generic method
17 votes

You should create a Dictionary<Type,Type> that would associate your object's type with their DTO equivalent, so you wouldn't need any if. Then you can use the dictionary to find the types to map....

View answer
Find easter on any given year
15 votes

When I finish writing code, I usually ask myself : "Okay, do I think I'd be able to understand my code in six months?" If the answer is no, it means I'm not done. My code needs to be clearer....

View answer
Immutable objects in Java
15 votes

This answer isn't about immutability since @rolfl has it covered. You should put spaces between your = in your assignations, it is easier to spot the = so it makes it easier to see the assignation. ...

View answer
Input validation for a user
Accepted answer
13 votes

You could merge your two foreach loops using SelectMany in LINQ. So instead of : foreach (DbEntityValidationResult result in results) foreach (DbValidationError error in result.ValidationErrors)...

View answer
Is this queue properly implemented?
13 votes

This is a partial review about some things I saw reading through your code fast. The setter of your Count property shouldn't be public, because I could set it to one million when there's only one ...

View answer
Earliest time frog can jump to the other side of a river in C#
Accepted answer
12 votes

Readability The code needs more spacing. Considering the comments, the indentation and the ifs, it's hard to read without some good old empty lines. Use brackets when using conditions, especially if ...

View answer
Python3 Health System
12 votes

I gotta start by saying it's weird to ask how much damage you want to take, my answer would always be zero, you know! Your variable names could be more descriptive. I understand that DMG means ...

View answer
I have a problem ...factory
12 votes

When passing parameters to a public method, you always need to assert that the parameter isn't null, otherwise your code will throw NullReferenceException and nobody likes them. The generic types C ...

View answer
Thread safe logging class in C# to use from DLL
12 votes

I don't think Log should be public, it is confusing. Me, as a client of your logging API, shouldn't need to create an instance of TextWriter to be able to log something. If I want to do some IO, I'll ...

View answer
Multithreaded Mandelbrot Generator Ver 2
11 votes

I think that your limit checking is a little obscure. Why are the constants defined in the constructor? They're related to your class, not to your constructor. If I were to be able to change the ...

View answer
Implementing common fraction using TDD
Accepted answer
11 votes

A denominator can be negative! \$\dfrac{1}{-2} = \dfrac{-1}{2}\$ You should check that the denominator isn't zero, because that's bad. You could use operator overriding! public static Fraction ...

View answer
Interpreting Brainfuck code to C#, then compiling to a .exe
Accepted answer
11 votes

Dayum, there's some static around here. Why not make it an instance class? What you have here is a workflow, where methods have a specific order. This is clearly seen in your Main method. The ...

View answer
Another attempt at a Blackjack game
Accepted answer
10 votes

Boolean used in conditions Let's take the following example : if (skyIsBlue == true). In a proper English sentence, we'd read : If it is true that the sky is blue But wait, we don't talk this ...

View answer
Blackjack game interview task
10 votes

main method In the main method, I don't think you should pass args to the createDeck function. The main's job is pretty much to check input and be the workflow of your game. After all, if I was to ...

View answer
You are Promoted, You are Promoted, We All are Promoted!​
10 votes

You don't check your entry parameters for null. In FindSelection, this query could be reduced : var target = _declarations .Where(item => !item.IsBuiltIn) .FirstOrDefault(item => item....

View answer
Generic asynchronous SQLite search builder
Accepted answer
10 votes

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 ...

View answer
Simple password generator
10 votes

Your CharacterSet method could be more performant! Let's work on this first. Using String to keep characters to include and exclude isn't a good plan. String concatenation is kind of expensive and ...

View answer
Operations to a bank account
10 votes

Don't catch NullReferenceException, this is a programming exception, it shouldn't happen when to code is running if you're doing your job well. If something throws a NullReferenceException, find why ...

View answer
Counting Calories
Accepted answer
10 votes

Welcome on Code Review! By convention, your namespace shouldn't contain underscores, you should split them with . if it is neccessary (which isn't in your case I think) Weight_Watching_Program -> ...

View answer
Singleton Design Pattern implementation in a not traditional way
9 votes

There's a potential bug with this scenario. According to this Jon Skeet post, if an exception is thrown inside the static constructor, it is never retried. Which means that if your Singleton ...

View answer
Shortening boolean with if statement for document version comparison
9 votes

I believe it could be a one line, but I don't think you should. One liner's are kind of "cool" to write, but they are a pain to debug, and a pain to read. I think you should keep your variable, it ...

View answer
Is this Repository Design Pattern Valid and efficient?
9 votes

I think your abstract class should "implement" the IRepository interface, because otherwise you will always have to put both the interface and the base class in your inheritance list. Also, I think ...

View answer
Beginner's Number Guessing Game
8 votes

Overall you've done a good job with your code, there are a couple things I'd look out for : You aren't paying for whitespace! What I mean is that it's important to let your code breathe. Use some ...

View answer
Python Calculator Test Driven Development
Accepted answer
8 votes

TDD It's quite hard to review TDD as it's more of a way to do work than an observable product. What I mean is that whether you did TDD or not, the unit tests would mostly be the same. One minor thing ...

View answer
Reading data from network stream
Accepted answer
8 votes

You should never throw an empty exception. Imagine someone uses your code and receives an empty IOException. What happened? How do you debug this? First, you need a message that explains the ...

View answer
Architecture to cache data
Accepted answer
8 votes

Let's go over the basics first! The indentation is quite a mess. But I suppose that might be related to the SE formatting. Otherwise, you should make your indentation coherent. Stuff like : public ...

View answer
Converting strings to PascalCase
8 votes

Compared to the String, StringBuilder is mutable. Meaning you don't really need to Append multiple times, you could just replace the characters from the original string: //that's just for the test ...

View answer
Async insecurities
8 votes

The first thing that needs to be addressed is the indentation and the spacing, because that's the real issue here. You should line your braces with the start of the line above it, like so : if() { }...

View answer
The way to realise data access
8 votes

Your code seems really good, I would change the ITestingRepository methods to return IEnumerable<> instead of List<>. Also, I suggest you would add a method signature to your generic ...

View answer
1
2 3 4 5
8