Hot answers tagged

54

Your inner loop appears to be unnecessary. Why not create a lookup: var knownAccountNumbers = new HashSet<string>( dataStructure.Tables["AccountData"].Rows .Cast<DataRow>() .Select(row => row[0].ToString())); Now your loop is simply: foreach (DataRow row in dt.Rows) { var accountNumber = row[0].ToString().Replace("\""...


41

Your loop is close to 200 lines, that's waaaay too long. That's long for a full class, let alone one method, let alone part of one method. Why do you use for (int i = 1; i < count - 1; i++) when you don't do anything with i? Why not use foreach(var line in lines)? Oh wait, I see, you've called your collection line -- a bad name, since this doesn't tell ...


40

Why web-scrape, when you can get the data properly through the YouTube Data API, requesting the mostpopular list of videos? If you make a GET request to https://www.googleapis.com/youtube/v3/videos?key=…&part=snippet&chart=mostpopular, you will get the same information in a documented JSON format. Using the Python client, the code looks like: ...


33

.*?;.*? will cause catastrophic backtracking. See this post on more details on the problem: http://www.regular-expressions.info/catastrophic.html To resolve the performance issues, remove .*?; and replace it with [^;]*;, that should be much faster.


18

I'm sorry to have to say so, but reading back over the answers to your two previous questions, it seems to me as if you've been given some bad advice. In particular, this answer advised you to switch to using the csv module. But that was a mistake, because: The csv module has to handle all the details of the CSV format, which can be quite complicated (...


18

1) I would save constants (doubleQuote, etc.) as fields, so they don't take up extra space in already fairly large method body. 2) I think your use of goto is fine. However you can also rewrite it without goto. At first glance it boils down to: var buffer = new StringBuilder(); var line = new List<string>(); foreach(var ch in csv) { var newLine = ...


18

Welcome to Code Review! Kudos to writing a fairly large program. Several things pop-out from your program. But, a few things first. If you are using any intelligent editor; please see if you can get a python linter (or a PEP-8 integration) in it. PEP-8 is the python's style guide, which makes the code consistent, and hence, readable/maintainable. I noticed ...


17

Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems. -- Jamie Zawinski A few things to be commented : Regular expressions might not be the right tool for this. .*?;.*?;.*?;.*?;.*?;.*?;.*?" is potentially very slow and might not do what you want it to do (it could match many more ; than what ...


17

You don't need an include guard in Main.cpp #ifndef MAIN_CPP #define MAIN_CPP Include guards are meant to avoid a header file from being included more than once by the compiler/preprocessor (#include works just like text copy-pasting, so yes, the preprocessor is that dumb). For a struct that has all of its members publicly accessible, having get* methods ...


17

Well, you could find a "Pythonic" way to skip around a file, but that still wouldn't make it good code, in my opinion. There are some fundamental issues with your approach that make it rather hackish: f.seek(0) means that the file isn't being processed sequentially, and creates extra work. You're hard-coding ano = 1977 when in fact ANO 1977 would be right ...


17

Instead of using linq lookups for the lists you should map your data into dictionaries, which are MUCH faster to access specific instance of your data, memorywise you might take a hit but the mapping is just onetime. var countries = new List<Country>(); var countriesByCode = countries.ToDictionary(c => c.Country_Code, c => c); Country cntry; if ...


17

Do not call main recursively. You are setting yourself up for stack overflow. Consider instead def main(): while True: try: your_logic_here except Exception as e: your_logging_here Testing for counter == 4 is better done in a loop: for _ in range(4): handle_acceleration handle_the_rest An ...


16

Use split, like so: >>> filecontent = "13P397;Fotostuff;t;IBM;IBM lalala 123|IBM lalala 1234;28.000 things;;IBMlalala123|IBMlalala1234"; >>> items = filecontent.split(";"); >>> items; ['13P397', 'Fotostuff', 't', 'IBM', 'IBM lalala 123|IBM lalala 1234', '28.000 things', '', 'IBMlalala123|IBMlalala1234'] >>> I'm a bit ...


16

The first thing I would do is break that up somehow. Define a struct to encapsulate the three common properties between each stat: public struct PlayerStat { public int Rank { get; } public int Level { get; } public int Experience { get; } public PlayerStat(int rank, int level, int experience) { Rank = rank; Level = ...


15

I have a few questions about your design. Classes What's the purpose of using classes if all of your data and methods are public? Why bother having getters and setters like getMlvl(), getClvl(), getDlvl()? Personally, I'd make all the instance variables private and leave the accessors because if you later expand this it will be difficult to track who's ...


15

Context manager You open a file at the beginning of the program and close it explicitly at the end. Python provides a nice way to allocate and release resources (such as files) easily: they are called Context managers. They give you the guarantee that the cleanup is performed at the end even in case of exception. In your case, you could write: with open('...


14

Few comments: Make your code more abstract, use IEnumerable instead of List public interface ICsvReader { IEnumerable<Contact> ParseCsv(string filePath); } Provide an asynchronous version public interface ICsvReader { IEnumerable<Contact> ParseCsv(string filePath); Task<IEnumerable<Contact>> ParseCsvAsync(string ...


14

Including the signature, scope opening and closing braces, and all the superfluous vertical whitespace you've got here, the Read method has 223 lines of code, checks if the end was reached in 7 places and includes 4 goto instructions. Other answers already stated it, but I don't think it can ever be stressed enough: goto is a synonym for bad control flow; ...


14

The code appears to be broken, you're not break;ing after finding a match so all records will likely have row[12] == "N". You should really be doing a join on accountNumber: var matchingRows = from DataRow row in dt.Rows let rowKey = row[0].ToString().Replace("\"", "") join DataRow queryRow in dataStructure.Tables["AccountData"].Rows on ...


14

Don't do wildcard imports Use import tkinter as tk and then prefix all tk classes and commands with tk. (eg: tk.Tk(), tk.Frame(...), etc). PEP8 discourages wildcard imports, and for good reasons. They pollute the global namespace, and they can overwrite variables and classes without you knowing. Tkinter is particularly susceptible to this since both tk ...


14

It looks like you found a pretty nice approach there already. A few general things stand out: Using globals Magical constants and unsafe C-style arrays/raw pointers: static vector<uint64_t> *movie_map = new vector<uint64_t>[1200000]; static vector<uint64_t> *actor_movie_map = new vector<uint64_t>[2000000]; I'd suggest using std::...


13

The code can be simplified and improved in several ways, and the inner loop can be made tighter. Let me show you how: private static List<List<String>> readTXTFile(String csvFileName) throws IOException { String line = null; BufferedReader stream = null; List<List<String>> csvData = new ArrayList<List<String>>...


13

Meeting the specifications The specifications said to accept a CSV file, so your test data should be comma-delimited, not space-delimited. In practice, it's not a big deal, but if you're answering an interview question, don't deviate from the instructions unless you can justify it with a good reason. Your output looks like this: [['Company D', '1991', '...


13

Performance Your performance problem is primarily due to the duplicate_id() function, namely the if id in id_list test. Testing whether an n-element list contains a particular item is O(n). If you call that for n rows, then it's O(n2). That would not be acceptable at all for large files. Sorting the input would help bring it down to O(n log n). But ...


12

Again just a few stylistic issues. Make it more LINQ-ish: Do all your list processing in one statement. Try to avoid converting between List<>s, arrays, and IEnumerable<>s unless you have to. Look for places where you're iterating over an enumerable using for or foreach, or where you're calling .Add() in a loop; these can almost always be ...


12

Kudos for creating a struct to strong type the data, but what does Index mean? It's an odd term for course information. I'd look for a better name. Also, don't string type all the things. You have a date here, you should store it as a date. public string CompletionDate { get; set; } MSDN article on parsing dates from strings. You're duplicating string ...


12

Have you already executed the code to see how it performs and if the battery will last? There is that famous Donald Knuth quote saying premature optimization is the root of all evil (or at least most of it) in programming. I never had to think about the energy consumption of a program, so I cannot tell you about the power efficieny. But as vnp already did, ...


11

1. Introduction By coincidence, I'm working on a similar problem right now, so here's a chance for me to write up an experiment I ran today, in the hope that it might prove useful. However, the solution presented below is far from ideal (see section 3). 2. The idea Processing your query results through Django's ORM and then through csv.writer is time-...


11

Use do...end for multiline blocks. You're already doing it for the innermost block, but not the File.open block for some reason CSV.foreach accepts a hash of options. One of them is :headers which you can set to true or :first_row. This will skip the header row when iterating (so you can skip skip and the if..else-branching), and let you refer to a column ...


11

The biggest problem with gotos is that they make it really hard to refactor code easily. Your loop body alone is 66 lines of code long and has a nesting depth of 5. Length of code and nesting depth are two of the prime indicators of complexity and are pretty good indicators for bugs. To reduce both of those metrics you usually break up your one large ...


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