Hot answers tagged

25

Parallel computing and asynchonous computing are two separate things. While they might be similar in some cases, the two terms have two very distinct meanings in C#-land: Asynchronous computing is used for the async/await and Task<T> idioms. Parallel computing is what you are doing here, using plinq: processing streams of data in different threads in ...


15

Understanding your code was a bit of a challenge (albeit an interesting one). Overall, I see a lot of function definitions that don't seem to accomplish anything. Why did you write public static IOrderedEnumerable<Pair> SortPairs(this IEnumerable<Pair> data) => data.OrderBy(p => p.Key); and then write .SortPairs() when you could have ...


12

Your algorithm isn't the most efficient out there. As you process the input, you can keep track of the numbers you have already seen and which numbers would complement those to sum to x. For instance, in your example, x=4: You read 1, you know that if you encounter a x - 1 = 3, you have found a pair. Next you read a 2, you know that if you encounter a x - ...


11

While your goals may be admirable, I think that, when performance takes a large hit, the whole approach needs to be re-examined. This is atleast 5X's slower than a comparable one liner, even after removing the Combine() method and just returning an IEnumerable<string>. If someone wanted to test with different terms(ie 7 and 9) and/or different words(...


10

English language These are relatively minor issues, but fixing them might help other people to use / maintain your code. The verb corresponding to permutation is permute. I'm pretty sure that reminder is intended as remainder. Code public static IEnumerable<T[]> Permutate<T>(this IEnumerable<T> source) { return permutate(...


9

The comments mention being reusable as the main goal of the code. It is however far from it. The main issues I see with most of the extension methods above: Looking at the name of the methods (and parameters) I can't always tell what the outcome will be. When the outcome is pretty clear, the implementation reveals surprinzing, totally unexpected side-...


9

Now, I fully congratulate you on the exercise - it's an interesting problem and I think a great learning opportunity. However, to learn from this we must acknowledge it's shortcomings. I think the biggest, most glaring issue for me is that this decomposes business logic beyond the simplest possible implementation! Take a second and look at these three ...


8

Exit early Instead of stuffing everything in one return statements, return as early as possible. if (!projects.Any()) return Enumerable.Empty<string>(); can be the first statement of your method. It saves recreating the hashset objects and it makes the flow of your method with its recursion more clear. I think sacrificing a bit of "conciseness" for ...


7

I would argue your garden-variety C# developer is pretty sharp with LINQ as it is, rather than abstracted away, and also hopefully keeping up with modern C# innovations such as first-class Tuple support. Your take becomes a two-liner which I would argue is just as readable and maintainable. If you are wanting real maintainability, usability, and ...


7

Using line breaks and {} in code is not a crime :-P Only because we have the nice => doesn't mean we have to use it everywhere. The code is so condensed that it's hard to say where anything begins and ends. I find you should first try to write this function in such a way that it is easy to read and one can see what and where could be optimized. So, I ...


6

