New answers tagged

0

... to produce something that shows off more skills? Requirements + Design + Code = Art. That's the thing. Design Does not implement of the Hangman game. With only a single hardcoded word there is - by design - a one use throw-away thing, not a game. Manually replacing the guess-word requires a program re-write each time. "No, only change the ...


1

You don't need to use a dictionary. Instead, order by the count directly: var champion = test.GroupBy(t => t[1]) .OrderByDescending(g => g.Count()) .First(); Note that this might do the count per group several times while sorting. For a very large number of items, you could improve it by storing the result in a ...


2

Here are my observations ValidGuess I would suggest to either call it ValidateGuess or IsGuessValid I would also suggest to use expression bodied method just like you did with other members private static bool IsGuessValid(string guess) => guess.Length == 1 ? char.IsLetter(guess[0]) : false; I would also suggest to invert the condition. With that form ...


3

for this button handler, I don't see a need for a separated method. However, you need an extension method instead, which would be useful in your case. public static class HttpResponseExtensions { public static void Redirect(this HttpResponse response , string url , Dictionary<string , string> queryString) => Redirect(response , url , queryString ...


2

I am not a web guy, so I will not answer the architecture part here. You could create a helper method to create urls with query strings: public static string CreateUrl( string url, params (string name, string value)[] queryParams) { var builder = new UriBuilder(url); var query = HttpUtility.ParseQueryString(builder.Query); foreach (var (name, ...


1

Your SQL function would only covers 1-day holiday, how about multi-day holidays like Memorial Day, Thanksgiving ..etc.? To cover them, you will need to get the maximum total holidays in your company, and use that number to your query. For example, say your company's holidays are usually 1-3 days, and only one holiday that is 9-day holiday. Then, what you can ...


0

Here is my 4 lines long alternative: var anagrams = from word in words let pair = new { Original = word, AlphabeticallyOrdered = string.Concat(word.OrderBy(@char => @char)) } group pair by pair.AlphabeticallyOrdered into anagram select anagram.Select(@group => @group.Original); Here are my line by line explanations: Iterate through the ...


1

Cryptography Yes, CBC followed by HMAC is fine (also known as encrypt-then-MAC), and you include the IV into the calculations, which is great. You indeed do not have to include the salt and iteration count into the HMAC calculation as they already influence the key being used. You may need to review this choice if you switch to GCM though - including more in ...


-1

If I understand the question, you have problem with printing the message (the code works good)... In your case you forgot to put "$" before quotes and name of the variables into the braces. It should be like: Console.WriteLine($"you have ${dollar}dollars,{quarter}quarters,{dime}dimes,{nickel}nickles,{pennies}pennies"); Try it out ...


0

A better way, in which way ? I'd appreciate if you could discuss with me to find the a better way [...] That's a tricky one. Make it easier to read, optimize it, make the maintenance later on easier ? The better way is way too vague to know how you want it better. What is better to your eyes ? [...] or a way that everyone would understand. This one is ...


2

Extra boiler plating To start with unless there is missing code from DeploymentManagerDbConnection.cs or DbConnection.cs they aren't doing anything interesting or cleaver with the underlying IDbConnction you're wrapping. If anything I would say to directly use it as is. Startup.cs services.AddScoped<IDbConnection>(_ => new SqlConnection(...


1

License counting foreach (IGrouping<int, Installation> group in groupUsersQuery) { if (group.Count() == 1) { copiesNeeded += 1; } else if (group.Count() == 2) { if (group.ContainsComputerType("LAPTOP") && group.ContainsComputerType("DESKTOP")) { ...


1

Just a few quick shots... Instead of having DateTime check15 which leads to the assumption there may be some more DateTime structs laying around like check1...check15 you could just juse currDate which by the way should be renamed to currentDate because abbreviations will make the code harder to read. By using currentDate you could replace (j * ...


3

1. Document your code While it seems a bit boring and useless to do now, in 6 months you will read your code again and it will take some time to remember what this class/field/property is used for... The minimum would be to document with XML comments all the public parts. You can type three slashes /// and Visual Studio will insert the following : /// <...


2

We can rewrite this to be more declarative. Not to mention StreamReader and StreamWriter are fairly low level types, in most cases there is a different call to make to express your intent better. private string writeUser() { string filePath = "usernames.csv"; var found = false; //we don't modify the existing list, don't store it //if ...


2

This is a bit of a reimagining, but I think it will help. Selenium actually has a smart wait helper package called DotNetSeleniumExtras.WaitHelpers. With it, you can greatly simplify your code to just: private void WaitFor(IWebDriver driver, By by, TimeSpan wait) { var smartWait = new WebDriverWait(driver, wait); smartWait.Until(ExpectedConditions....


2

WaitFor This method name is kinda weird for me If I would be your API's consumer then I would except from this naming that it would return with a IWebElement instance (or null) rather than a boolean value. But, if you want to stick to this existence check API then I would suggest: static bool CheckElementExistence(this IWebDriver driver, string ...


1

void ExploreBranch(List<Point> stem, List<List<Point>> masterList, Point target) Based on this signature, I can't understand what this function takes in (what is a "masterList"? what is a "stem"?). Since it returns void, I can't understand what it returns or updates, if anything. Include ALL your code in a short working ...


0

This is clearly a training exercise, and reviews on those are notable different because it makes no sense to address the overkill pattern for the simple implementation of mathematical operations. The implementations is simple specifically because the focus here is on the pattern. Overall, pattern-wise your code is on the right path. Naming-wise, I think you'...


0

Here are my observations: ICalculator As far I as I understand this code's main purpose is to educate yourself. The whole strategy pattern for this extremely simple problem seems an overkill for me. You have written a lots of boilerplate code just to implement something really basic. Without knowing the concrete requirements double might be an overkill, ...


2

Naming Convention current code is not using camelCase naming convention on local variables, and also is not given a good naming to some variables like DisplayFileName, Month, DirectoryCreate ..etc.it should be cameled-case. Some variables have wrong naming like DisplayFileName it should be file because it's used for Request.Form.Files[0] and not to Request....


3

In general, the optimization problems are supposed to be solved using a profiler, which is more effective than the people looking at the code for the first time. In this case I'd also add that before starting to optimize we would want to ask ourselves if the problem is really there. There are very few situations when a supposedly rare operation consuming ...


1

Review Explanation of details my question is focused on the naming The names of the variables and code elements are good, they are following the international standard which is good practice, the full class was not shared, but the code presented is ok Yes, the structure property names are fine and clean documentation The comments are great, but it's a ...


1

A possible addition to the other answers (which I fully agree with) is to design this using classes instead. C# works fine as a procedural language but really is designed with OO in mind. I think it's easier to get into a SOLID mindset for a beginner like that. A short sketch of that: We can start with a definition of a partially solved Sudoku "board&...


1

You can extract the filtering logic like this via LINQ to JSON: var filteredData = from data in source let item = JObject.Parse(data) where !string.IsNullOrEmpty((string)item["ClassId"]) select data; Then you can iterate through the filtered data with a simple foreach loop foreach (var ...


5

Overall, your solver is reasonable. All your functions look well-defined and they should exist. The worst-looking function is SolveSudoku, so that's the one I'll talk about some improvements for. It's OK (very explicit and self-contained, which is nice), but it could be much more readable. FYI The logic you're using is called "backtracking" -- fill ...


10

For a beginner this is actually quite good. For future reference, we can provide better reviews when the entire program is included. This code does not include the Using statements nor does it provide the class declaration. You might also want to consider some of the heuristics of Sudoku, such as solve for the number that appears the most in the puzzle first....


4

Functions are not just about reusing code. I thing the way you "cut" the code using specific functions is quite clean. I would strongly suggest using summaries above functions to describe things, even if your function names are quite explicatives At last, I would probably have a GetSolution() method returning a string that the main function could ...


5

I'm not familiar with Azure Functions and CosmosDb, so I won't review your code from API consumption perspective. Here are my observations: FunctionNameAttribute: Please prefer nameof operator over hard-coded values If you rename your class (from Function1) then the hard-coded value will not change with it accordingly On the other hand if you need to refer ...


2

I always go with the version that would be a best fit to the project. This means, you need to consider the overall project coding syntax, and how much code in the client side, to know where to focus more. Some projects focuses on server side more than using the client side handling, so in this case using the server side would be better to keep the code in ...


-1

You could do something like this with an extension class. I find extensions very useful void Main() { string startTime = "11:15 AM"; string endTime = "11:00 PM"; var mins = startTime.TimeDiffMinutes(endTime); var hours = startTime.TimeDiffHours(endTime); Console.WriteLine($"minutes: {mins}, minutes to hours: {...


3

UPDATE Thank you for adding more clarifications. Since you're working with a web service. There are some questions that I've asked myself with your solution. If the Web API provider updated their endpoints with new keys, do you need to add them to the current solution as well ? if yes, then what purpose the dictionary serves here ? what you will do on each ...


1

Right out of the gate, I would introduce OOP to this monolithic code, but I am unsure whether you are at the right stage of learning to develop to be ready for OOP, because it notably changes how you structure your code. It would also require an entire explanation of OOP principles, which is out of scope for the question at hand and is much better serviced ...


1

Let's take a look on your code: builder.RegisterAssemblyTypes(targetAssembly).Where(x => !_skippedTypes.Contains(x)) .As(type => type.GetInterfaces() .Where(interfacetype => interfacetype.IsAssignableFrom(typeof(ICommandHandler<>)) || interfacetype.IsAssignableFrom(typeof(...


1

Generally speaking, classes with private-only constructors are irrelevant in terms of DI. It makes no sense to include them in your container because the container can't do anything with them. The only exception here is when you register the specific instance of a type with the container, which only really makes sense in scope of a singleton, e.g. something ...


Top 50 recent answers are included