Hot answers tagged

55

You don't need multiple instances. In fact, HttpClient is designed specifically to persist and be used for multiple requests (see: HttpClient.DefaultRequestHeaders). There's already a lot written about this so I'll redirect you to those resources: Do HttpClient and HttpClientHandler have to be disposed? Is HttpClient safe to use concurrently? What is the ...


16

Assignment Branch Condition size for fetch_images is too high. [22.41/15] What that warning is saying is that, according to rubocop's standard, you have too many assignments, branches, and conditionals (ABCs) in that method. ABCs are associated with code that is complex and harder to reason about. It can also indicate that a method is trying to do too much, ...


15

Here I assume you just want to see how your current code can be improved without changing the algoritm. First of all, give HLint tool a chance to suggest you obvious improvements. In your case the only improvement was that do in do readHeaderLine' "" was redundant, so not much. Second, in my opinion many small top-level definitions are better than few ...


15

I have a few comments that are unrelated to the synchronous/asynchronous and/or header-only nature of the code. parameters bools I don't like passing bools as parameters. I really dislike a function like your SimpleSocketStreamBuffer constructor that take multiple bools. You need to do a fair amount of looking to be sure how: foo x("www.google.com", ...


14

Code Review A standard naming convention (if there can be said to be one). Is that user defined types start with an initial capitol letter. Other identifiers that name objects (or functions) start with an initial lower case letter. This lets you find user defined types very quickly in the mess that is the C++ syntax. You seem to have all the variables (...


13

Create a strategy to find the mapped implementation of the interface provided: IDictionary<Type, Type> mappings; //Populated with all interface to implementation types private DalServiceBaseClient CreateHttpClient<T>(string baseAddress) { var httpClient = new HttpClient { BaseAddress = new Uri(baseAddress) }; var ...


12

I would not directly write to the console. You are best off building an ILogger interface, then create a ConsoleLogger class that is passed as an ILogger to the constructor. public interface ILogger { void LogMessage(string message, params Object[] args); } public class ConsoleLogger : ILogger { public void LogMessage(string message, params Object[...


11

Let me start off by saying how ridiculous the restrictions are: You are free to use any library/technique, except for java.net.Url, java.net.URI or java.net.UrlConnection. Solutions using these classes will not be accepted. The fact is that the apache libraries which you are using internally use java.net.Url and java.net.URI. There are a few things that ...


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

It seems that every one of your getCity(), getDepartment() etc functions loads the same web page over and over. You should load each URL once with your curl_get_contents(), then pass its result into each get*() function to parse it.


11

Readability Let's reformat your code quickly: HttpURLConnection connection = null; boolean connected = false; outer: for (int retry = 0; retry <= RETRIES && !connected; retry++) { if (retry > 0) { log.warning("retry " + retry + "/" + RETRIES); Thread.sleep(RETRY_DELAY_MS); } ...


10

Please think twice before using your own http server. The better way is using Jetty or embed Tomcat - good way to get rid of defects and to save time for developing business logic. If you're going to use multithreading please look into java.util.concurrent.atomic package in order to start using AtomicInteger etc. Your multithreading is not actually ...


10

You're trying to read from stdin until "\r\n" is encountered? You could just recursively call hGetChar until you find the right sequence (the buffer is maintained in reverse order and reversed at the end for efficiency reasons and to allow nice pattern-matching): readHeaderLine :: Handle -> IO String readHeaderLine h = reverse <$> go "" where ...


10

There are a few things I see that I would want to refactor if I were working on this The getContents() method throws exceptions, which are simply caught and re-thrown in the JGet constructor in a way that modifies the error message, but doesn't include the cause (the original exception). This can make troubleshooting the code more difficult because you're ...


10

A few items: Use STDIN_FILENO Instead of fileno(stdin) you can use the preprocessor symbol STDIN_FILENO, defined in <unistd.h> which you have already included. should_parse should be declared bool Assuming you're using a compiler that isn't decades old (bool was added in the c99 standard), you can include <stdbool.h> and use bool for the type ...


10

It can be tough, but pretend you're not the developer who wrote this code and try to understand what it does by reading through it. At a quick glance some questions you might ask could be: What does isValid actually check the validity of? What is strInput? The line isValid(strInput) just tells me that someone has deemed some input string valid for doing ...


10

It appears that you're using a class-level variable (strErrorMSG) to retain your error information. That is a style of programming that I highly discourage, where values are essentially "passed" between methods inside of a class using class-level variables instead of actual parameters. The reason that I discourage this style of programming is that I have ...


10

JavaDoc, JavaDoc, JavaDoc. // States: // 1 = downloading // 2 = completed // 0 = paused // -1 = failed AtomicInteger currentState = new AtomicInteger(1); This is the very reason why there exists Enum. What you're using here is a called a "magic integer" and is bad for many reasons, like readability, usability and a documentation nightmare. If you have ...


10

Linq to XML is your friend var serverName = XDocument.Load(response.GetResponseStream()).Element("Server").Value; Your code will be easily broken if someone adds a node to the XML, and your code should be as flexible as possible. Never do such an assumption (It's gonna be the first node) Your method is blocking, consider making it asynchronous. using (...


10

Your FileStream looks like it is an IDisposable as well, you should put it into a using statement as well, then you don't have to worry about missing a chance to dispose it. Right here if (ExistingLength > 0) saveFileStream = new FileStream(Path, FileMode.Append, FileAccess.Write, FileShare.ReadWrite); else saveFileStream = new ...


10

Are you sure that &(int) { 1 } does what you want it to do? I'm not sure it does. Using malloc()/strcpy() is probably better handled with strdup() (which you've used elsewhere, so you're already okay with that). In webserver_handle_connection, you're using malloc() to allocate space for an unsigned long. Why? Declare a normal local variable, and pass ...


10

If you are making many requests, it is worth it to have a look at requests.Session. This re-uses the connection to the server for subsequent requests (and also the headers and cookies). I would also start separating your different responsibilities. One might be global constants (like your header and cookies), one a function that actually does the submission,...


9

Style Use CamelCase only. last_status -> lastStatus, site_json -> siteJSON, etc. Even in constants. UP -> Up or StatusUp, etc. Imports are usually split into groups by source, so separate memcache from stdlib with a newline. Stuttering. E.g. configuration := Configuration{} could be c := Configuration{}. People would search for main() in main.go. So I would ...


9

Just a few quick remarks (for now): vbNullString is a remnant of VB6 (which I think would be under the Microsoft.VisualBasic.Constants namespace), and used to be a null string pointer often confused with "", an empty string. In VB.NET vbNullString is Nothing, so when you do this: If targetFolder = vbNullString Then You're actually verifying whether ...


8

Python has a mechanism for handling exceptional situations, but you go to great lengths to suppress it: you catch all the exceptions that might result from your request method and replace them with a string in the .error property of the APIClient object. This has several problems. First, it needlessly complicates every API call. A caller can't just write: ...


8

The way I would do it is by simply representing the headers as map of char* to char*. That'll be the most flexible and extensible situation, since HTTP headers are added over time, and custom headers are completely acceptable and allowed for by the standard. Any solution involving struct fields will require updating as the standard expands, and can't handle ...


8

I see a flurry of downloading activity, but I don't see that you do anything with the pages that you download except parse some URLs for more downloading. There's no rate limiting or any attempt to check robots.txt, making your web crawler a poor Internet citizen. PEP 8 mandates four spaces per level of indentation. Since whitespace is significant in ...


8

General As example code, there is a fair amount to comment on. For an eclipse plugin, I would at least expect you to select-all and Ctrl-Shift-F .... consistent use of braces (on the end of the line, not start of the new line) consistent spacing between values and operators jsonResponse+=line; to jsonResponse += line; The formatted code looks like: ...


8

There's something odd about your constructors - normally chained constructors call the constructor with the most parameters, not the other way around: public ProsperApi(string username, string password, string baseUrl) : this(username, password) public ProsperApi(string username, string password) Should be: public ProsperApi(string username, string ...


8

In total, a process in Unix systems can have a limited number of open filehandles. By default this is only say 1024 per process in Linux; all of the open sockets also consume file descriptors. Even if you optimize the system, then reading the file for each request will be costly. In this case, if you are really concerned about the performance, read the ...


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