Hot answers tagged

14

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


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


10

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


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


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


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

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


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


6

One thing I don't see you address, is that INotifyPropertyChanged.PropertyChanged has the signature of: public delegate void PropertyChangedEventHandler(object sender, PropertyChangedEventArgs e); where object sender should be the owner of the property (typically this). You don't provide the code for ObservableObject, but from your code above it seems that ...


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

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


5

This is not much a wrapper. Rather call it a wannabe Query Builder. I don't know the reason, but many people are constantly trying to write something like this. And I don't understand why. Okay, for the DML queries it makes sense - it always makes a cool feeling when you automate a routine task using a template. So, for the insert query it asks for the ...


5

Some specific questions I have about this code/approach are, if applicable, OK. Error handling design decision: should the constructors throw, or should I leave error checking, if desired, to the caller? Either the object is correctly initialized and ready to use or it should throw. Two stage initialization (construct then check) is a bad idea as it ...


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


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