Stack Exchange Network

Stack Exchange network consists of 175 Q&A communities including Stack Overflow, the largest, most trusted online community for developers to learn, share their knowledge, and build their careers.

Visit Stack Exchange

New answers tagged

1

Review Firstly, is there a name for this variant of the visitor pattern? You have made what I like to call the inviting visitor pattern. I'm not sure this pattern is common, but I have seen it before, disguised and usually involves 3 (types of) classes instead of 2, each with their own concern. inviter: let reference invite visitor reference: let ...


2

Just an idea: Define FrameworkUser as: public class FrameworkUser<TKey> : IdentityUser<TKey> where TKey : IEquatable<TKey> { // Either you could do this, if ParentUserId is relying on ParentUser public virtual UserId<TKey>? ParentUserId => ParentUser != null ? ParentUser.Id : (UserId<TKey>?)null; //.. or ...


0

This is supported by convenience extensions: public static FingerprintBuilder<T> For<T>( this FingerprintBuilder<T> builder, Expression<Func<T, string>> expression, StringOptions options) { Func<string, string> format = input => { if (ignoreCase) { input ...


1

I don't think it can be done at design time given generic constraints. Second best option would be at runtime. There are 2 runtime constraints: TNullableKey must be null-assignable (reference type or nullable type) The underluying type of TNullableKey must be of type TKey code public class FrameworkUser<TKey, TNullableKey> : IdentityUser<TKey> ...


1

Data Model I would add a restriction on the attribute and move the convenience method from the creator to this class. [AttributeUsage(AttributeTargets.Class)] class ParentAttribute : Attribute { public ParentAttribute(Type type) { Parent = type; } public Type Parent { get; private set; } } [AttributeUsage(AttributeTargets.Class, AllowMultiple=...


4

Prelude I'm using the static member Depth to keep track of how deep the visitor has gone. Could it be problematic having state on my extension methods static class? It would be uncommon. In fact, I wouldn't use extension methods to begin with. Tree walking should be performed by a dedicated tree walker. Would the SurveyPart abstract class make ...


-1

This is the new code with @Henrik Hansen's suggestions. #region Editor code class Editor { Buffer _buffer; Cursor _cursor; Stack<object> _history; public Editor() { _buffer = new Buffer(new List<string>()); _cursor = new Cursor(); _history = new Stack<object>(); } public void Run() {...


2

I think the original API is really not as intuitive as I thought. I simplified it by replacing this public static FeatureService Configure<TScope, T> ( this FeatureService features, INamespace<TScope> scope, Expression<Func<TScope, T>> feature, Func<FeatureOptions, FeatureOptions> configure ) where TScope : ...


3

Review Keep in mind I am using an older version of .NET so my semantics can be somehow verbose. Navigation You are lacking a reference back to the parent, which makes bottom-up navigation much harder. A tree that knows its parent allows for much more flexibility in navigation. root.children.Add(new Tree() { id = node.id, text = node.text, ...


2

Review Is this helper intuitive and easy to use? In the code snipper below: INamespace<TScope> scope seems unused? public static FeatureService Configure<TScope, T> ( this FeatureService features, INamespace<TScope> scope, Expression<Func<TScope, T>> feature, Func<FeatureOptions, ...


1

Review Don't trust parameters of public methods purely on their name orderedArray. I will address how to handle this further in this answer. private static string AddDashesBetweenConsecutiveNumbers(int[] orderedArray) { You mentioned but this code has some disadvantages, especially in readability of the LINQ expression We could create a ...


1

Lexicon I'm a fan of using common terminology for best practices. You are creating a lexer rather than a parser. Parser: Within computational linguistics the term is used to refer to the formal analysis by a computer of a sentence or other string of words into its constituents, resulting in a parse tree showing their syntactic relation to each other, which ...


3

There are a lot of improvements that had to be made, the most important for you to understand are arrays, especially 2D arrays; for loops; and ternary operators. using System.Threading; using static System.Console; namespace Test1 { class Program { const int boardWidth = 7; const int boardHeight = 10; static bool[,] lines = ...


1

I've thought about a factory method, but that won't work because it can't be translated to SQL by the expression interpreter. Why don't you call (as suggested by t3chb0t) AsEnumerable() right before your factory method? You would perform the factory method in memory. var project = entities.Tickets .Where(p => p.ID == id) .AsEnumerable(...


0

I can't check this with a compiler right now but general idea is that you can benefit from IQueryable. Define method like this IQueryable<ListProject> SelectListProjects(IQueryable<Tickets> tickets) { return entities.Tickets .Where(p => p.vonProjekt == id) .Where(p => p.phaseID == 0) .OrderBy(p => p....


4

If I were in a position to be unaware of them and this is the code I managed to write, what's your thoughts on it? No need to call ToCharArray() because you can iterate over the chars of a string as well By using an else if instead of the second if you won't need to check for toChar[i] == '0' but basically you only need a else. Using a foreach loop ...


2

General Guidelines Please put any technical restrictions in the initial post. Like, for instance, framework 3.5 :-( Declare variables as var rather than their type for readability. Split big methods into smaller ones for readability and testibility. Performance Your biggest bottleneck is the sequential loop of 7000 users. You could use the TPL to maximize ...


0

Review You define ReturnedValue, use it as parameter for MethodInfo.Invoke, but could have also stored (T)Arguments[1] and default(T) (your last return statement) to it. This way you could call return ReturnedValue once at the end of the flow. Please use camelCase in C# variable names. Alternative Solution So I got sick of several things about the way ...


4

Let's start with the basics: names and namespaces. To me the following names have the following meanings: Deck: either the set of 52 cards (basically a wrapper around ISet<Card>), or, more likely, the cards which haven't yet been dealt (basically a wrapper around IList<Card>). In the latter case I'd expect it to have methods to deal from the ...


4

Performance Your code tests against all possible hand result kinds. Is this useful or could you abort whenever you get a match? Is performance an actual requirement anway? public HandResult GetHandResult(int playerID) { IEnumerable<Card> set = GetPlayerSet(playerID).OrderBy(x => x.Face); return new[] { ...


2

You're checking both the start and end twice and you only modify when both start and end have duplicate letters. Is that intentionally?: if (firstName[0] != 'a' && firstName[firstName.Length - 1] != 'l' && firstName[firstName.Length - 1] != 'e') { if (firstName[0] == firstName[1] && firstName[firstName.Length - 1] == ...


0

Review The initial question did not state it had to work only for positive integers. But after an edit, it does, rendering my answer mute. Anyway, I leave my answer up for those how are interested in an altered solution to work for all integers, regardless of sign. public static void Main() { // 11111111111111111111111111110011 Console....


2

There is no need to convert the integer into a string. That's too expensive in terms of CPU usage and memory consumption. As I answered in a Java question on the same topic a few days ago, there's a simple solution using only integer arithmetics. That solution can be easily translated into C#.


1

I think that part of the exercise is to make you re-invent the wheel just to learn that documentation is your friend, discovering something you just did already existed, in your case you really wrote everything also the Math.Max. Part to become a good programmer is find a way to make your code reusable, so one BIG advice is refactor your code in a way that ...


3

If you're just looking for the longest stretch of zeros you don't need to use a list at all. You just iterate through the binary representation until you find a stretch of zeros. If you hit a new high score, you can overwrite the old high score with the new one and keep moving forward, forgetting about the past. This is called a "greedy algorithm." ...


1

The adviced way to call a service endpoint when the connection is not an open stream, is to have the lifecycle of you client in line with the operation you are calling. So rather than storing an instance of the client.. private WebService() { client = new MyWebServiceContractClient(); } You should create a client on demand. To work around ...


3

If you determine the valid types to handle in code as it seems from your method, I would do it more rigidly than by using Reflection. There is no meaning in determining the byte length by "hand" and then afterwards call a method by Reflection relying on consistency in names (not to mention possible types with the same byte length). Why not try something ...


0

From the problem statement, we could lay out classes like below: class Product { private String name; private doublt price; //Getter setter, constructor etc } class Book extends Product implements NonTaxable { ... } class Food extends Product implements NonTaxable { ... } A product can be: interface Taxable { float taxRate(); } Or, it ...


2

Proposed Solution public static JObject ModifyDoubleIntegers(JObject source) { if (source == null) return null; var target = new JObject(source); var properties = target.DescendantsAndSelf().Where( x => x.Type == JTokenType.Float).Select(x => x.Parent).OfType<JProperty>(); foreach (var property in ...


5

case nameof(UInt16): return 2; case "Byte": return 1; Be consistent. nameof is clearly preferable. private void ConvertArray(byte[] response, ref object[] items) { int index = 0; for (int i = 0; i < items.Length; i++) { var item = items[i]; var itemType = item.GetType(); var itemSize = ...


1

A simple improvement for readability could be to eliminate the - 1 in the indices by replacing size with max = mat.GetLength(0) - 1;: void rotateMatrix(int[,] mat) { int max = mat.GetLength(0) - 1; for (int x = 0; x < (max + 1) / 2; x++) { for (int y = x; y < max - x; y++) { int temp = mat[x, y]; // save 1 mat[x, ...


1

You could allow for some more flexibility in type conversions by using type converters. public static T Get<T>(this IDictionary<string, object> dictionary, TypedKey<T> key) { return (T)Convert.ChangeType(dictionary[key.Name], typeof(T)); } static class DictionaryExtensions { public static T Get<T>(this ...


2

Please review for performance. The performance looks fine to me. Some key observations: It's clear that all nodes must be visited to compute the correct answer, so the solution cannot be better than \$O(n)\$ time. Traversing in level order as you did will require as much additional space as the number of nodes on the most dense level. Traversing depth ...


2

Is it good to map every DTOs into Proxy's DataContract? As my this class will grow with more service calls with different DTOs. Yes, mapping to your own domain improves modularity by decoupling the DTO library from your domain. How this class should be architect so that it doesn't violate SRP/Design-patterns? Use either a custom or existing mapping ...


1

You should not instanciate a new DbContext each time your code go throw your ActionFilter. What you should do is to use dependency injection and to define an execution scope. Because you are using .net Framework and not .net core, I advise you to look into DI providers such as Autofac or Ninject. I advise you to look into why to use DI and think about ...


0

I think, I would hold on to your decorator pattern because it is more reusable than the selection with anonymous objects. public class DistancedAgent : Agent { public DistancedAgent(Agent source, double distance) { AgentId = source.AgentId; Latitude = source.Latitude; Longitude = source.Longitude; Distance = distance; ...


2

The time complexity of Sum is \$O(m n)\$, because in the worst case it may need to enumerate all nodes. An \$O(m)\$ implementation is possible with \$O(n)\$ extra space, and a bit of extra work in Insert: Add a Dictionary<string, int> to track the current values of the keys, let's call it values. Use TrieMSNode.Value to store the sum of all values ...


7

var agents = db.GetAgents(); if(agents == null || agents.Count == 0) { Console.WriteLine("No Results"); } You should exit your application/code block when you detect an invalid state. Here even if "No Results" is printed, the app will still run into a NullReferenceException or IndexOutOfRangeException in the next few steps. Also, talking of ...


4

I'd say you have the following options: Keep that new class if you think it's relevant in your business. Here, it should be important to know if the logic of calculate the distance should be on that web service (I assume that 'ws' variable means that) or within your model (i.e., the 'Agent' class). Use an anonymous class if you think you won't pass that ...


1

it is usually a good idea to use the shared pool to maximize re-use; however, if you're specifically worried about code incorrectly (or maliciously) snooping into your arrays (without even needing a memory debugger - just by "returning" things to the pool, but keeping a reference to them), then you might want to have your own pool; of course, this is not a ...


8

I don't have much to add to Henrik's answer at a low level, but I think you've missed the point of the exercise at a high level. The spec could be implemented as simply as public class MapSum { private readonly IDictionary<string, int> data = new Dictionary<string, int>(); public void Insert(string key, int val) => data[key] = val; ...


8

In Insert(...) you should use TryGetValue instead of ContainsKeyfor efficiency: foreach (var letter in key) { if (!current.Edges.TryGetValue(letter, out var edge)) { edge = current.Edges[letter] = new TrieMSNode(); } current = edge; } Name your methods after what they do, not after their implementation: DFS(...) could be ...


2

If you decide to go the generic way, without applying attributes on your properties, we could build further on Adriano Repetti's answer. My starting point is the IsNullOrEmpty method, which is invoked against any property in the tree of properties of any specified complex object or list of properties of any specified simple object. The goal is to test ...


5

Given that you say you're fetching the data from an SQL database, you should use it. It should be much faster to filter the data with a WHERE in the SQL query than to serialise it, deserialise it, and then filter it without the benefit of any indexes. Given this tiny fragment of code it's impossible to give more detailed advice on how to do that. It depends ...


1

Custom vs. normal enum This looks like some kind of custom enum type, but in its current form it's not very useful: All properties in PrinterAttribute are protected, which means that only derived classes can access them. That's not very useful for other code. A property of type Enum is cumbersome to use because it's too weakly typed. It only tells you that ...


1

Inspired by JanDotNet's answer, I have made an attempt at improving the OP's code using best practices. I am using both the M-V-VM and State Machine pattern. View The calculator view is no longer a part of MainWindow. MainWindow <Window x:Class="Calculator.MainWindow" xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" ...


5

As seen with @t3chb0t and @dfhwze I have been misusing the TPL. I also don't see any point on .ConfigureAwait(false) a Task.WhenAll() call, also with the async all-the-way rule, using an awaitable task for this case would have me to convert many of my APIs to async one. So they provided me these solutions : Modifiy my task creation logic this way : tasks....


2

The algorithm can be rewritten to use Linq instead of a Queue. public IList<IList<int>> LevelOrder(Node root) { IList<IList<int>> result = new List<IList<int>>(); Queue<Node> Q = new Queue<Node>(); .. This increases readability. (At the cost of performance?) I use IEnumerable ...


1

A recursive approach that takes into account the threshold of max depth 1000. The threshold of 5000 nodes is ambigious, because what behavior do you expect when there are more nodes? using System; using System.Linq; using System.Text; using System.Collections.Generic; using System.Globalization; using System.Text.RegularExpressions; public class Program { ...


2

I would use regular expressions to make a simple calculator that can only perform one binary operation. There is less overhead with tokenizing and casting. If you do decide to keep making calculator more and more complex, I suggest using the lexer-parser approach instead. using System; using System.Linq; using System.Text; using System.Collections.Generic; ...


Top 50 recent answers are included