New answers tagged

1

The code is understandable, perhaps the naming used in some fields is quite hard to recognize for others who are seeing this code for first time. Example Adjs (AdjacentVertices) is not intuitive, you are used to see this code and understand why you'd put everything on its place but other person who comes will spend more time getting familiar with it. ...


1

EF knows about all navigation properties. So, instead of using reflection and try to figure it out by myself, I can just ask the DbContext about navigation properties. Here's a wonderful SO answer that does exactly that.


1

I agree that the question takes a little time to understand. Once I understood it, I felt compelled to code a solution, which is below. The solution you built gets the length between each letter, but the question is more nuanced. The formal logic is in the code, but in plain English: A scene only ends when all of the shots it contains are done. On a ...


2

In reviewing the questions and answers it struck me that one way to simplify things would be to use linked list as the backing data structure instead of a List (which is based on an array). A LinkedList, allows adding and removing from the "head" in O(1) without concern for an index. Sample code is below. Additional points: I agree with the others that ...


2

I don't know why you are surprised plain loops are faster, without an explanation of why you were surprised, it is hard to comment on that. Linq is bound to have some overhead. By vectorisation I guess you mean the use of special machine instructions, I doubt these would have any significant effect here. Most of the comparisons will fail at the first ...


1

If the objective is to avoid duplicating information in memory, the code you have given won't archive it. The new keyword hides the original value. It doesn't replace it. You should try another approximations, like the Decorator pattern: https://en.wikipedia.org/wiki/Decorator_pattern The MasterData inherits from the GlobalData class and stores internally ...


1

The most important part of Clean Code is that your code should be easily understood. The author's indication about the number of parameters of a function is meant to be something like: "Better no parameters than one parameter. Better one parameter than two. You should avoid more than two parameters whenever it's possible" For the function that checks the ...


2

Instead of Type Casting to 'objectorobject[]or usingRouteValueDictionary`. A simple way to achieve the same is using “Newtonsoft.Json” If using .Net Core 3.0 or later; Default to using the built in System.Text.Json parser implementation. @using System.Text.Json; ……. @Url.Action(“ActionName”, “ControllerName”, new {object = JsonConvert.SerializeObject(‘@...


1

You don't care about null references? IMO you create an object in order just to do something, that is more suitable for the concept of extensions: public static class StringExtensions { public static byte[] ToUTF8Bytes(this string text) { return Encoding.UTF8.GetBytes(text); } public static string ToUTF8(this byte[] bytes) { ...


1

Over the years I have seen other students here and they are quite proud of the performance of their code. In some of those past instances I have discovered that they had simply run their code and thought it was fast. They did not compare it to other methods. You don't mention any testing or provide any numbers. If you were to do this, just be sure that (...


2

One thing the other answers don't address is your implementation of IEnumerable: public IEnumerator GetEnumerator() { while (list.Any()) yield return Pop(); } An iterator is supposed to iterate over all elements in the collection, but it should never ever change the state of the collection. Your implementation actual pops ...


3

You have posted several such exercises before, and I have commented on them as well. First let's state the good: you are performing such exercises to improve your C# and .NET skills. But I have noticed a pattern that if someone questions you on why you do some things a certain, you quickly take refuge under the blanket of that's what the exercise states. ...


7

