We’re rewarding the question askers & reputations are being recalculated! Read more.
64

The code leaves a bad first impression because of how it is formatted. In particular, the extra whitespace between the return type and the function name, and that between the variable's type and its name, looks odd. Unless you are writing code that needs to be compatible with pre-C99 compilers, you should break the habit of declaring variables at the top of ...


53

Don't do this yourself. Instead use java.net.URLEncoder or another library implementation of url encoding. You'll also get support for other characters as well besides space.


52

Don't include unnecessary headers We're not using <vector> or <stdio.h>, so drop them. We only need <ostream> rather than <iostream> for the class, and we prefer <cstring> to <string.h> to place the standard functions in their proper namespace. Avoid importing all of namespace std It's a bad habit that will cause ...


31

So the idea behind this is mainly educational... Cool. Always good to try and understand how things work under the covers. ...but I might even consider using it in reality if turns out to be good. Please rethink this. Smart pointer implementations are incredibly difficult to get right. Scott Myers, of Effective C++ fame, famously tried to implement a ...


29

Issues Your vector assumes that the type being stored has a default constructor. arr = new element[size]; Also this is really inefficient for expensive to create classes where you don't use all the size members. You want to design your class so that the objects in the vector are only constructed when the element is first added. Bug You don't implement ...


28

Have you tried myString.replaceAll(" ", "%20")? This method is available since Java 1.4. BTW, a StringBuilder is recommended over a StringBuffer if you do not require the synchronization offered by the latter. This class is available since Java 1.5. edit: myString.replace(" ", "%20") also exists, this should be preferred for literal replacements and is ...


27

So my first hunch when I read your question is: Okay this guy is making a complicated solution to a simple problem based on a hunch that the trivial solution is "grossly inefficient" without any measurements to back up said claim. I suspect OPs solution is slower than the trivial solution. So I set out test this, I pit your fancy implementation against ...


21

