New answers tagged

0

This looks like a good attempt. Let's first start assuming we only have to deal with positive values. private int getProductOfMaximalsForMoreThanTwoElements( List<int> B) { int max1 = B.Max(); B.RemoveAt(B.IndexOf(max1)); int max2 = B.Max(); B.RemoveAt(B.IndexOf(max2)); int max3 = B.Max(); return max1 * max2 * max3; } This ...


2

The first step in your method is to determine the longest common prefix. Here it would be sufficient to determine the length of the longest common prefix. Building the prefix string itself is not necessary at this point. The next step is to decrease that length until it divides both string lengths. An explaining comment would be helpful here. We still don't ...


0

One potential security issue, as mentioned in this answer to a similar piece of code, is that the use of SequenceEqual() in CheckIfPasswordMatchesHash() may open this up to timing attacks.


0

I'm not that great at C#, however in other languages when I come across something like this, I usually write a private or helper function to iterate through the object so my code doesn't look like large blocks of checks. Something like private void checkDays(WorkingDayRequestDto dayObject){ string [] days = new string[] { "Monday", "Tuesday", "...


4

WorkingDayRequestDto workingDayDto The name of this object is IMO misleading because it seems to represent a whole week. I would call it WorkWeek. If it's possible to change the data model, I would invent a WorkDay object with a Start and End time: public class WorkDay { public WorkDay(DateTime? start, DateTime? end) { Start = start; End = end;...


0

How about you made a list for EndForDays and StartForDays and do loop? For example, I made a code snippet by Java. List<Boolean> EndForDays = Arrays.asList(true, false, false); List<Boolean> StartForDays = Arrays.asList(true, true, true); for(int i = 0; i < EndForDays.size(); i++) { if(!EndForDays.get(i) && StartForDays.get(i)) {...


1

Your method visits mosts nodes twice: Here int leftDiameter = DiameterOfBinaryTree(root.left); int rightDiameter = DiameterOfBinaryTree(root.right); int leftHeight = Height(root.left); int rightHeight = Height(root.right); calling DiameterOfBinaryTree on the left and right subtree determines the height of both subtrees, and then calling Height on the ...


0

Would it be better to unfold the reccursion to iteration? As I understand calculations are performed on back propagation (after you reached the leaf). With an iterative approcah you can avoid aditional stack allocation and perform calculations on forward propagation - no need to return to root in order to get the result. Update 1: According to letcode ...


0

let's suppose I have a file in the following format: (header: 0AAAANNN .....), (Record ( type 1 ): 1NNNNNDDD ...), (Record ( type 2 ): 2AAAAAAAA ....), (N records of type 1 and 2), (Trailler: 9SSSSSSS), how can this be implemented in your logic? You could use an extension method to parse: static class RecordReader { public static Record ReadRecord(this ...


1

To the many useful points already made I have only a small contribution to add: public class Test { public static void Main() { BowlingGame newGame = new BowlingGame(); // Test first game - SUCCESS! (190) //newGame.RecordFrame(4, 6); //newGame.RecordFrame(3, 6); //newGame....


8

Inconsistency This is a minor thing, but I tripped over it when skimming your code: throw2 = (throws.Length == 1) ? 0 : throws[1]; throw3 = (throws.Length == 3) ? throws[2] : 0; Why did you invert one of the ternaries? It'd be easier to read if you kept the same structure, e.g. throw2 = (throws.Length >= 2) ? throws[1] : 0; throw3 = (throws.Length >...


2

public interface IOneWayEncryptionService { string Encrypt(string text); } public interface ICryptographyService : IOneWayEncryptionService { string Decrypt(string text); } These seem to be crippled. The basic operations on which almost everything else can be built are byte[] Encrypt(byte[] plaintext, byte[] key, byte[] iv) byte[] Decrypt(byte[] ...


3

The question seems to be missing an important constraint in case there are repeating elements in the input collection that satisfy the condition given. Therefore, there is an assumption around the solutions provided that we are interested in the first encounter only. How about a more general approach to return an IEnumerable collection so that the caller ...


5

Private versus public public int frame = 0; // The current frame public int frameScore = 0; // The total of first two throws in a normal frame public int[] throws; // Input public int throw1; // First throw in a frame public int throw2; // Second throw in a frame public int throw3; ...


5

People usually pay not much attention to that, but redundant *Manager, *Processor, *Service suffixes are just some form of signal noise. It is also questionable that two way is really needed the same time at the same place on the consuming side. I would go with: interface IEncryptor { byte[] Encrypt(byte array); } public static class Encryptor { ...


1

Let’s capture parsing/formatting logic in the attributes to simplify extensibility. You would need to inherit the following base attribute class to define new column types: [AttributeUsage(AttributeTargets.Property)] public abstract class FieldAttribute : Attribute { protected FieldAttribute(int length, int order) => Length = length; ...


4

I will try to focus on things that have not been said, but I apologise in advance if I have repeated something that has been mentioned in any previous answers. Separating your code into Constatnts, Enums and Interfaces folders probably isn't particularly helpful. Especially when you have only one or two files in each folder. Similarly naming a folder src ...


4

Disclaimer: my first post here and I'm not C# programmer If the input is different, the function will of course call itself again (that's why it is a recursive one, right?) This is dangerous, as user can keep entering wrong inputs, which will increment scoping stack (eventually resulting in StackOverflowException being thrown). Even doubly so, your ...


3

All in all, you did quite well. If you have the option, I'd recommend letting a system like Active Directory handle user credentials rather than storing and validating themselves. However, as there's a cost involved with that, password hashes are the next best thing. PBKDF2 is a better choice than what I'm used to people using and you're salting it too, so ...


0

I think the following would be the reasonable approach to implement it. Please note that To prefix in ToIterable documents full sequence materialization as framework design guideline naming conventions dictate - compare to As prefix. It would be interesting to figure out how AsIterable could be done :) public static class Iterable { public static ...


0

So what you are trying to do is to have an implementation of IEnumerable<T> which has a singleton IEnumerator<T>? Why not implement that explicitly, instead of using undocumented (?) behaviour of List<T>'s iterator? Simply create a class that consumes a IEnumerable<T>, takes its iterator and wraps it in an implementation of ...


4

Next step: Make it Expression<>-oriented. Making your own math tools is tons of fun and practical! Since you're asking about potential improvements, my suggestion is to move toward Expression<>-oriented coding next. First, you define: public abstract partial class Expression<T> { public T Evaluate() { return this....


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


4

The comment here int[] numbers = new int[nums.Length +2]; // we add 2 because of the question is not really useful, it requires to read the entire question to figure out why 2 is added to the number of balloons. I would suggest something like // Allocate array for all balloons, plus the two "imaginary" balloons // at positions -1 and n. which closely ...


3

Global static state makes your code untestable... I would define your car make lookup as: public class CarMakeLookup { public CarMakeLookup(IEnumerable<(short Id, string Make)> data) { Makes = data.ToDictionary(d => d.Id, d => d.Make); Ids = data.ToDictionary(d => d.Make, d => d.Id); } public ...


7

New line character differs depending on system. Can't rely on \n. Let me introduce you to your new friend, Environment.NewLine Concatenating strings in a loop is very inefficient. It's recommended to use StringBuilder instead Code reuse. There are a couple of spots where duplicated code can be extracted away. Iterating an array and setting values. This ...


7

Collected thoughts: Your Helpers class is weird. It mixes e.g. command line input (GetArrayFromSplitInput) with a bad pattern that you don't need, that is, the RunCommandIfTrue method: just use an if statement where you need it instead of using higher-order functions! You just make your code unnecessarily harder to read at the call sites of RunCommandIfTrue....


6

Styles and conventions The using should always be at the top of your file. Variable names like _f means...? Facing? Face? Force (obviously not, but see where I'm going?)? Don't hesitate to use meaningful variable names. Instead of _f, the variable could be named direction since it's a Direction object. The indentation of the switch is real bad. But I'd ...


2

private void initializeImages() { string appRoot = System.IO.Path.GetDirectoryName(Application.ExecutablePath); files = System.IO.Directory.GetFiles(appRoot + @"\Resources"); files2 = System.IO.Directory.GetFiles(appRoot + @"\Resources"); files3 = System.IO.Directory.GetFiles(appRoot + @"\Resources"); files4 = ...


4

Some things come to mind when I read your code. There is a nice feature on arrays in C# called CopyTo(). It gives you the possibility to copy an array without using a loop. Like so: int[] newNums = new int[n]; nums.CopyTo(newNums, 1); newNums[0] = newNums[n-1] = 1; //This line I really like. There is also something called a multidimensional Array which ...


2

Here is another generic and deferred extension method version that seems to work for me. We iterate through all sequences one by one at once and only stop when there is nowhere to go within any sequence or the number of items requested have already been yielded. public static IEnumerable<TIn> FecthFromEach<TIn>( this IEnumerable<...


10

Your commenting style needs improvement Either you comment all methods or none This is generally not a good rule of thumb, but for coding tasks in a job interview setting can show consistency If you comment a method use "DocComment" style, i.e. ///<summary> /// Moves robot one unit in direction faced ///</summary> public void Move() { ......


11

A bug: Transpose does data[i, j] = data[j, i];, but it returns a new matrix, it's not supposed to change the matrix in-place, and that doesn't actually work (going out of range for a non-square matrix and losing data for a square one). Manual loops to copy between arrays: did you know you can use Array.Copy on multi-dimensional arrays? The indexes work as ...


18

I like that your braces and indentation is beautifully consistent. But, this is C#, and your Java is showing ;-) - Kudos for the mostly-consistent PascalCase type and member names, but it's the K&R same-line { opening brace that clashes with the typical Allman next-line { opening brace standard most people would expect of C# code: public DMatrix(int n) {...


5

Great job. Here are some minor notes though. In newer versions of C# you can leverage async Main in order not to wait for your test method. public static async Task Main(string[] args) { await Test(); } You can speed things up a bit with executing your tests in parallel public static async Task Test() { var successExecute = Execute("https://...


2

using (SqlConnection sqlConnection = new SqlConnection("Data Source=;Initial Catalog=;Persist Security Info=True;User ID=sa;Password=P@ssw0rd;pooling=true")) { ... } finally { sqlConnection.Close(); } } You're correctly using a using statement for the connection, which will both close and dispose the connection ...


0

The answer is the Unit Of Work pattern. You are right that using is modern only it can not be used if you have a Unit Of Work in your repository that has lets say 3 write actions in it that are part of ONE transaction, but can be split in 3 writing methods. For those 3 write actions you will need the same context from the same repository. Most of those times ...


4

A few things I noticed: In maskPass after each ReadKey you're either concatenating a character with pass or you're assigning pass to a Substring of pass. This is very inefficient. Each concatenation and each call to Substring, creates a new string. It would be much better to use a StringBuilder to store the string until you're ready to return it. In ...


3

What about a simpler approach? public class PD0Format { public static PD0Format Empty => new PD0Format(new byte[35]); public static PD0Format Parse(string text) => new PD0Format(Enumerable .Range(0, text.Length / 2) .Select(i => Convert.ToByte(text.Substring(i * 2, 2), 16)) .ToArray()); ...


2

The biggest improvement would be to keep track of potential matches while you are finding if the current match is valid. As Raymond mentioned, in his comment, you could look at KMP or Boyer Moore algorithms but you trade performance for complexity. Searching though 900000000 Byte will just take some time. There is no magic answer for that but I did a ...


1

Implemented Henrik Hansen's suggested edits: Define an abstract class, PD0Bytefield, to encapsulate how byte fields are specified (PD0Bytefield.StartHexDigitIndex and PD0Bytefield.HexDigitLength) with abstract methods to specify how they should be casted when read from (PD0Bytefield.Extract()) and how they should be written to the underlying string (...


4

Implemented Henrik Hansen's suggested edits: Removed goto statements in switch statements of indexer HexToString() now uses Encoding.UTF.GetString() to convert hex to strings StringToHex() now uses LINQ .SELECT(...) to convert bytes of byte[] to string Added default constructor to initialize _hasciiData to string of 70 empty zeros Structural changes to ...


-2

public class SqlDatabaseUtility { //static string ConnectionString = @"Data Source=192.168.248.237\SQL;Initial Catalog=TestDB;Persist Security Info=True;User ID=appuser;Password=123456;Connection Timeout=300"; private static StringBuilder ConnStr = new StringBuilder(); public static void CreateSqlConnectionString(string Server, string Database, ...


-2

public static Dictionary<string, object> ExecuteQuery(string connectionName, string storedProcedure, Dictionary<string, object> parameters, Dictionary<string, object> outParameters) { // the usings will gracefully dispose/close everything disposable Dictionary<string, object> result = new Dictionary<...


9

There are not much to review, so to answer your questions: It's a good extension if you can see it useful in more than one place. It makes sense. I can't see anything else to consider - a null check on the predicate maybe? If the exception thrown by the predicate when @this is null is good enough for you, don't bother further. I don't see why the ...


2

Task.Run schedules the work specified by the lambda to be executed on the thread pool. Scheduling the work and managing the thread pool is costly in terms of CPU time, allocations and possible thread starvation. Task.Run is useful for CPU-bound tasks but usually a smell in asynchronous code in web applications. Most of the time it hints at misunderstanding ...


4

I would not do that. Sure you can replace strings if not valid but you're going down a rabbit hole, better force valid json or you will keep getting more and more errors if you're not fixing them upstream. Also what's the json standard, there are a few that are valid I use JsonSerializerSettings settings = new JsonSerializerSettings() { Formatting = ...


7

ByteSpecification spec = null; switch (category.ToLower()) { case "date": spec = dateSpec; goto case "cast"; case "constants": spec = mathConstantsSpec; goto case "cast"; case "secrets": spec = secretsSpec; goto case "cast"; case "cast": string hascii_bytes = ...


3

Your code is somewhat messy because almost everything is inside a single Program class. It can use some abstractions. Few things off the bat: 1) Declare and implement a strongly typed api for your service. For example: interface IChuckService { Joke[] GetRandomJokes(Category category); Category[] GetCategories(); Name[] GetNames(); //other ...


1

An offline code review came up with a critical piece of feedback: The constructor for Task takes an Action<T>. The async delegate therefore generates an async void method, which has 2 implications: If an unhandled exception is thrown from the async void method, it will crash the process (unlikely, since it's just calling Task.Delay(), but it's an ...


Top 50 recent answers are included