public static TSource ArgBy<TSource, TKey>( MinBy and MaxBy are very intelligible names, but ArgBy doesn't say much to me. Really this is a specialised Aggregate (and I wonder whether an implementation using Aggregate might be simpler - or, indeed, whether the existence of Aggregate makes ArgBy unnecessary). Func<(TKey Current, TKey ...


6

As part of you regular testing, since you are using recursion, you should run this code at a large input that creates many nested calls. C# doesn't support require tail call optimization, therefore the code will throw a stack overflow error. To fix this, switch to the usual while loop.


6

Using straight linq you could get to two iterations over Neighbours, assuming Neighbours is a class and not a struct. var neighbour = current.Neighbours.FirstOrDefault(n => n.DeadEnd) ?? current.Neighbours.Where(n => !current.Linked(n)).First(); Pretty similar to yours except we using FirstOrDefault to return null if not found and using the ?? to ...


5

For set-based operations like this it's better to use HashSet<T>. The HashSet<T> class provides high-performance set operations. The items can easily be converted to ones containing HashSets. Then, the Overlaps method does the comparison: var hashed = items .Select(i => new { Name = i.Name, Values = i.Values.ToHashSet() }) .ToList(...


5

var result = new List<ScrapCategory>(); ... if (!result.Contains(parent)) There's one performance problem. Use a data structure which gives fast Contains checks: typically HashSet<>. var parent = categories.Where(x => x.Id == parentId)?.First(); There's another one. I assume that IDs are unique, in which ...


5

where TValue : IComparable<TValue> Don't do this. OrderBy doesn't require its type parameters to implement this interface like this either. Instead it uses the IComparer<T> interface. What's the difference? With IComparable<T>, the implementation of the comparison logic is sitting on the class T itself. This means that there is one and ...


5

As slepic in his comment, I also wonder why you use an enumerator in the first and foreach in the second place? You can eliminate null checks in the versions that call other overrides: public static int DistinctCount<TSource>(this IEnumerable<TSource> source) => source?.DistinctCount((IEqualityComparer<TSource>)null) ?? throw ...


5

Because the question is about performance and efficiency that's why some sort of benchmarking would be essential to compare different implementations. I have found BenckmarkDotNet really useful for these kind of experiments. You can define the for loop version as your baseline and the tool will compare the other implementation against that. Here is my ...


4

You did also ask for a shortened version. I believe readability should not be a concern here (meaning the code should aim at being functional, not readable). Remove the local recursive function and allow the public API to have an optional parameter. public static IEnumerable<T[]> Permutate<T>( this IEnumerable<T> source, IEnumerable<...


4

First, you're going through the list of items to find all distinct children. Then, for each child, you're going through the list again to find all items that it belongs to, and then you take the lowest item ID. That's \$O(n^2)\$ in worst-case scenarios, though in practice it'll be closer to \$O(n)\$ if you don't have too many distinct children. A more ...


4

Indeed, you could use GroupBy but I think it's easier to solve with the query syntax because we also need the item from the first list to sort by its Id. Without it, we would need to do additional lookups like you do. var result = from item in items // from + from = SelectMany from child in item.Children orderby item.Id // sort in ascending ...


4

It might be boring to hear it again, but I have to ask :) Why do you think your application need performance optimisations? Is your bottle neck in these parts of code which you provided? Maybe problem is with database/external service/network? Have you measured processing and communication time? Stopwatch Do you have some kind of monitoring of application, ...


4

Welcome to CodeReview. What you are trying to do is simple enough that your LINQ implementation is okay. Let's face it, with dominoes you are not going to have a million players with a million bones, so no need to worry about performance issues with a fairly small set. You didn't ask explicitly for review of other parts of code, but I will offer them ...


4

There are a couple of constructs in there that can be improved. string.Join("", charArray), repeated a lot. string has a constructor that takes a char[], which is a lot faster. They are both linear time, which your question seems to focus on, but in terms of actual efficiency the difference is nearly two orders of magnitude in some tests (actual impact of ...


4

The bad part is that ToEnumerable returns an IEnumerable that blocks for each element until it is produced by the IObservable it is called on. This means that you might as well be doing: public static IEnumerable<R> SelectAsync<T, R>(this IEnumerable<T> source, Func<T, Task<R>> selector) { foreach (var item in source) ...


4

Sort it As @Peska suggested, sort the array first. But sort it descendand, or simply iterate in reverse order. Overflow The multiplication of integers can overflow. It would be wiser to approach the iterations from product, divide by one element and check if the division equals second element, but only if product modulo first element is zero. Contains() ...


4

Wow, those methods are pretty similar. Initially, these objects: List<ProjectObject<ExhaustEquipment>> exhaustEquipment List<ProjectObject<Infiltration>> infiltration are only different in which template parameter refers (ExhaustEquipment,Infiltration), so you could do a generic method: //this all, could be written as a Generic ...


4

I would suggest a two lines of code solution instead of one. :) Let me show you first the solution then the explanation. Solution private static readonly Lazy<IEnumerable<string>> enumValues = new Lazy<IEnumerable<string>>(() => Enum.GetValues(typeof(AttributesInMethod)).Cast<AttributesInMethod>().Select(option => option....


3

Since you mentioned morelinq in your question. It does have the Batch method which is similar. It's an IEnumerable<IEnumerable<TSource>> instead of IEnumerable<TSource[]> but if you look at the source code it's actually an IEnumerable of an array but you can't count on that as it's an implementation detail. The only issue I see with ...


3

I think in general MinBy and MaxBy are useful but handling all the null cases might be tricky, and indeed it is seeing the repeated logic and two similar loops. For this reason I also think that it'd be easier if we do not handle them at all but let the user decide what he wants to do with them and implement both simply with OrderBy(Descending).First() ...


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