A simple improvement to improve performance by reducing the number of reallocs is to start with a decently sized buffer and when it fills up, grow it by a multiple of the size. For example, you could double the size when it fills up. Of course this approach is less efficient memory wise, but in order to mitigate that, we can multiply by a smaller multiple (...


19

Why not: static string Read(istream &stream, uint32_t count) { std::string result(count, ' '); stream.read(&result[0], count); return result; } Though not strictly C++03 compatible that is easily validated. One of the reasons the committee found it easy to add the new constraint in C++11 was that no implementation did not use ...


18

Not a bad first crack at a linked list. Here's some things that are obvious issues to me: Your getters and setters for list_node_generic aren't really encapsulating much. They allow unchecked access to private fields. Essentially rendering those fields public, but with much verbosity. Since you don't intend to let any code other than the enclosing ...


17

Silent bug: Bad move constructor Your move constructor will perform a copy because your parameter is aligned_type const&&; you cannot move from a const value because moving implies modifying what you're moving from. Simply change the signature to: aligned(aligned_type &&other); // ^^^^^^^^^^^^^^^ no more const Bug: Bad dereference ...


17

p = malloc(n*sizeof *p); This is dangerous if n gets large, because the multiplication could overflow. After the overflow, too little memory has been allocated but code will continue without detecting the error. This is especially dangerous if n comes from untrusted source, such as some file format or remote user. Then it gives attacker easy way to ...


16

Things you did well on: You make good use of comments. You try to make the user experience as smooth as possible, printing out a lot of useful information. Things you could improve: A few notes that others haven't covered: Running your program through Valgrind, I didn't see any memory leaks besides where your if conditions fail and you exit main(). ...


16

1. Performance This memory management implementation maintains a single doubly-linked list of memory blocks. The main causes of the performance problems are as follows: When malloc is called, the global pointer to the list is needlessly updated (see §2.18 below) and then the whole list is traversed in order to find the start of the list again. Allocated ...


16

This is a design issue, and there's no single answer. We need to consider what's important about a Weapon. Is it value-like, or is its identity important? The real-world analogy is, "are copies all equivalent?". The answer to this lies partly in the type, but also in how we intend to use it. In the real world, Excalibur has an identity. You can't copy ...


14

Things you did well: Some use of comments. Use of typedef. Things you could improve: Syntax/Styling: Never use goto. Yes, there are some rare situations where you may find it necessary to use it. This is not one of them. err: printf("could not allocate mem\n"); return 0; As you can see, all you do in your little block of code here ...


14

Generally, malloc, realloc and free are all part of the same library. One of the things this allows is some 'behind the scenes' meta-data chicanery. For example if you wanted to call malloc(16), the memory library might allocate 20 bytes of space, with the first 4 bytes containing the length of the allocation and then returning a pointer to 4 bytes past ...


14

@Toby Speight describes how it should probably be done. But I want to describe what is happening in your case so that you understand the concept of copy construction and assignment. The problem is that there are a couple of methods that are automatically generated by the compiler that you did not account for. If you don't specify a copy constructor and ...


14

First of all, lets answer the questions you have: Yes, no, maybe. Some people do it one way, some the other. Personally, I prefer splitting declarations and definitions up so that I have an overview of what my class offers at the top. Some people argue, however, that this method tends to be very verbose (and it can be, especially with multi-layer templates),...


14

Names beginning with _ are reserved for the implementation; I'm surprised that -Wpedantic doesn't help with this! Include what you use - it helps if your implementation and test program include malloc.h before any standard headers. In this case, it's depending on <stddef.h> (or one of the other headers which define size_t). With -Wconversion, I get ...


13

Learn from existing implementations, they usually have solutions to corner cases, common pitfalls and performance bottlenecks. For example, Apache Commons Lang StringUtils also has a remove function. It's implementation is quite simple and similar to yours: public static String remove(final String str, final char remove) { if (isEmpty(str) || str....


13

Tile.h Answering the question in the comment, yes you always need an include guard for a file if you plan on including it in more than one .cpp. Note: #pragma once is also viable. BTW, Tile.h is so small, maybe remove it an place the Tile enum inside Level.h. C++ 11 enums: C++ 11 introduced typed enums, very similar to the ones in C#, so you will ...


13

Dynamic Memory Management Am I doing it right in terms of memory menagment? You should rather use smart pointers to store the Shape instances in the vector: std::vector<std::unique_ptr<Shape>> shapes; and std::unique_ptr<Shape> generateRandomShape() { int Shape; // Take care! Some compilers don't like it if variables ...


12

...it [the program] is to be used in embedded devices with possibly very low available RAM In that case, we should be getting rid of everything that isn't of absolute necessity, and adding some things that are. Overall: If one provides no memory modifier (such as __flash) then many embedded systems compilers will copy the data into RAM (even sometimes ...


12

You need to use the two Streams as, well, streams: read a manageable part of the input, transform it, write it to the output and repeat. int bufferSize = 4096; // or whatever char[] characters = new char[bufferSize]; using (var reader = new StreamReader(inFile)) using (var writer = new StreamWriter(inFile + ".NonAsciiChars")) { while (true) { ...


12

First, a word on naming: The name you've chosen for your type, _arraylist is a bad name for a library interface type. Names starting with _ are not pleasant to work with in user code. They are commonly used inside library internals. Better names would be ArrayList or array_list. Actually, in your usage example, you have ArrayList. Does this mean that in ...


12

General Comments Don't like the naming of your class _stack_node or stack. The first one has a leading underscore. This is usually a bad sign. Do you know all the rules for leading underscore? More importantly, does everybody that reads your code understand all the rules? Also it is traditional to use a leading uppercase letter for user defined types. ...


12

The most unforgivable fault with this function is that calling isValidBuffer() also erases the buffer! That completely violates the Principle of Least Surprise. If anything, the parameter should be const.


12

In the 99.9% of cases, your architecture will use IEEE floats/doubles, and using calloc() is ideal for both simplicity and speed. calloc() is far more efficent when allocating large zeroed memory since the operating system will often simply allocate a blank zero page of memory from the page table and not need to actually set each word's value. So you can ...


12

You are checking if malloc succeeded before calling memcpy, but not before calling memset. You are copying len bytes from ptr to the new memory location, but don't know how many bytes ptr actually points to. It probably points to less, this being a major reason why one would want to call realloc in the first place.


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