One way that you can simplify much of this code is by building out a set of classes to represent your different operations. You can then encapsulate the rules for that operation within those classes. To get started, you can define an abstract Operation class to define the shared properties and methods of the operations: public abstract class Operation { ...


4

Questions: Why do you use the class field _object? Does it serve any real purpose that a method-local variable cannot? Why do you implement IEnumerable? (only consider this if you have a decent understanding of concurrency issues, and want to practice) How can you make this code thread-safe? Suggestions: Several method throw exceptions in unexceptionable ...


1

I think you are too concerned with the implementation right now. The last part of the question ... recommended ... for scalability and unit-testability? is the most important one. Let's imaging you were to write the test for this feature. You would start off by designing the API that the user of the functionality would ultimately consume. Secondly as ...


2

Here's some C# in accordance with my first suggestion of sorting the array and looping. public bool IsPossibleDivide(int[] nums, int k) { if (nums.Length % k != 0) { return false; } var dict = new Dictionary<int, int>(); foreach (var num in nums) { if (!dict.TryGetValue(num, out var value)) { ...


1

Since you just getting the host url, remove the dots, then append it as a subdomain to the given domain with the approprate protocols (Https or Http). You can do this : public IEnumerable<string> GetRoutedUrls(IEnumerable<string> urls, string domain, int httpPort, int httpsPort) { if(urls == null) { throw new ArgumentNullException(nameof(...


2

If the connection and queue name are not changing per record then there is no reason to be creating a new client for each record in the loop. Especially for the amount of times stated in the original post. Move that to the constructor of the class. public class Function { private static readonly JsonSerializer _jsonSerializer = new JsonSerializer(); ...


3

In terms of conventions there is nothing to poke at there. Where I find that approach lacking is that it tries to merge two different concepts. Namely a point in time an a time difference (or offset). Another non .NET example that I can think of is swift which has Date and TimeInterval. TimeInterval being an alias for a double which represents the offset in ...


2

That's a lot of code to review. I've given it a cursory review and have a few comments, but by no means is it an exhaustive review. Overall, I like your style. Nice indentation, uses of braces, class access modifiers, IEnumerable(s), and variable names not being abbreviated. Most of the times, but not all, your names are clear. Other things stand out. ...


3

Just a note but I think you have the parameters backwards in GetBytes: byte[] key = Extract(salt, inputKeyMaterial); Based on your implementation of: byte[] Extract(byte[] inputKeyMaterial, byte[] salt)


1

This is the nth version, it is broken down into smaller functions, but uses more CPU to operate. I took the above advice to reduce comparisons and also attacked the Collections namespace and And Tuples. I also changed it from eagerly parsing the entire bitmap search area upfront to only parsing 4 lines at a time. In an attempt to make it more readable, I ...


2

On the whole, I quite like the approach. There's a few things to consider. Test Size Your first happy path test seems like it's testing quite a lot more than if the grid is initialized... [Fact] public void WhenMessageIsValid_InitializesGrid() { var input = Lines( "5 5", "1 2 N", "LMLMLMLMM", "3 3 E", "MMRMMRMRRM"); ...


1

You can optimize a bit by make it possible to step out of the first outer loop if the inner foreach-loop doesn't make any changes to dist in one iteration. Wouldn't it be possible to test for maxWait whenever you update dist[v] in the foreach-loop: { dist[v] = dist[u] + w; maxWait = Math.Max(maxWait, dist[v]); } This may og may not be an ...


3

Not sure about your full implementation, but from what I see, you could do a fluent API something like : public class PhysicsProcess { private readonly float _delta; private readonly Vector2 _velocity; public PhysicsProcess(Vector2 velocity) { _velocity = velocity; } public PhysicsProcess Run() { // Run logic ...


2

Your Insert is wrong, you have to do it the same way as the Remove with shifting Elements. ArrayIndex is not increased to decreased on Remove/Insert. Better don't call any parameter a parameter or add the DataType to it. Name as you speak. You insert an 'item' or an 'element' to your list. Not a parameter. Always Doubling the array on overflow can lead to ...


1

Recent versions of .NET have Double.IsNormal(): public static bool IsNormal(double d); Returns true if the value is normal; false otherwise. Applies to .NET Core 3.1, 3.0, 2.2, 2.1 .NET Standard 2.1


1

Thank you to everyone that helped me out on this. This is what it has refactored into based on everyone's suggestions. Looks more concise and easy to read. Appreciate the advice so I can continue to learn C#. using System; namespace ExerciseTwo { internal class Post { public string Title { get; set; } public string Description { ...


2

Your ID is a long created from 2 ints, so you can fit both ints inside this one long side by side: long key = (long)nums[i] << 32 | i; You have a lot of unnecessary if statement evaluations. You could get rid of them by adding the first k elements from the array into the list in a separate loop, and then starting a second loop from i = k that can ...


2

In the BiDirectionalReader constructor, the while loop condition can be shortened to while (++top <= --bot) (or replaced with a for loop). If the bitmap height is odd, the middle row will have duplicate entries in CachedItems. It is unclear to me why you're excluding the last column when constructing the BiDirectionalIndex and CachedItems data. The ...


3

I am not familiar with C#. Here are some of my observations: ++top <= (SetHeight - 1) / 2 && --bot >= (SetHeight - 1) / 2 is hard to read for two reasons: There are inline pre-/post-increments (++top and --bot) Short circuiting may make the behaviour not obvious for beginners There are two conditions (++top <= (SetHeight - 1) / 2 and --...


2

You have a couple of good answers. Since you are an experienced developer who is new to C#, I will address some other things. Things you do quite well Braces and indentation Naming (most of the time) For the last item, most of your naming is good. As @iSR5 mentions, TimeDateCreated could have a better name. I have been programming since the 1980's, and ...


3

There is nothing to add more than @potato's answer. However, I just want to re-enforced the answer. The naming convention for TimeDateCreated can be changed to CreatedOn or CreatedDate or any related naming for creation date. The keynote here is that you don't need to specify the datatype name in the properties as the property is public and I clearly can ...


4

I went a bit crazy about it just for fun, and found a solution that is much more efficient than a loop :) If you look at the bits as the game progresses, you can reframe the problem. Subtracting 1 from an odd number is flipping the right-most bit from 1 to 0. Dividing by 2 is shifting the bits 1 place to the right. Example in binary: 1001101 There are ...


2

My answer is an upgrade of Henrik's answer. (so it's a review of a review lol) Dividing an int by 2 is the same as shifting the bits 1 to the right. I'd expect a good compiler to make such optimizations automatically, but I think it's still worth mentioning. I got rid of the if statement at the start, which in most cases (non 0 input) is a waste of time. ...


3

You need to guard against a negative num, or else your algorithm will run infinitely (num = num - 1). Dividing 36 and 37 with 2 are both 18 with reminders of 0 and 1. So it should be possible to keep dividing by 2 and adding the reminder, in order to add 1 for odd and 0 for even numbers: public int Review(int num) { if (num == 0) return 0; int result =...


2

Your code is pretty much perfect, the only problem is that you chose to sacrifice some performance in most cases (non 0 operands) for the sake of making the rare case faster. Here's a slight improvement based on the assumption that most of the times none of the operands will be 0. The trick is to keep it efficient in the case of 0 without wasting time on ...


6

I don't know which version of C# you use, but as of C# 6 it's possible to set initial values on property definition, which has 2 advantages: Can eyeball the initial values quickly by looking at the property definitions. Doesn't require you to copy the same initial value assignment code into additional constructors you may create. Example initial value ...


4

Not much to say for such a simple algorithm. However there is an optimization that you could employ. Check for odd instead of even, then convert to even. Then divide by 2. At most this cuts your iterations by half. It would look like this: public int NumberOfSteps(int num) { int count = 0; while (num != 0) { if(num % 2 == 1) ...


1

Taking the two answers in this post in consideration, as well as in the linked post, and the discussions, I came up with this solution. I believe it now finally respects all the SOLID principles! public abstract class Animal : IEater { public AnimalKind Kind { get; set; } public AnimalSize Size { get; set; } public bool ...


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


1

This is my refactor based on all the suggestions made here. Works quite nice. Thank you all for your help and teaching me a few things. Stopwatch.cs using System; namespace ExerciseOne { public class Stopwatch { private DateTime timeStart; private DateTime timeStop; private bool isRunning; private void StartTimer(...


5

I'd suggest: public static class AttributeExtensions { public static string GetMyAttributeNameOfProperty<TClass>(string propertyName) { var propertyInfo = typeof(TClass).GetProperty(propertyName); return propertyInfo?.GetCustomAttribute<MyAttribute>()?.Name ?? default; } } Usage: var awesome = AttributeExtensions....


2

As for the primary code I think it's very well written, with good documentation, naming and general structure. Some minor comments: Avoid the use of this.-prefix - it is rather un-C#-ish, unless it's absolutely necessary: public PathCompressor(int[] elements) { this.elements = elements; } if (size < 0 || size > int.MaxValue) An int can never ...


3

Using two arrays of int That's nice. There is a tendency to use Node objects that refer to each other by address, which still requires an array to initially find a node by index anyway, and then the nodes become a pointless extra step. Trap avoided. PathCompressor This class is a bit dubious, it works by "stealing" (temporarily, so maybe "borrowing") the ...


1

As others have covered the technical details of the encodings and encryption, here's example of a more object-oriented approach (via inheritance rather than encapsulation). As encrypting and decrypting have a lot of common elements, this example also aims to reduce code duplication. class App_FileEncryption { public void Run() { var ...


2

As we are working in C#, below is an example of an object-oriented approach, in contrast to the static/procedural approach. UPDATE: In the original answer I neglected to create a Number class to handle the reversing of the value. I have added it. And here's the output: public class App_ReverseNumbers { public void Run() { var list = new ...


4

Invoke is a blocking call that returns only after that call has competed. That means your loop is also including the time it takes to marshal over to the GUI thread and complete. You probably don't want that. I would use BeginInvoke instead, which does not wait for the method to complete on the GUI thread. This is also the difference between ...


2

I agree with @Henrik_Hansen's answer. My answer builds upon his. I absolutely don't see a reason for anything to be static. Your Stopwatch class should only be for a stop watch. Right now you have commingled a UI into the stopwatch. In the name of Separation of Concerns, I see no reason for any Console.Write or Console.WriteLine in the Stopwatch class....


1

Math.Abs(y) >= EPSILON I think this condition is only satisfied for roots around zero on the x-axis. Maybe you confuse yourself by naming the next x as y? A better name would be x1. The actual continue condition should be Math.Abs(x - x1) >= EPSILON and you then can skip the test if (x == y) break; Your rather large value of EPSILON may result in ...


Top 50 recent answers are included