Hot answers tagged

13

public Key(string unitID, int address, int comPort, int id) Clearly this class represents something much more specialized than an all-purpose "key". Name the type for what it stands for! private Tuple<string, int, int, int> _impl; The private field should be readonly. But why don't you have this instead? private readonly string _unitId; private ...


13

General Stuff Would be nice to have inline documentation (///) on classes and public members, but everything is pretty simple and understandable. Some might question the name _hashSet, which tells you nothing about it's purpose, but it's private so not a massive concern. You've correctly identified that you can use a ReaderWriterLock to control access, as ...


8

After studying your code for a good long while, I realized that the reason you haven't gotten an answer is that there isn't really much to say about your code. Well done! I would recommend adding some thorough ScalaDoc though. It has been my experience that Scala library developers have far too much faith in the ability of library users to magically ...


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

Poor cohesion It's called logical cohesion when a routine that does multiple things of different logic. Your function will either insert records, or select records, or update records. These are very different operations, and their implementations should be in separate functions. If you refactored this way, it would be much better: if ($Type == "INSERT") ...


7

static struct sigaction * handle_signal(int sig, int flags, void (*handler)()) { Two things here: The signal handler function should have the signature void handler(int signum); [man page for sigaction] . Your compiler should — at the least — be warning you about assigning a function pointer of a different type ...


7

I heard you like wrappers So I wrapped your wrapper (jQuery) in a wrapper (your thing) so you can do ajax. Why do you do this again?? Instead of wrapping over (slow) jQuery you could just wrap the native ajax functionality. In general I expect that to be faster. Additionally this frees your API to be changed independently of jQuery and it removes that ...


7

I'm the lead engineer for the MongoDB Perl driver with a couple of thoughts for you: you're using 'authenticate', which is for a very old version of the driver which is not recommended for use. In the v1.x series, you should provide username/password in the URI or in MongoClient parameters. Most of the value of this module seems to be setting parameters ...


6

This seems overcomplicated to me. Lazy-loading something doesn't require placement-new trickery and mucking around with alignments. Why not simply store a pointer to the stream which is only initialised when the user performs some kind of operation? This is the general way to do lazy-loading: struct error_stream { private: std::unique_ptr<std::...


6

Standard naming convention in C is usually lower_case_with_underscore. You should be using void * rather than char * I would consider making the return value deterministic when the new size is 0 by always returning NULL rather than the implementation defined behaviour (which could technically be a pointer which is not NULL but still not allowed to be ...


6

Why build your own implementation when you can use the features available in the Java standard libraries? IdentityHashMap gives you all the features you need, and a Java Stream/collector will allow you to extract the map easily from your collection.... Collection<Test> tests = ..... Map<Test,Test> uniques = tests.stream().collect(Collectors....


6

It is kind of hard to say without seeing the whole code but at a glance the whole thing seems kind of pointless to me. You're not making it object oriented but rather just adding your own "flavor" API over libusb. I can tell this by the fact that I see a DevHandlePtr deviceHandle being passed around just about everywhere. If you want to actually make a ...


5

With the variety of behaviors exhibited by different implementations, I think the only safe thing is to test for zero size before the realloc call and set a minimum allocation (eg 1 byte). Note that realloc will probably set errno to ENOMEM when it fails, but I don't think this is guaranteed.


5

I haven't been on the site in a while so figured I'd give this one a go: My first point is debatable and not essential but reflects my own preference. Consider making the class sealed by default. Whenever I say this to people, most seem surprised. You're probably asking why, I mean, does it matter? Your class is not designed with inheritance in mind, it ...


5

Naming Either use m_ or _ or no prefix for your class variables, but don't mix the styles. Also if you use one of the first two, there is no need to use this. Based on the naming guidelines method names should be named using PascalCase casing. Properties should be named using PascalCase casing too. Method parameters should be named using camelCase ...


5

Are you sure you want your database wrapper to be static? What if you want to mock it in your unit tests? I would make it a plain old class, pass the connection string through the constructor and do any kind of checks that I want right there, because that means your database wrapper won't be allowed to even exist without a valid connection string. With your ...


5

There are big problems with your approach. Cloning BigDecimal with .doubleValue() is broken Cloning a BigDecimal bd using new BigDecimal(bd.doubleValue()) doesn't work in most practical cases. For example, try this code: BigDecimal bd = BigDecimal.valueOf(0.02d); BigDecimal clone = new BigDecimal(bd.doubleValue()); System.out.println(bd); System.out....


5

Great that you provided a docstring! You made one minor formatting mistake though, there should be a blank line between your brief single line summary and the rest of the docstring. It's recommended in the style guide partially for readability and partially for script parsers. class MongoDB(object): """Provides a RAII wrapper for PyMongo db connections. ...


5

My first question looking at this is, as a developer, what would be the advantages for me to use your window class over just dealing directly with Windows? It seems like I'd still have to do most of the same work as if I didn't use the class. The way one would use this class appears to be: Create an instance of the class Optionally call registerClass (but ...


5

Construction In your constructor from iterators: template <class It> Simple_Array(It first, It last) : Simple_Array(std::distance(first, last)) { unchecked_copy(first, last); } You do two things: first you default-construct a n objects, and then you copy-assign them. This is both inefficient and reduces the usability of your class. What if ...


5

l is a terrible name for anything. Depending on the font used, it may be indistinguishable from the number one. It's generally recommended to avoid using the letter "l" for anything. (In some languages this recommendation is part of the official style guide.) log would be better, still short, and with a more obvious meaning than just "l".


5

As I commented on the question, this has no difference from running console.log without your function. As saying on the documentation of Function.prototype.apply(): The apply() method calls a function with a given this value and arguments provided as an array (or an array-like object). The arguments object is an "array-like object". What is an array-...


5

I would personally prefer to use HttpClient directly as it provides full control and perfect testability - see here. We can define our REST API as: // see https://jsonplaceholder.typicode.com/ // this site exposes some REST API example to automate public interface ITypicode { Task<BlogPost[]> GetPostsAsync(); Task<BlogPost> GetPostAsync(...


5

Design Issue Creating a new thread for every connection is not a good idea. Creating a thread is expensive. Also a single thread can easily handle thousands of connections, so utilizing a single thread for a single connection is very wasteful. Also the way you are using the threads is all wrong. This should never happen. busy = true; task->...


4

a condition to be met like if (someBoolean == true) can be simplified to if (someBoolean). For reverse check use the Not operator ! like if (!someBoolean). Dipsose() should not only call Close() but also get rid of session, sessionOptions and transferOptions by calling Dispose() if they implement IDisposible or at least setting them to null. SendFile() ...


4

I'm not too familiar with Android but I've found an issue with the code: both persist and retrieve methods contains the similar if-else chain. You should replace them with polymorphism: Refactoring: Improving the Design of Existing Code by Martin Fowler: Replacing the Conditional Logic on Price Code with Polymorphism Replace Conditional with Polymorphism ...


4

Two minor notes: The following is not too easy to read: curl_setopt($r, CURLOPT_ENCODING, 1); Readers have to be really familar with curl parameters or have to check the documentation. It says the following: The contents of the "Accept-Encoding: " header. This enables decoding of the response. Supported encodings are "identity", "deflate", and "gzip"....


4

First off: The only response i can get from the SOAP client except from data in case of success is exceptions. Of course, if your call wasn't successful, there's something wrong, and an exception should be thrown. If you query for an ID that doesn't exist, don't catch the exception. That means the user has provided your client with invalid data, and its ...


4

I have been using php for over 10 years and never found a need for a session wrapper, but maybe you find it easier/better. There are a few improvements that could be made with your code If we are checking to see if the session has a key, a simpler test is public static function has($Key) { // return (bool)(isset($_SESSION[$Key])) ? $_SESSION[$Key] : ...


4

First of all, why are you using C++/CLI? The C API looks relatively simple, you should be able to write the wrapper in C# using PInvoke, with the advantage that you won't have to write all that copying code. int Left; Public fields are considered bad practice in .Net. Instead, you can use shorthand property syntax: property int Left; Frame() { Top = ...


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