New answers tagged

1

Just a quick shot regarding performance..... The code is calculating xIndex * xSize xSize*ySize*zSize times yIndex * ySize xSize*ySize*zSize times zIndex * zSize xSize*ySize*zSize times while it only should be calculated once for each iteration of the while loop. In addition, x only changes in the first for loop and y only changes in the second for loop.


0

There is plenty room for improvement Attribute There is a built-in class in the BCL with same name which help you define decorators. That's why I would not recommend to call your class like this. AttributeEnum: I personally don't like to include enums inside a class definition. The usage is also really weird (stuttery): Attribute.AttributeEnum.Cat You ...


0

Just a couple of questions: Can you add tanka and tankb as properties in Info class perhaps (any reason about Info being static?) Pumping method, you are also passing a Pump object, why not using the one from your class? i.e. pump.Pumping1(tanka, tankb) Do the tanks changes? Or you create one object tanka, tankb, and pump, and you only need to read the ...


2

To reduce the lines of code to copy/paste, you could create a helper method: public static class Helper { public static void EnsureIsInRange(short value, short min, short max) { if (value < min || max < value) { throw new ArgumentOutOfRangeException("The passed value is not between " + min + " and &...


1

DbContext is a Repository and Entity Framework uses Unit of Work pattern to wrap it, so if you need to wrap DbContext with a Repository you will need to narrow it down to more focused scope. You'll find some arguments about wrapping a repository with another repository (e.g. using repository with EF). Generally, it's not about right or wrong, but it's about ...


2

Since I don’t see any function of type IEnumerable, I guess you have not taken advantage of Unity’s StartCoroutine function. It starts a new thread that runs parallel to the main thread. The main thread is the one making sure the game renders without lag. So from your description it sound exactly like the solution to the kind of problem you are having. I ...


4

Just two things. The using keyword is just synctatic sugar for a try/finally block. There's no need here to embed using block in a try block. In general you can use either: try { sr = new StreamReader(filename); txt = sr.ReadToEnd(); } finally { if (sr != null) sr.Dispose(); } or using (StreamReader sr = new ...


3

Quick remarks: IMHO the code inside ProcessPaymentFile (and its related methods) should be a class of its own. It's 50+ lines and should be broken into smaller methods; the fact that you're using #regions is already telling. I'm not a fan of _configuration["UploadPath"];. You should consider the approach detailed here: have a POCO object ...


4

In this review let me try to focus on the longest part of your code: WinCondition. Try to separate condition checks from user interaction, do not mix them Try to use better naming, like: CheckVictory, HasAWinner, HasGameEnded, etc. Try to separate data and logic during condition checks When they are not separated var conditions = new List<bool> { ...


10

Choosing good identifiers When I see a name like options I think of something a player can choose from like "single player" (i.e., play against the computer) or "multiplayer". A better name would be board because that is what it is. torf? Does it mean "true or false"? Every Boolean is true or false. This does not reflect the ...


7

I think you have a reasonable grasp to understand the technical ins and out of design patterns, but you are struggling to use them sensibly. So this answer focuses on intent rather than specific syntax. I just want to take some time here to point out something that you may wrongly infer about my feedback. Just because I call your implementations bad, doesn't ...


0

Some quick remarks: IDatabase is IMHO a bad name, and you can see that with a method name like CreateDatabase. The same problem also applies to other things, e.g. SqlServer which is in reality a connection string (which you should never hardcoded, it should be part of a configuration file). And so on. I haven't checked your github, but I suspect you are ...


1

In this review I'll focus only on Abstraction and Encapsulation because the other two is not applicable in this scenario. Whenever we are talking about OOP then we usually refer to well-defined objects (with known responsibilities) which are communicating with each other to solve a greater problem. Abstraction We are decomposing the original problem into ...


3

Just use Array.ConvertAll that is already exist. For instance, you want to convert array from string[] to int[] you can do this : var converted = Array.ConvertAll(array , item => int.TryParse(item?.ToString() , out int result) ? result : 0); Another point is, if you only targeting one-dimension array, then you don't need to use the abstract class Array, ...


3

There are a few alternative solutions which might work LINQ It is possible to use Enumerable.Cast<T>() to achieve the same results (at least for a 1D array) [TestMethod] public void CheckConversion() { // One Dimensional Array with int element convert to int[] var array1 = Array.CreateInstance(typeof(int), 10); for (int i = 0; i < ...


3

After reading the code in question it seems every method is doing exactly the same hence I will focus only on one method and tell you what I would change: public static int[] ToIntArray1(Array input) { Type elementType = input.GetType().GetElementType(); if (input.Rank != 1) { throw new System.InvalidOperationException(); } if (!...


3

The Wait call is tricky. It throws by default AggregateException not OperationCanceledException. If you provide the cancellationToken to the Wait then it will throw an OperationCanceledException. The biggest problem with this approach is that it cancels only the Wait not the underlying task. Please read this article for more details. So, if we switch to ...


1

If you don't want to pull in DynamicLinq its not a lot of Expression tree work to get exactly what OP is looking for. In the constructor getting the methods we need to call for the Queryable, I'm assuming this is IQueryable and not IEnumerable. If IEnumerable just swap all the Queryable for Enumerable. For the ToLower matching of the property using the ...


5

If you're willing to use the DynamicLinq library, the problem becomes trivial. It allows you to pass a string parameter to the OrderBy method, bypassing your entire logic: queryResult.OrderBy(column) If this is related to doing a SQL database lookup, if you think about it, the issue you're facing it a bit of a self-imposed issue, because you start from a ...


3

While aepot answer is a good answer and got my upvote it does only work for type int. Which I know the question for an example uses int in their code but if wanting it to work for all data types will need to change the code a bit. public static class EnumerableExtensions { public static IEnumerable<TSource> FilterRepeatingElements<TSource>(...


2

In this review let me try to focus on a single thing: eliminating duplicates. What is the problem with duplicates? The size of your code base could become quite lengthy. It ruins legibility. If you need to change one thing on the functionality, which affects all duplicates, then you have to do that in all occurrences. If you need to extend your datasource ...


6

Quick remarks: Why do you convert a string to a string: Convert.ToString(ConfigurationManager.ConnectionStrings["MyDBConn"].ConnectionString);? It's even called a ConnectionString. Why do you use Dapper and then don't use it properly? Why return a DataSet? Why use SqlMapper and DynamicParameters? Why make life so hard for yourself when Dapper is ...


3

Some quick remarks: Properly name things. avail_recordings and fpcalc_output do not follow the Microsoft guidelines (e.g. don't use "snake case", don't needlessly abbreviate,...). request = (HttpWebRequest)HttpWebRequest.Create("http://api.acoustid.org/v2/lookup?client="+API_KEY+"&duration="+fpcalc_output[0].Replace("...


3

Here are my observations: There are couple of unused namespaces right click on the namespaces then select the Remove and Sort Usings menuitem acoustid_mbids \ recording: The general guideline is to prefer Pascal Case for class names. So, AcoustidMbids and Recording would be the suggested names. Please applies these naming for the sub-classes as well. ...


2

Here are my observations: List<BacktestResult> data: Please try to use more expressive names than data. It is way too general and it does not help the reader / maintainer of the code. (same apply to table and query) Populating columns: Your implementation has a lot of repetition in it: tables.Columns.Add. One way to reduce that by using the AddRange:...


0

This looks like a bug: int end = str.Length - 1; for (int i = 0; i < end; i++) { if (Char.IsWhiteSpace(str[i])) { if (i != offset) { if (str[i] == Seperator) { if (!Char.IsWhiteSpace(str[i + 1])) That str[i+1] can be out of range, on the last ...


3

It seems like your end goal is to apply ConvertAll to all of the different types of C# arrays, nested or not, and with with lower bounds not equal to zero. All arrays can be manipulated through the Array base class and any level of nesting can be handled by using recursion. First some plumbing is needed to enumerate all possible indices of an array of ...


0

In view the extra property is highly unlikely to cause a breaking change (unless for some reason the consumer tracks the count of properties returned from the API call for whatever reason), the answer is this extra property does not warrant the need to create an additional version.


2

I don't want to echo BCdotWEB's observations, they are all valid ones. Rather I would like to focus on the RegisteryKey API usage. OpenSubKey Let's take a look at the signature of the method first: public Microsoft.Win32.RegistryKey? OpenSubKey (string name, bool writable); As you can see it may return a RegisteryKey or a null. So, it might make sense to ...


3

When I see that many recurrences like this then it is a good sign for me that this is a perfect place to introduce meta-programming via T4. Step #1 Separate the overloads from the core logic: using System; namespace Sample.Core { internal static class Converter { public static Array ConvertArray<T, TResult>(Array array, Converter<T, ...


2

Overall, the code is good on a technical level. This answer addresses the design philosophy behind it, because I think you've overthought the default size approach to the point of generating more work and complexity without actually adding value. We've all been there :-) The Size.Default is used in situations where the image is required in the size it was ...


2

Are you sure that you need two queries for this? E.g. you could do something like this: UPDATE Product SET Quantity = Quantity - @usedQuantity WHERE Id = @productId AND Quantity >= @usedQuantity Try to execute that and if it doesn't affect 1 row - you don't have enough quantity for the line item. The drawback for your current logic is that you no ...


Top 50 recent answers are included