Changing a pixel based on a keypress I have this code that is working as intended I would just like to speed it up.

#include <windows.h>

void pressW();

int main(int argc, char** argv) {

    DWORD x = 566;
    DWORD y = 967;
    DWORD red = 2039797;
    DWORD gold = 43963;
    DWORD blue = 15715270;

    while (true) {
        if (GetAsyncKeyState(0x31)) {
            pressW();
            while (true) {
                HDC dc = GetDC(NULL);
                COLORREF color = GetPixel(dc, x, y);
                ReleaseDC(NULL, dc);
                if (color == blue) {
                    pressW();
                    break;
                }
            }
        }
        else if (GetAsyncKeyState(0x32)) {
            pressW();
            while (true) {
                HDC dc = GetDC(NULL);
                COLORREF color = GetPixel(dc, x, y);
                ReleaseDC(NULL, dc);
                if (color == red) {
                    pressW();
                    break;
                }
            }
        }
        else if (GetAsyncKeyState(0x33)) {
            pressW();
            while (true) {
                HDC dc = GetDC(NULL);
                COLORREF color = GetPixel(dc, x, y);
                ReleaseDC(NULL, dc);
                if (color == gold) {
                    pressW();
                    break;
                }
            }
        }
    }
}

void pressW() {
    INPUT i;
    i.type = INPUT_KEYBOARD;
    i.ki.wScan = MapVirtualKey(0x57, MAPVK_VK_TO_VSC);
    i.ki.time = 0;
    i.ki.dwExtraInfo = 0;
    i.ki.wVk = 0x57;
    i.ki.dwFlags = 0;
    SendInput(1, &i, sizeof(INPUT));
    i.ki.dwFlags = KEYEVENTF_KEYUP;
    SendInput(1, &i, sizeof(INPUT));
}

Basically what it does is when I press 1/2/3 it presses W and waits for the the pixel at x, y coordinates to change color corresponding to what key I pressed and presses W again.

I want it to have 10-15ms delay from the point when I press 1/2/3 to when it scans the pixel and presses the 2nd W, but I don't really know how to optimize this. Right now it's taking a snapshot of the entire screen but I just want to check that single pixel, also it's declaring everything for the sendinput every time I call that function because simply putting

SendInput(1, &i, sizeof(INPUT));
i.ki.dwFlags = KEYEVENTF_KEYUP;
SendInput(1, &i, sizeof(INPUT));

in the pressW() didn't work.

The Fundamentals: Style & Readability

Before we get into optimizing, there are a couple of glaring problems with your coding style—that is, the way the code is written and the way it looks.

The biggest problem is that there is a lot of duplication. You repeat yourself over and over inside the if blocks of the main function. Any time you see repeated code, you should extract it out into a function. Even if there are some subtle differences between the various blocks of code, this can be accommodated by passing parameters to the function to customize its behavior. You want to be DRY—don't repeat yourself. It just makes the code harder to read, harder to maintain, and increases the chances for bugs to slip in.
Variables that never change their values are called constants, and the C++ language has a special way to indicate such variables: the const keyword. Using this helps to prevent bugs, like accidentally writing code that changes a variable's value, when that variable is supposed to be constant.

Thus, the x, y, red, gold, and blue variables declared at the top of the main function should all be const, since these are effectively read-only. Also, I would personally recommend making even local variables whose values do not change after initial initialization (such as hdc and color) explicitly const.

(Also, I'm a little bit obsessive-compulsive, so I like to vertically align all of my assignments. Others may disagree that this is worth the effort, but I use tools in my editor that semi-automate it. I find that it not only makes the code look better overall, but makes inconsistencies—i.e., possible errors—more likely to be caught because they stick out like a sore thumb.)
Speaking of the red, gold, and blue variables, these appear to be magic numbers, the values of which would defy most people's intuition. Why, exactly, would 2039797 be a representation of red?

Instead, you should be declaring these color values using the Win32 RGB macro. This takes individual red, green, and blue color components (a representation with which we are all familiar), and packs them into a 32-bit DWORD. Not only is this more readable, but it also ensures that your code is endianness-independent. The Platform SDK determines whether the encoding is RGB or BGR (in the case of Win32, it is actually BGR).

Furthermore, you should prefer to use the COLORREF type for color values in Windows. While COLORREF is simply a typedef for DWORD, it is clearer and more semantically accurate to use COLORREF. Besides, you used the COLORREF type to store the return value of GetPixel, so you should do it here for the comparison constants in the interest of consistency.
In C++, you should be taking advantage of the RAII paradigm to produce cleaner, more correct code. What I mean by this is that any time you have an unmanaged object (such as a Windows handle), you should wrap that object in a class. Make the class's constructor responsible for allocating the object, and the class's destructor responsible for deallocating/freeing the object. When an object of that class is declared, its unmanaged resources are automatically initialized; when that object goes out of scope, its unmanaged resources are automatically released. This way, the compiler does all of the book-keeping automatically, and you never leak.

Your code as written is actually correct, in that you call ReleaseDC for each call to GetDC, but I bet you had to think about this to make sure that you got it right. Best not to make yourself think so hard. Also, using RAII ensures that your code will be exception safe. (Again, not a problem in this case, since the only code in between GetDC and ReleaseDC is a call to GetPixel and an assignment to a POD type, neither of which can throw, but it's all too easy to add something later and forget to audit carefully for exception safety.)

Most people use a framework like MFC or WTL that provides the wrapper classes for you, but if you're just writing a simple app, you don't necessarily need anything that complicated. It only takes a few lines of code to write a simple wrapper class that does everything you need:

class DC
{
  public:

    explicit DC(HWND hWnd)
        : m_hWnd(hWnd)
        , m_hDC (GetDC(hWnd))
    { }

    ~DC()
    {
        ReleaseDC(m_hWnd, m_hDC);
    }

    HDC Handle()
    {
        return m_hDC;
    }

  private:

    HWND m_hWnd;
    HDC  m_hDC;
};

You could improve this substantially by adding error checking—at the very least, you should probably have an assert that checks for invalid handles and/or failed function calls in debugging builds.
Instead of using the sizeof operator on the type, prefer to use the sizeof operator on the object. For example, change sizeof(INPUT) to sizeof(i). That way, if i ever changes to, say INPUTEX, there will be fewer places in the code that you have to remember to change.
Don't repeat constant literals throughout the code. To be fair, you've only done it one place—inside the pressW function by hardcoding the value 0x57. The damage is relatively contained, but still, 0x57 is repeated twice and this makes changing it in the future an error-prone task. Declare a constant instead.

Better yet, replace the inscrutable 0x57 with 'W'. In other words, let the compiler do the evaluation and determine what the ASCII code of the W character is. The result is the same, but the code is significantly more readable. (Also do the same thing for 0x31, 0x32, and 0x33.)

In most cases, the pressW function should probably be pressKey, and take the virtual-key code as a parameter. Personally, I would prefer writing it this way from the get-go because it's no harder. But you do have to be careful not to make the code you're writing overly generic, because that wastes development time for things you may not ever need (see also YAGNI).

One thing you are doing well, though, is following a consistent style throughout. While K&R is not my preferred brace style, you are using it consistently, and that is really what's most important.

Taking these points into account, here is how I would revise your current implementation:

#include <windows.h>

void pressW();
void waitForColorChange(DWORD x, DWORD y, COLORREF expectedColor);

class DC
{
  public:

    explicit DC(HWND hWnd)
        : m_hWnd(hWnd)
        , m_hDC (GetDC(hWnd))
    { }

    ~DC() {
        ReleaseDC(m_hWnd, m_hDC);
    }

    HDC Handle() {
        return m_hDC;
    }

  private:

    HWND m_hWnd;
    HDC  m_hDC;
};

int main(int argc, char** argv) {
    const DWORD    x    = 566;
    const DWORD    y    = 967;
    const COLORREF red  = RGB(245,  31,  31);
    const COLORREF gold = RGB(187, 171,   0);
    const COLORREF blue = RGB(198, 203, 239);

    while (true) {
        if (GetAsyncKeyState('1')) {
            waitForColorChange(x, y, blue);
        }
        else if (GetAsyncKeyState('2')) {
            waitForColorChange(x, y, red);
        }
        else if (GetAsyncKeyState('3')) {
            waitForColorChange(x, y, gold);
        }
    }
}

void pressW() {
    const WORD vkW = 'W';
    INPUT i;
    i.type           = INPUT_KEYBOARD;
    i.ki.wScan       = MapVirtualKey(vkW, MAPVK_VK_TO_VSC);
    i.ki.time        = 0;
    i.ki.dwExtraInfo = 0;
    i.ki.wVk         = vkW;
    i.ki.dwFlags     = 0;
    SendInput(1, &i, sizeof(i));
    i.ki.dwFlags     = KEYEVENTF_KEYUP;
    SendInput(1, &i, sizeof(i));
}

void waitForColorChange(DWORD x, DWORD y, COLORREF expectedColor) {
    pressW();
    while (true) {
        DC dc(NULL);
        const COLORREF color = GetPixel(dc.Handle(), x, y);
        if (color == expectedColor) {
            pressW();
            break;
        }
    }
}

There are, of course, other equally valid ways of refactoring your code, but the important thing is to be DRY! The above example achieves that, and without significantly changing the object code generated by a compiler (in other words, without slowing anything down at run-time).

Going Deeper: Cautions & Potential Optimizations

The GetAsyncKeyState function actually is asynchronous, meaning that if you call it twice in a row, you will get two different results if the keyboard state changes between those calls. If you're not very careful, this can result in glitchy behavior. You are probably okay here, but this is something to watch out for.

The call also isn't free. Spinning in a busy loop, repeatedly calling GetAsyncKeyState is a good way to burn a bunch of CPU cycles and turn your computer into a space heater. A better design would be to listen for notifications of a key press event, rather than polling for a state change.
Since you're not passing either the KEYEVENTF_SCANCODE or KEYEVENTF_UNICODE flags, you can avoid computing the scan code and initializing that field each time you call SendInput. This won't save you a whole lot of time, but it does elide one function call.

Even if you needed to initialize the scan-code field, you could save yourself some time by calling the MapVirtualKey function once and caching its result in a static (or global) variable. The mapping of virtual-key codes to scan codes is not going to change while your application is running, so there's no need to call the function each time.
You can save another function call by collapsing the two calls to SendInput into one. This function allows you to pass an array of INPUT structures, and it will synthesize a separate event for each one. The key-down event goes in the first INPUT structure, and the key-up event goes in the second INPUT structure. The required modifications to the code are trivial, but just in case:

void pressW() {
    const WORD vkW = 'W';

    INPUT i[2];
    i[1].type           = INPUT_KEYBOARD;
    i[1].ki.wVk         = vkW;
    i[1].ki.time        = 0;
    i[1].ki.dwExtraInfo = 0;
    i[1].ki.dwFlags     = 0;

    i[2].type           = INPUT_KEYBOARD;
    i[2].ki.wVk         = vkW;
    i[2].ki.time        = 0;
    i[2].ki.dwExtraInfo = 0;
    i[2].ki.dwFlags     = KEYEVENTF_KEYUP;

    SendInput(ARRAYSIZE(i), i, sizeof(i));
}

Notice that I've used the ARRAYSIZE macro (defined in the Win32 SDK headers) to automatically determine the size of the array so I don't have to hard-code another constant. This looks like more code, but it actually executes more quickly because the cost of initializing that extra INPUT structure is dwarfed by the cost of a function call.

If you really wanted to go all out, you could recognize that the values of these INPUT structures never change and decide to make it statically-initialized. This saves the compiler from having to emit code that performs the initialization before each call to SendInput. It simply pushes a pointer to constant data in the executable's DATA segment and calls SendInput. Micro-optimization at its finest! A different syntax will be required for this static initialization:

void pressW() {
    const WORD vkW = 'W';
    static INPUT i[2] = {
       { INPUT_KEYBOARD, { vkW, 0, 0,               0 } },
       { INPUT_KEYBOARD, { vkW, 0, KEYEVENTF_KEYUP, 0 } },
    };
    SendInput(ARRAYSIZE(i), i, sizeof(i));
}

(I really wanted to make i const there, but I can't safely do so because the SendInput function expects a non-const pointer to the array of INPUT structures. I could reasonably assume that the function is not going to modify them, but I cannot prove that without inside knowledge of its implementation, so I cannot rely on it.)
Acquiring and releasing a device context (GetDC/ReleaseDC) are not particularly expensive operations, but they aren't exactly free, either. Therefore, it makes good sense to minimize the number of times you do this cycle. It certainly isn't necessary to do it on each iteration of a tight spin loop! The waitForColorChange function that I wrote could do it at the top, then enter the while loop, and only release the DC after it breaks. Using RAII, this is simple:

void waitForColorChange(DWORD x, DWORD y, COLORREF expectedColor) {
    pressW();
    DC dc(NULL);
    while (true) {
        if (GetPixel(dc.Handle(), x, y) == expectedColor) {
            pressW();
            break;
        }
    }
}

But actually, this waitForColorChange function is itself called from within a tight spin loop! In fact, your code could be revised such that the DC is acquired at the beginning of the program, used throughout its lifetime, and released when your program exits (which will happen automatically, thanks to RAII). This will require a bit of refactoring—a const reference to the DC object will need to be passed to the waitForColorChange function—but doesn't make the code any less readable.

The only concern is that, generally speaking, device contexts shouldn't be held onto any longer than necessary. It seems justified in this case because you are actively using it the entire time, but that may just speak to a larger design flaw with your application. I've already mentioned my reservations about polling.

The Main Event: Optimizing GetPixel

At the core of your code's logic is a call to GetPixel to retrieve the current color value of a particular pixel on the screen. That is unfortunate if you're concerned about speed, because GetPixel is a very slow function, indeed. Why is it so slow? The above-linked documentation doesn't give much of a hint. In fact, this API has a dirty little secret: individual pixels cannot be directly manipulated by the graphics subsystem, so GetPixel (and its brother, SetPixel) have to simulate it, which makes an ostensibly simple, straightforward function call into an extremely slow operation. In addition to the usual overhead of a function call, parameter validation, and switching to and from kernel mode, the GetPixel function also has to create a temporary bitmap, copy the contents of the DC into that temporary bitmap, perform all necessary color-mapping, map the coordinates and locate the pixel, retrieve its color value, and then destroy the temporary bitmap. A lot of work for a single pixel!

The throughput of GetPixel is on the order of 100,000 pixels per second, which is usually fast enough. If you want to speed up drawing operations that involve the manipulation of multiple pixels, the general strategy is to minimize overhead by copying the DC's contents into a temporary device-independent bitmap (DIB) and then using pointer arithmetic on this DIB to access individual pixels. This gives you direct access to each pixel, and allows a throughput of millions or more pixels per second. The problem for you is that you cannot simply use a cached snapshot of the screen—you really do need to update it each time through the loop, which means that this strategy doesn't save you a whole lot in terms of overhead. But it'll still be slightly faster.

Here's an approximation of the code that would be required:

COLORREF FastGetPixel(DWORD x, DWORD y)
{
   // Retrieve a device context for the screen.
   const HDC hdcScreen = GetDC(NULL);

   // Create a DIB section.
   BITMAPINFO bi;
   bi.bmiHeader.biSize         = sizeof(bi.bmiHeader);
   bi.bmiHeader.biWidth        = 1;
   bi.bmiHeader.biHeight       = 1;
   bi.bmiHeader.biPlanes       = 1;
   bi.bmiHeader.biBitCount     = 32;
   bi.bmiHeader.biCompression  = BI_RGB;
   bi.bmiHeader.biSizeImage    = ((((static_cast<unsigned int>(bi.bmiHeader.biWidth) * 32U + 31U) / 32U) * 4U) * bi.bmiHeader.biHeight);
   bi.bmiHeader.biClrUsed      = 0;
   bi.bmiHeader.biClrImportant = 0;
   BYTE* pBits;   // points to the DIB section's raw bit data
   const HBITMAP hbmpDIB = CreateDIBSection(hdcScreen,
                                            &bi,
                                            DIB_RGB_COLORS,
                                            reinterpret_cast<void**>(&pBits),
                                            NULL,
                                            NULL);

   // Create a memory device context that is compatible with the screen.
   // We need to select our DIB section into this DC so that we can draw onto it.
   // Specifically, we'll copy a 1x1 pixel area from the screen into our DIB section.
   const HDC     hdcMem  = CreateCompatibleDC(hdcScreen);
   const HBITMAP hbmpOld = static_cast<HBITMAP>(SelectObject(hdcMem, hbmpDIB));
   BitBlt(hdcMem,    0, 0, bi.bmiHeader.biWidth, bi.bmiHeader.biHeight,
          hdcScreen, x, y,
          SRCCOPY);
   SelectObject(hdcMem, hbmpOld);
   GdiFlush();

   // Read the RGB color value of the top-left pixel in our DIB section
   // (which is actually the only pixel that it contains).
   // Note that in a bitmap, the byte order is always RGBA!
   const COLORREF clr = RGB(pBits[2], pBits[1], pBits[0]);

   // Clean up.
   DeleteObject(hbmpDIB);
   DeleteDC(hdcMem);
   ReleaseDC(NULL, hdcScreen);

   return clr;
}

The code can be improved by writing simple C++ wrapper classes to implement automatic resource management, as was discussed above. You should also add error-checking.

To really squeeze speed out of it, you'll need to resort to some ugly tactics. As written, this does much the same thing as the Win32 GetPixel function, and like I mentioned above, your setup is such that you need to call this function repeatedly. The obvious way to make code execute faster is to reduce the amount of code that you have to execute. Here, that means factoring out common tasks and doing them only once:

You don't need to retrieve a screen DC and release it each time through the FastGetPixel function. You can retrieve one once at the start of your program, and free it at the end once you're done.
The values of the BITMAPINFO structure are not changing, so you could make it static const and initialize it only once, instead of each time. This is basically the same as what we did with the INPUT structure.
You don't need to create a new DIB section and free it each time. You can just reuse the same one. Create it once at the start of your program, use it as needed, and free it at the end. (This actually makes the above bullet unnecessary, since the initialization of BITMAPINFO will no longer be on the critical path.)
As with the screen DC, you don't need to create a memory DC and free it each time. Keep the same memory DC, with your DIB section selected into it, and re-use it each time. This saves not only repeated calls to CreateCompatibleDC and DeleteDC, but also repeated calls to SelectObject.

Although making all of these objects global is pretty much breaking every design guideline in the book, sometimes performance concerns justify ugliness. Address maintenance concerns by wrapping them in C++ classes, following a "singleton" pattern.

Now, your inner loop will simply call the following function:

// Required global objects
// (shown here as simple global variables, but should really be managed by
// singleton classes to ensure correct lifetime management)
HDC   hdcScreen;
HDC   hdcDIB;
BYTE* pDIBBits;

COLORREF FastIterativeGetPixel(DWORD x, DWORD y)
{
   // Validate global objects' state in debug builds using assertions
   // so you can easily diagnose problems but don't lose performance
   // in release builds.
   // ...

   // Copy the current contents of the screen into our DIB section.
   BitBlt(hdcDIB, 0, 0, 1, 1, hdcScreen, x, y, SRCCOPY);
   GdiFlush();

   // Retrieve the RGB color value of the top-left pixel in our DIB section
   // (which is actually the only pixel that it contains).
   // Note that in a bitmap, the byte order is always RGBA!
   return RGB(pDIBBits[2], pDIBBits[1], pDIBBits[0]);
}

All initialization and clean-up has been deferred, so that all we need to do is a single BitBlt and, to be on the safe side, a GdiFlush. If you were willing to live a bit dangerously, you could omit the flush. (In fact, you're probably safe omitting it in your case, because all that would happen is you'd get stale color values that didn't accurately reflect the current state of the screen, which would just defer your action until the next time through the loop.)

Applying all of the optimizations mentioned in the last two sections, you should see a significant performance gain—an order of magnitude, if you're lucky. But then you will have hit a wall. Real performance gains will only come—as they always do—with reimagining the fundamental algorithm and/or design of your application. Like finding a way to receive notification from the other application when it has changed the color.


Implementing binary output to a file in C++11 I would like to have a class which supports outputting bits into a file. This is what I came up with but I fear there are many poor decisions. Should I inherit from a std::ofstream instead of keeping it as a data member?

class OFBitStream final
{
    static const std::size_t bitset_size = 8 * sizeof(unsigned long);
    std::bitset<bitset_size> bitset;
    std::size_t current_bit;

    std::ofstream &_stream;

    void flush() {
        for (auto i = current_bit; i < bitset_size; ++i) {
            bitset[i] = false;
        }

        auto buffer = bitset.to_ulong();
        while (buffer != 0) {
            _stream.write(reinterpret_cast<char *>(&buffer), 1);
            buffer >>= 8;
        }
    }
 public:
    OFBitStream(std::ofstream &stream)
        : bitset(0), current_bit(0), _stream(stream) {}

    ~OFBitStream() {
        flush();
        _stream.close();
    }

    friend OFBitStream &operator<<(OFBitStream &dst, const bool &data)
    {
        dst.bitset[dst.current_bit++] = data;

        if (dst.current_bit == dst.bitset_size) {
            dst.flush();
            dst.current_bit = 0;
        }

        return dst;
    }
};

I would not write an adapter for a stream.
But rather a stream manipulator.

Try this:

#include <ostream>

// The object that does all the work.
class BBoolWrapper
{
    std::ostream&          str;
    mutable std::size_t    position;
    mutable std::bitset<sizeof(unsigned long) * CHAR_BIT> data;

    public:
        BBoolWrapper(std::ostream& str)
            : str(str)
            , position(0)
        {}
        ~BBoolWrapper()
        {
            // If there is data to be flushed when we destroy the
            // object then make sure it goes on the stream.
            flush();
        }

        // Testing state.
        bool empty() const      { return position == 0;}
        bool full() const       { return position == data.size();}

        // Just the way I do it.
        // You can add bits any way you like one at a time.
        void addBit(bool nextbit) const
        {
            data <<= 1;
            data[0] = nextbit;
            ++position;
            if (full())
            {
                flush();
            }
        }

        // Simple Flush to the stream (if there is any data).
        void flush() const
        {
            if (!empty())
            {
                unsigned long value = data.to_ulong();
                str.write(reinterpret_cast<char*>(&value), sizeof(unsigned long));
                position  = 0;
            }
        }

    // The interesting part.
    // If we have an object like this and a bool using the `<<` operator
    // Then add the bit and return this object so we are chaining the
    // `<<` operations.
    friend BBoolWrapper const& operator<<(BBoolWrapper const& stream, bool nextbit)
    {
        stream.addBit(nextbit);
        return stream;
    }

    // If we use any other type with `<<` operator then flush this object.
    // Return the underlying stream object to get back to normal stream
    // operations.
    template<typename T>
    friend std::ostream& operator<<(BBoolWrapper const& stream, T const& val)
    {
        stream.flush();
        return stream.str << val;
    }
};

// A simple empty class to start the streaming of bits
// via the above wrapper class.
struct BBool
{
    friend BBoolWrapper operator<<(std::ostream& stream, BBool const&)
    {
        return BBoolWrapper(stream);
    }
};

Example:

#include <iostream>
#include <ifstream>

int main()
{

    // Use the BBool() object on a stream to turn binary mode on.
    // It stays on until you add a non boolean object to the chain. (like "\n")
    std::ofstream log("log");
    log       << BBool() << false << true << false << false << false << false << false << true << "\n";
    std::cout << BBool() << false << true << false << false << false << false << false << true << "\n";
}


Get better conclusions from a javascript randomizer I have a simple javascript randomizer that compares two random numbers with values 1 to 6.

I want to get a good estimate about the probability of the first been greater than the second. I could do it using math, probability theory e.t.c. but lets say that I want to stick to javascript. Using millions or billions of itterations gives better estimates but I can't push that too far (especially since codepen, jsFiddle e.t.c. don't allow that many). Am I missing something? Is there a better way to estimate this probability? Right now, I just run the program (with 1000 itterations) a couple of times and calculate the average value but that gives values in quite a wide range, I'd like to get more restricted results.

var xIsGreater = 0;
var yIsGreater = 0;
for(j=0;j<1000;j++){
  var x = Math.floor(Math.random() * 6) + 1  ;
  var y = Math.floor(Math.random() * 6) + 1  ;
  if(x>y){
    xIsGreater++;
  }else if(y>x){
    yIsGreater++;
  }
}
var draw = 1000-xIsGreater-yIsGreater;
console.log(xIsGreater+'--'+yIsGreater+'--'+draw);

Math.random() should be adequate for most pseudo-random use cases that don't need to be cryptographically secure.

You can consider using RandomSource.getRandomValues() (MDN Documentation) for a a more random distribution, but this may also come at the cost of more resources being utilized in generating the random number.

You may also have cross-browser compatibility issues as this is newer javascript API.


Implementation of insertion sort in JavaScript I have written an insertion sort algorithm in JavaScript:

function insertionSort(inputArray){
  var totalElements = inputArray.length - 1;
  var temp = 0;
  var lastIndex = 0;
  var prevIndex = 0;
  for(var i = 0; i <= totalElements; i++){
    for(var currentIndex = i; currentIndex > lastIndex; currentIndex--){
        prevIndex = currentIndex - 1;
        if(inputArray[currentIndex] < inputArray[prevIndex]){
            temp = inputArray[currentIndex];
            inputArray[currentIndex] = inputArray[prevIndex];
            inputArray[prevIndex] = temp;
        }else{
            break;
        }
    }

  }

  return inputArray;
}

console.log(insertionSort([1,31,26,4,3,12]));
console.log(insertionSort([5,6,1,2,3,4]));

The output is also proper:

rahul@rahul:~/myPractise/Algo$ node sorting.js
[ 1, 3, 4, 12, 26, 31 ]
[ 1, 2, 3, 4, 5, 6 ]
rahul@rahul:~/myPractise/Algo$

Can this be further improved?

Your function can only sort numbers and strings

The built in Array.sort function offers an optional compare function as a parameter. Adding this feature to your insertionSort function would allow for users to sort any type of object:

function defaultCompare(x, y) {
  if (x < y)
    return -1;
  else if (x > y)
    return 1;
  else
    return 0;
}

function insertionSort(array, compare) {
  compare = compare ? compare : defaultCompare;
  //...
}

You have quite a few variables that aren't necessary

totalElements

You only use this variable once, in the condition of your first for loop. JavaScript has some incredibly fast engines and with JIT-Compiler (just in time compiler) you don't have to worry about micromanaging optimizations like what if js isn't smart enough to know that it's recalculating inputArray.length - 1 each iteration. Defining totalElements arguably makes your code harder to read by adding another variable the scope, and the JS engines are smart-enough/fast-enough that performance isn't likely affected, so I would recommend getting rid of it.

temp

You could eliminate temp by abstracting out a function to swap indices in an array:

function swapIndices(array, index1, index2) {
  var temp = array[index1];
  array[index1] = array[index2];
  array[index2] = temp;
}

This also improves your code's cohesiveness by abstracting out the technicalities of swapping variables, and it allows you to use swapIndices elsewhere in your code.

lastIndex

This one is just a constant for zero. At the very least you should change the var to const:

const lastIndex = 0;

But I'd recommend to just get rid of it.

prevIndex

I find currentIndex - 1 more clear than prevIndex. The former is obviously the index 1 less than currentIndex whereas the latter could also mean the previously swapped element (precedingIndex is another possible name).

Summary

With all the above changes your code would look like:

function defaultCompare(x, y) {
  if (x < y)
    return -1;
  else if (x > y)
    return 1;
  else
    return 0;
}

function swapIndices(array, index1, index2) {
  var temp = array[index1];
  array[index1] = array[index2];
  array[index2] = temp;
}

function insertionSort(array, compare) {
  compare = compare ? compare : defaultCompare;

  for (let i = 0; i < array.length; i++) {
    for (let currIndex = i; currIndex > 0; currIndex--) {
      if (compare(array[currIndex], array[currIndex - 1]) < 0) {
        swapIndices(array, currIndex, currIndex - 1);
      }
      else {
        break;
      }
    }
  }

  return array;
}


Simple Java addition calculator I made this simple addition calculator in Java and I'm wondering how I can improve it.

package com.craftxbox.main;

import java.util.Scanner;

public class Main {
    public static void main(String[] args){
        int inInt1;
        int inInt2;
        int outInt;
        @SuppressWarnings("resource")
        Scanner in = new Scanner(System.in);
        inInt1 = in.nextInt();
        inInt2 = in.nextInt();
        outInt = inInt1 + inInt2;
        System.out.println(outInt);

    }
}

You have written some good code, but what if the user does not enter a number, but makes a mistakes? You can, and should, catch that error. What about this:

import java.util.InputMismatchException;
import java.util.Scanner;

public class Calculator
{
    static Scanner scanner;

    public static void main(String args[])
    {
       try
       {
           scanner = new Scanner(System.in);
           double firstNum, secondNum, answer;

           System.out.println("Enter your first number: ");
           firstNum = scanner.nextDouble();

           System.out.println("Enter your second number: ");
           secondNum = scanner.nextDouble();

           answer = firstNum + secondNum;
           System.out.println(answer);
       }
       catch (InputMismatchException e)
       {
           System.out.println("Invalid input");
       }
       finally
       {
           scanner.close();
       }

}

}

I think a try - catch - finally is a great way of getting user input. This way your problem of the Scanner not being closed, is solved. If this is a little over you head right now, don't worry. It does what it says: we "try" the code in the try block, but if something goes wrong, in this case a "Inputmismatchexception", which means if the user inputs something wrong, we catch it and we display a message saying something went wrong.

Note that I have also changed the variable names. By reading the name, you should immediately know what it is and so names like firstNum and answer are a little better than inInt1 and stuff.

Hope you're still with me. If something is not clear, please feel free to ask some more questions. I myself am a beginner to Java and programming in general and this is the first question I answered.


Three Card Monte game using Java I made a Three Monte Card game using Java
Problem:

This is the original "ball and cups" game where you try to find out which cup has the ball under it. You may play with three cups and a ball, three cards (two jacks and an ace), or three doors and a car. Basically, randomly select a cup to hide the "ball". Let the player guess which cup hides the ball. The player wins if they guess correctly.

Problem taken from:

https://programmingbydoing.com/a/three-card-monte.html

My code:
/*
 * Code by CLint
 */

import java.util.Random;
import java.util.Scanner;

public class ThreeCardMonte {
    public static void main(String[] args) {
        Random random = new Random();
        Scanner input = new Scanner(System.in);
        int userInput;

        int randomN = random.nextInt(3)+1;

        System.out.println("You slide up to Fast Eddie's card table and plop down your cash.\n" +
                "He glances at you out of the corner of his eye and starts shuffling.\n" +
                "He lays down three cards.\n");
        System.out.println("Which one is the ace?\n" +
                "\t##\t##\t##\n" +
                "\t##\t##\t##\n" +
                "\t1\t2\t3");
        System.out.print("\n> ");
        userInput = input.nextInt();

        if (userInput == randomN) {
            if (randomN == 1) {
                System.out.println("\nYou nailed it! Fast Eddie reluctantly hands over your winnings, scowling.\n" +
                        "\tAA\t##\t##\n" +
                        "\tAA\t##\t##\n" +
                        "\t1\t2\t3");
            } else if (randomN == 2) {
                System.out.println("\nYou nailed it! Fast Eddie reluctantly hands over your winnings, scowling.\n" +
                        "\t##\tAA\t##\n" +
                        "\t##\tAA\t##\n" +
                        "\t1\t2\t3");
            } else if (randomN == 3) {
                System.out.println("\nYou nailed it! Fast Eddie reluctantly hands over your winnings, scowling.\n" +
                        "\t##\t##\tAA\n" +
                        "\t##\t##\tAA\n" +
                        "\t1\t2\t3");
            }
        }

        if (userInput != randomN) {
            if (randomN == 1) {
                System.out.println("\nHa! Fast Eddie wins again! The ace was card number 1.\n" +
                        "\tAA\t##\t##\n" +
                        "\tAA\t##\t##\n" +
                        "\t1\t2\t3");
            } else if (randomN == 2) {
                System.out.println("\nHa! Fast Eddie wins again! The ace was card number 2.\n" +
                        "\t##\tAA\t##\n" +
                        "\t##\tAA\t##\n" +
                        "\t1\t2\t3");
            } else if (randomN == 3) {
                System.out.println("\nHa! Fast Eddie wins again! The ace was card number 3.\n" +
                        "\t##\t##\tAA\n" +
                        "\t##\t##\tAA\n" +
                        "\t1\t2\t3");
            }
        }

    }
}

The output:

PS: I am open for corrections to make my code for efficient and clean.

single responsibility
your class does all things together

reading user input
printing dialogs
handle game logic

by this you violate the openClosed principle. If you want to change your code (eg. play a one of four game) you have to make a lot of changes. If you want to add handling for InputExceptions (see Scanner.nextInt()  you would find the proper place. Make Classes for each responsibility!
minor issues

magic numbers: (3)+1
hardcoded Strings (put them in a language file) - that also helps to separate concerns between dialogs and logic
redundancy (use a formatter for the text from the language files)


Identify MOT Dates which could be in various formats I want to significantly tidy this code as its cluttered and repetitive

var myDate = "No MOT"

if ((myData[0]["motTests"][0]["expiryDate"].string) != nil) {

    let theDate=myData[0]["motTests"][0]["expiryDate"].string
    let msYear = theDate!.prefix(4)
    let msMonth = theDate!.suffix(5).prefix(2)
    let msDay = theDate!.suffix(2)
    let mYear = Int (msYear)
    let mMonth = Int (msMonth)
    let mDay = Int(msDay)
    self.motDue = self.convertDate(aYear: mYear!, aMonth: mMonth!, aDay: mDay!)

    myDate=msDay+("/")+msMonth+"/"+msYear
}
if ((myData[0]["motTestExpiryDate"].string) != nil){
    let theDate=myData[0]["motTestExpiryDate"].string

    let msYear = theDate!.prefix(4)
    let msMonth = theDate!.suffix(5).prefix(2)
    let msDay = theDate!.suffix(2)

    let mYear = Int (msYear)
    let mMonth = Int (msMonth)
    let mDay = Int(msDay)
    self.motDue = self.convertDate(aYear: mYear!, aMonth: mMonth!, aDay: mDay!)

    myDate=msDay+("/")+msMonth+"/"+msYear
}

self.myMOT.text=String(myDate)

The code checks to see if the vehicle has an MOT, if it does its sets myDate to the date. If not it checks when the first MOT is due and sets myDate to that's and if not myDate stays set to "No MOT"

It works but needs improving.

Best practice is to get away from manipulating date strings yourself. And use Date types in your model.

So, when you parse the JSON, use DateFormatter to convert the non-user-friendly date strings to Date objects. And when presenting the dates in the UI, use another DateFormatter to present the date in a nice, user-friendly format. I’d also suggest using JSONDecoder for decoding the JSON into your model, so you don’t have to sprinkle your code with cryptic, error-prone, dictionary keys.

So, first, I would suggest specifying a date formatter when you decode. Because most of the dates are in the form yyyy.MM.dd, I’d use that. So, for example:

do {
    let decoder = JSONDecoder()
    let formatter = DateFormatter()
    formatter.locale = Locale(identifier: "en_US_POSIX")
    formatter.dateFormat = "yyyy.MM.dd"
    decoder.dateDecodingStrategy = .formatted(formatter)
    let result = try decoder.decode([Vehicle].self, from: data)
    print(result)
} catch {
    print(error)
}

The trick, though, is that one field, completedDate in MOTTest, uses a different string format, namely, yyyy.MM.dd HH:mm:ss, so I’d have JSONDecoder just parse that as a string, but then have a computed property to translate that one date/time string into a Date:

struct Vehicle: Decodable {
    let registration: String
    let make: String
    let model: String
    let firstUsedDate: String
    let fuelType: String
    let primaryColour: String
    let vehicleId: String
    let registrationDate: Date
    let manufactureDate: Date
    let engineSize: String
    let motTests: [MOTTest]
}

struct MOTTest: Decodable {
    private static let formatter: DateFormatter = {
        let formatter = DateFormatter()
        formatter.locale = Locale(identifier: "en_US_POSIX")
        formatter.dateFormat = "yyyy.MM.dd HH:mm:ss"
        return formatter
    }()

    let completedDateString: String
    var completedDate: Date? { MOTTest.formatter.date(from: completedDateString) }
    let testResult: String
    let expiryDate: Date?
    let odometerValue: String
    let odometerUnit: String
    let odometerResultType: String
    let motTestNumber: String
    let rfrAndComments: [ReasonForRejection]

    private enum CodingKeys: String, CodingKey {
        case completedDateString = "completedDate"
        case testResult, expiryDate, odometerValue, odometerUnit, odometerResultType, motTestNumber, rfrAndComments
    }
}

struct ReasonForRejection: Decodable {
    let text: String
    let type: String
    let dangerous: Bool
}

That way, JSONDecoder not only does the decoding for us to our model structure (avoiding that unstructured collection returned by JSONSerialization), but it also converts all of those yyyy.MM.dd strings to Date instances for us. And for completedDate, our computed property takes care of that for us.

By the way, I used the following sample JSON:

[
    {
        "registration": "ZZ99ABC",
        "make": "FORD",
        "model": "FOCUS",
        "firstUsedDate": "2010.11.13",
        "fuelType": "Petrol",
        "primaryColour": "Yellow",
        "vehicleId": "4Tq319nVKLz+25IRaUo79w==",
        "registrationDate": "2010.11.13",
        "manufactureDate": "2010.11.13",
        "engineSize": "1800",
        "motTests":[
            {
                "completedDate": "2013.11.03 09:33:08",
                "testResult": "PASSED",
                "expiryDate": "2014.11.02",
                "odometerValue": "47125",
                "odometerUnit": "mi",
                "odometerResultType": "READ",
                "motTestNumber": "914655760009",
                "rfrAndComments": []
            },
            {
                "completedDate": "2013.11.01 11:28:34",
                "testResult": "FAILED",
                "odometerValue": "47118",
                "odometerUnit": "mi",
                "odometerResultType": "READ",
                "motTestNumber": "841470560098",
                "rfrAndComments":[
                    {
                        "text": "Front brake disc excessively pitted (3.5.1h)",
                        "type": "FAIL",
                        "dangerous": true
                    },
                    {
                        "text": "Nearside Rear wheel bearing has slight play (2.6.2)",
                        "type": "ADVISORY",
                        "dangerous": false
                    }
                ]
            },
            {
                "completedDate": "2018.05.20 11:28:34",
                "testResult": "FAILED",
                "odometerValue": "57318",
                "odometerUnit": "mi",
                "odometerResultType": "READ",
                "motTestNumber": "741489560458",
                "rfrAndComments":[
                    {
                        "text": "Nearside Parking brake efficiency below requirements (1.4.2 (a) (i))",
                        "type": "MAJOR",
                        "dangerous": false
                    },
                    {
                        "text": "Front brake disc excessively pitted (3.5.1h)",
                        "type": "DANGEROUS",
                        "dangerous": false
                    },
                    {
                        "text": "tyres wearing unevenly",
                        "type": "USER ENTERED",
                        "dangerous": true
                    }
                ]
            }
        ]
    },
    {
        "registration": "YY09DEF",
        "make": "BMW",
        "model": "Z4",
        "firstUsedDate": "2009.01.25",
        "fuelType": "Petrol",
        "primaryColour": "Green",
        "vehicleId": "3Fv916dPLGx=43PRaKa45e++",
        "registrationDate": "2009.01.25",
        "manufactureDate": "2009.01.25",
        "engineSize": "1800",
        "motTests":[
            {
                "completedDate": "2012.01.10 10:27:56",
                "testResult": "PASSED",
                "expiryDate": "2013.01.09",
                "odometerValue": "12345",
                "odometerUnit": "mi",
                "odometerResultType": "READ",
                "motTestNumber": "345655760009",
                "rfrAndComments": []
            }
        ]
    }
]

Needless to say, if you want to decode manually, like you did in your question, just have two parsing date formatters, one for the yyyy.MM.dd format and another for the yyyy.MM.dd HH:mm:ss format.

Finally, you might ask “how do I show these Date objects in my UI?”

You define UI date formatters:

let dateOnlyFormatter: DateFormatter = {
    let formatter = DateFormatter()
    formatter.dateStyle = .medium
    return formatter
}()

let dateTimeFormatter: DateFormatter = {
    let formatter = DateFormatter()
    formatter.dateStyle = .medium
    formatter.timeStyle = .medium
    return formatter
}()

Then you can do things like:

if let date = result.first?.motTests.first?.expiryDate {
    let string = dateOnlyFormatter.string(from: date)
    print("Expiry date:", string)
}

  Expiry date: 2 Nov 2014

And

if let date = result.first?.motTests.first?.completedDate {
    let string = dateTimeFormatter.string(from: date)
    print("Completed date:", string)
}

  Completed date: 3 Nov 2013 at 9:33:08

Clearly, use whatever dateStyle and timeStyle you want. But the idea is that we’ll show these strings in a nice, localized format as dictated by the user’s device settings.


Manage InputBox method reference entry Exploring the VBA Application.InputBox method I've realized that catching user cancellation is tricky when type is reference, so return value is a range object.

As cancellation's return value is False I can't assign it to an object with Set, so I've created a UDF to return Nothing when user pressed cancel / Esc.

Function mjInputBox(prompt As String, title As String, default As Variant, dataType As Integer) As Variant

    If dataType = xlInputReference Then
        On Error GoTo InputCancelled
        Set mjInputBox = Application.InputBox(prompt:=prompt, title:=title, default:=default, Type:=dataType)
        On Error GoTo 0
    Else
        mjInputBox = Application.InputBox(prompt:=prompt, title:=title, default:=default, Type:=dataType)
    End If

    Exit Function

InputCancelled:
    Err.Clear
    Set mjInputBox = Nothing
End Function

(xlInputReference is a constant = 8)

Is it a good practice, or can I improve it?

having a one line code instead of the UDF would be a big improvement
According to MSDN:

  For example, for an input box that can accept both text and numbers,
  set Type to 1 + 2.

Is it possible to combine input types including reference too? (note that I can assign return value of inputBox without set which will combine my reference to a cell value, which isn't acceptable).

I'm not sure where you got xlInputReference from, or 16. According to the MSDN page you linked, the correct type for a reference input is 8.

I'm not sure I like your approach. You only need this for range inputs. Personally, I would handle this (prompting for range inputs) by asking for a cell reference (as text) then converting that text into a range object separately (with appropriate error handling).

But, to take your current approach:

As it stands, I've got very little that I can see improvements for.

You shouldn't use Integer. It is completely deprecated. Use Long instead.

Your method name isn't great and breaks convention (that Metohd names use PascalCase). I would use something like HandleInputBox.

Since yes, you can have compound return types but obviously, trying to return some combination of object + text + number is going to fail, you probably want to check for it:

If dataType Mod 16 >= 8 Then
    If dataType <> 8 Then
    '/ Compound of 8 with other values
        '/ Error handling
    Else
        On Error GoTo InputCancelled
        Set mjInputBox = Application.InputBox(prompt:=prompt, Title:=Title, Default:=Default, Type:=dataType)
        On Error GoTo 0
    End If
Else
    mjInputBox = Application.InputBox(prompt:=prompt, Title:=Title, Default:=Default, Type:=dataType)
End If

You should probably implement all the other optional arguments for Application.InputBox() as well since you only have to pass them through.

Other than that, I have nothing to critique. The code is good.


Get value from a dictionary contained in a dictionary I have an API that returns a dictionary of dictionaries.

I currently have this code to access the value of the dictionary, but it looks like a code smell. Is there a better way to structure this?

// Dictionary<string, Dictionary<DateTime, string>> CountryCode();

var response = api.CountryCode();

string GB2001;
if (response.ContainsKey("GB") && response["GB"].ContainsKey(new DateTime(2001, 04, 01)))
{
    GB2001 = response["GB"][new DateTime(2001, 04, 01)];

    // Use the result of GB2001 here
}

The 'problem' with this approach is that you perform a lookup twice for each key.

ContainsKey
indexer lookup [..]

code

if (response.ContainsKey("GB") && response["GB"].ContainsKey(new DateTime(2001, 04, 01)))
{
    GB2001 = response["GB"][new DateTime(2001, 04, 01)];
}

Use TryGetValue to combine lookup and indexer.

string GB2001;
if (response.TryGetValue("GB", out var country))
{
    country?.TryGetValue(new DateTime(2001, 04, 01), out GB2001);
}


Pythonic code to convert 3 digit number into all possible letter combinations Given a dictionary where 1:a , 2:b ... 26:z. I need to find all the possible letter combinations that can be formed from the three digits.

Either each digit should translate to a letter individually or you can combine adjacent digits to check for a letter. You can't change the order of the digits. For example -

121 translates to aba, au, la;

151 translates to aea, oa;

101 translates to ja;

I was able to get this working but I feel my code is not very "pythonic". I am trying to figure out a more efficient & python-like solution for this problem.

# creating the dict that has keys as digits and values as letters
root_dict = {}
for num in range(0,26):
    root_dict[str(num+1)] = string.ascii_lowercase[num]

# asking user for a three digit number
sequence_to_convert = raw_input('Enter three digit number \n')

# storing all possible permutations from the three digit number
first_permutation = sequence_to_convert[0]
second_permutation = sequence_to_convert[1]
third_permutation = sequence_to_convert[2]
fourth_permutation = sequence_to_convert[0]+sequence_to_convert[1]
fifth_permutation = sequence_to_convert[1]+sequence_to_convert[2]

# checking if the permutations exist in the dict, if so print corresponding letters
if first_permutation in root_dict and second_permutation in root_dict and third_permutation in root_dict:
    print root_dict[first_permutation]+root_dict[second_permutation]+root_dict[third_permutation]
if first_permutation in root_dict and fifth_permutation in root_dict:
    print root_dict[first_permutation]+root_dict[fifth_permutation]
if fourth_permutation in root_dict and third_permutation in root_dict:
    print root_dict[fourth_permutation]+root_dict[third_permutation]

First, Python 2 is going to be no longer supported in less than a year. If you are starting to learn Python now, learn Python 3. In your code the only differences are that print is a function now and no longer an expression (so you need ()) and that raw_input was renamed input (and the Python 2 input basically no longer exists).

Your building of the root dictionary can be simplified a bit using a dictionary comprehension:

from string import ascii_lowercase

num_to_letter = {str(i): c for i, c in enumerate(ascii_lowercase, 1)}

For the first three different permutations you can use tuple unpacking:

first, second, third = sequence_to_convert

Note that you are currently not validating if the user entered a valid string. The minimum you probably want is this:

from string import digits
digits = set(digits)

sequence_to_convert = input('Enter three digit number \n')
if len(sequence_to_convert) != 3:
    raise ValueError("Entered sequence not the right length (3)")
if not all(x in digits for x in sequence_to_convert):
    raise ValueError("Invalid characters in input (only digits allowed)")

(A previous version of this answer used str.isdigit, but that unfortunately returns true for digitlike strings such as "¹"...)

Your testing and printing can also be made a bit easier by putting the possible permutations into a list and iterating over it:

permutations = [(first, second, third), (first, fifth), (fourth, third)]
for permutation in permutations:
    if all(x in num_to_letter for x in permutation):
        print("".join(map(num_to_letter.get, permutation)))

However, in the end you would probably want to make this more extendable (especially to strings longer than three). For that you would need a way to get all possible one or two letter combinations, and that is hard. It is probably doable with an algorithm similar to this one, but it might be worth it to ask a question on Stack Overflow about this.


Given a list and a number k return whether any two numbers from the list add up to k The task:

  Given a list of numbers and a number k, return whether any two numbers
  from the list add up to k.

  For example, given [10, 15, 3, 7] and k of 17, return true since 10 +
  7 is 17.

  Bonus: Can you do this in one pass?

const lst = [14, 10, 3, 7, 0];
const k = 14;

My solution 1:

const addUpTo = (lst, k) => lst.some((x, i) => !isNaN(lst.find((y, j) => i !== j && y === k - x)));

console.log(addUpTo(lst, k));

My solution 2:

const addUpTo2 = (lst, k) => lst.some((x, i) => {
  const find = k - x;
  return !isNaN(lst.slice(i + 1).find(y => y === find));
});

console.log(addUpTo2(lst, k));

These solutions have quadratic runtime.  You can store counts (in an object) and check the object for k-x, or sort the array, check values up to k/2 and binary search for k-x.  The second approach is asymptotically slower but constant storage.


Interpolating code delimited with character that can appear in code I've got a string that consists of an arbitrary combination of text and {} delimited python code, for instance, A plus b is {a + b}.  However, braces are used for dictionary and set literals in python, so You chose the {{1:"first", 2:"second"}[choice]} option should also be interpretted correctly.  It's also valid to have more than one python expression in the input, so You picked {choice1} and {choice2} is valid.

Here's my current code:

protected String ParseStringForVariable([NotNull] String str)
{
    PythonEngine.PythonEngine pythonEngine = GetCurrentCore().PythonEngine;

    for (int i = 0; i < str.Length; i++)
    {
        if (str[i] != '{')
        {
            continue;
        }

        int opening = i;
        foreach (var expression in from closing in str.IndexesWhere('}'.Equals)
                                   where closing > opening
                                   select new
                                          {
                                              Template = str.Substring(opening, closing - opening + 1),
                                              Code = str.Substring(opening + 1, closing - opening - 1)
                                          })
        {
            PythonByteCode compiled;
            try
            {
                compiled = pythonEngine.Compile(expression.Code, PythonByteCode.SourceCodeType.Expression);
            }
            catch (PythonParseException)
            {
                // not valid python, try next expression
                continue;
            }
            String result = pythonEngine.Evaluate(compiled).ToString();
            str = str.Replace(expression.Template, result);
            break;
        }
    }

    return str;
}

It works by looking at progressively longer strings, attempting to parse them, and ignoring them if it's not valid python.  This is done with the PythonEngine class, which is a wrapper around the IronPython interpretter, and has an extensive test suite, so can assumed to be correct.

It appears to burp slightly if the value of the first of multiple python expression contains an opening brace: {"{"} {"}"}, what's the best way to prevent that?
Resharper is complaining about access to modified closures, can you provide a test case that exposes why that is an issue?
It works under all inputs I've tested it with, but what edge cases should be included in the test suite? (It's intended behaviour that invalid python code is left as-is).  Current thoughts include:

Empty string
without braces
empty braces
valid python in braces
invalid python
both valid and invalid
both valid and empty
both invalid and empty
valid, nested braces
invalid, nested braces
valid, evaluated to contain open brace
valid, evaluated to contain close brace - irrelevant, as parsing is LTR?
valid, evaluated to contain open brace followed by further invalid
valid, evaluated to contain open brace followed by further empty
unmatched open brace
unmatched close brace

Are there any improvements that jump out?  Clarity, performance, style?

I see an extra unnecessary variable that you can get rid of, or change another variable so that it makes more sense.

int opening = i;

you use this inside of a for loop.  You can do 2 things with this.

Just use the variable i where ever you had the variable opening
change the i variable to opening in the for loop like

for (int opening = 0; opening < str.Length; opening++)

hopefully this shows you how unnecessary that extra variable is, it isn't used anywhere else for anything other than the loops.

Other than that I would need to play with it and spend some time with it to see if it plays well with others.


Nodejs Async get request I made a small application that makes async calls to an api (local in this case). The target application will make a million of calls per day.

With maxSockets set to 1 I had a time of 45 seconds. With maxSockets = 10, the execution time has dropped to 40 seconds.

How can I make this code go faster please?

var request = require('request');
var mysql      = require('mysql');
var async = require('async');

var connection = mysql.createConnection({
    host     : 'localhost',
    user     : 'root',
    password : '',
    database : 'testAPI'
});
var inputData = [];
for(i=1; i<=9999;i++){
    inputData.push('number' + i);
}
var options = {
    url: 'http://localhost/test.txt',
    maxSockets: 10
};
var limit = inputData.length;
var counter = 0;

var fetch = function(file,cb){
    request(options, function(error, response, body) {
        if (!error && response.statusCode == 200) {
            var time = (new Date().getTime()/1000)
            connection.query("insert into testAPI (name) values ('" + time + body +"')", function(err, rows, fields) {
                if (err) throw err;
                //console.log('Inserted: ' + time + body)
                counter++;
            });
        }
    })
}

async.map(inputData,fetch,function(err, results){
    if ( err){
        // either file1, file2 or file3 has raised an error, so you should not use results and handle the error
    } else {
        options.count = i;
//        request(options, fetch)
        // results[0] -> "file1" body
        // results[1] -> "file2" body
        // results[2] -> "file3" body
    }

})
var checkIfFinished = setInterval(function(){
    if(counter === limit){
        console.log('finished')
        connection.end();
        clearInterval(checkIfFinished);
    }
},1000)

When you use a control-flow library, you don't poll. That's what the callbacks are for. They get called when operations complete. That's how you know you're done.

Let's start by eliminating dependencies as they are black boxes, and we don't really know how they are built. If you use newer versions of Node.js, you can use Promises which should pretty much replace async. As for request, you can simply use Node's http.get to read the remote file.

All your API calls can also be transformed into promise-returning functions. If you can't find an existing module that already does it for you, you can simply wrap the call in a Promise and call resolve or reject accordingly. What I normally suggest is to split operations into a promise-returning function, and leave it to the caller to chain them up. In this case, log manages the sequence of readFile and insertToDb.

function insertToDb(body){
  return new Promise((resolve, reject) => {
    var time = Date.now();
    var query = `insert into testAPI (name) values ('${time}${body}')`;
    connection.query(query, (err, rows, fields) => {
      if(err) reject(err);
      else resolve(body);
    });
  });
}

function readFile(file){
  return new Promise((resolve, reject) => {
    http.get(url, body => {
      if(error) reject(error);
      else resolve(body);
    });
  });
}

function log(file){
  return readFile(file)
    .then(fileData => {
      // Now we have read the file, time to insert to db
      return insertToDb(fileData);
    }, error => {
      // Failed to read file
    })
    .then(fileData => {
      // Insert succeeded, we conclude by resolving with the file contents
      return fileData;
    }, error => {
      // DB operation failed
    });
}

async.map simply just calls each value against an iterator then calls the callback when everything's done. This is something easily done with array.map and Promise.all. We can use array.map to call log and collect the returned promises to an array, then we use Promise.all to wait for all promises to resolve.

Promise.all(inputData.map(file => log(file))).then(results => {
  // results[0] -> "file1" body
  // results[1] -> "file2" body
  // results[2] -> "file3" body

  console.log('finished');
}, () => {
  // Something failed
});

Now reading a remote file 9999 times (and over HTTP) and writing to the db 9999 times in rapid succession is the real bottleneck here. You're essentially flooding both servers. Even on localhost, servers still have CPU and memory constraints. No matter how you make this script run faster, the script will run no faster than the slowest of the two.

I believe what you're simply doing is trying to log changes of the file into the DB. I suggest you read and insert at a slower interval, like say every 5 seconds. That should give the servers enough time to breathe. Besides, do you really need to read the file in a way that reads are milliseconds apart. That's some file you got there.

Other alternatives would be to stream the data instead of reading the file at an interval. For the DB, try looking into solutions like Redis.

A few other things:

You can use Template Strings instead of string concatenation to build strings.
Instead of new Date().getTime(), use Date.now(). Does the same thing without creating a new Date object.
fetch isn't really "fetching", it appears to read a file and store it to the DB. It's technically "logging". Name your variables and functions properly.


Codewars: Reduce strings to one character I am trying to solve a puzzle in a performant way. This is the puzzle.

Simply explained: you have a String and you must reduce it to one character with the given rules. This is the solution to the simple variant.

COLORS = set("RGB")
def triangle(row):
    while len(row) > 1:
       row = ''.join(a if a == b else (COLORS-{a, b}).pop() for a, b in zip(row, row[1:]))
    return row

In the harder variant the task is to do the same, but performance is asked.
I thought adding a dictionary and "remembering" already solved words would boost the performance. But actualy it is not performing faster than the simple variation. Is there a way to boost one of my codes? Do I have an efficiency lack in my code? Is there maybe a efficiency lack in the algorithm?

COLORS = set("RGB")
mapping_simple = {}
def triangle_simple(row):
    result = ['']
    while True:
        if len(row) == 1:
            result[0] = row
            return result[0]
        if row in mapping_simple:
            result[0] = mapping_simple[row][0]
            return result[0]
        mapping_simple[row] = result
        row = ''.join(a if a == b else (COLORS-{a, b}).pop() for a, b in zip(row, row[1:]))
    return result[0]

First I would choose a different representation of the colors during
the computation, to avoid the “expensive” COLORS-{a, b}).pop()
    operation, which is executed
$$
   (n-1) + (n-2) + \ldots + 1 = \frac{(n-1)n}{2}
$$
times for an input string of length \$ n \$. It becomes simpler if we
represent the colors as numbers instead, for example
$$
   \text{Red} = 0, \text{Green} = 1, \text{Blue} = 2 \, .
$$
Now the combination of two colors a and b can be computed as

a if a == b else 3 - a - b

without requiring any set or dictionary lookup. This leads to the
following implementation:

def color_to_num(col):
    """ Translate the colors 'R', 'G', 'B' to the numbers 0, 1, 2. """
    return 0 if col == 'R' else 1 if col == 'G' else 2

def num_to_color(num):
    """ Translate the numbers 0, 1, 2 to the colors 'R', 'G', 'B'. """
    return 'R' if num == 0 else 'G' if num == 1 else 'B'

def triangle(row):
    numbers = [color_to_num(col) for col in row]
    while len(numbers) > 1:
       numbers = [ a if a == b else 3 - a - b for a, b in zip(numbers, numbers[1:])]
    return num_to_color(numbers[0])

In my test (on a 1.2 GHz Intel Core m5 MacBook) this reduces the time for
an input string with 10,000 characters from (approximately) 18 seconds to
5.5 seconds.

But one can do better. The above solution still has a \$ O(n^2) \$ complexity.
In order to get rid of that, we need some mathematics (compare
Three Color Triangle Challenge
on Mathematics Stack Exchange).

The are two key observations:

If all (numeric) colors are viewed “modulo 3” then the combination of
\$ a \$ and \$ b \$ is just \$ -(a+b) \$.
The final color can be computed “directly” from the initial row, using
binomial coefficients (the numbers from Pascal's triangle).

Here is an example for the first observation: The combination of Red and Blue
is computed as
$$
  - (0 + 2) \bmod 3 = -2 \bmod 3 = 1
$$
and that is Green.

And an example for the second observation with an initial row of 4 colors:
$$
 a_0, a_1, a_2, a_3 \\
 -(a_0 + a_1), -(a_1 + a_2), -(a_2 + a_3) \\
 a_0 + 2a_1 + a_2, a_1 + 2a_2 + a_3 \\
 -(a_0 + 3a_1 + 3 a_2 + a_3)
$$

The general formula for an initial row of \$ n+1 \$ colors
$$
 a_0, a_1, \ldots, a_n
$$
is
$$
 \text{FinalColor} = (-1)^n \sum_{k=0}^n \binom{n}{k} a_k \bmod 3 \, .
$$

There is still one problem: the calculation of the binomial coefficients.
Using the definition
$$
 \binom{n}{k} = \frac{n(n-1) \cdots (n-k+1)}{k!}
$$
can lead to large intermediate results and has complexity \$ O(k) \$,
so that the total complexity is still \$ O(n^2) \$.

Here we need mathematics again: Lucas's theorem
provides a way to calculate \$  \binom{n}{k} \bmod p \$ efficiently
if \$ p \$ is a prime number. Luckily, 3 is a prime number!

This gives the following code:

def binomial_mod3(n, k):
    """ Compute the binomial coefficient C(n, k) modulo 3.

    It is assumed that 0 <= k <= n.

    The implementation uses Lucas's theorem, see for example
    https://en.wikipedia.org/wiki/Lucas%27s_theorem .
    """
    result = 1
    while n > 0:
        n3 = n % 3
        k3 = k % 3
        # 'Ad hoc' computation of C(n3, k3):
        if k3 > n3:
            return 0  # Return immediately if a factor is zero.
        temp = 1 if k3 == 0 or k3 == n3 else 2
        result = (result * temp) % 3
        n = n // 3
        k = k // 3
    return result

def color_to_num(col):
    """ Translate the colors 'R', 'G', 'B' to the numbers 0, 1, 2. """
    return 0 if col == 'R' else 1 if col == 'G' else 2

def num_to_color(num):
    """ Translate the numbers 0, 1, 2 to the colors 'R', 'G', 'B'. """
    return 'R' if num == 0 else 'G' if num == 1 else 'B'

def triangle(row):
    # Compute the sum of C(n, k) * col[k] modulo 3:
    n = len(row) - 1
    result = 0
    for k, col in enumerate(row):
        temp = binomial_mod3(n, k) * color_to_num(col)
        result = (result + temp) % 3
    # Invert the result if n is odd:
    if n % 2 == 1:
        result = (- result) % 3
    return num_to_color(result)

Note that we iterate over the given input string, so that
no additional lists are created.

In my test this computes the final color for an input string with 10,000
characters in approximately 0.1 seconds, and for 100,000 characters in
about 0.2 seconds.


JS calculator, respecting order of operations I have a function that receives an array of numbers and operations

Ex: userEntry = [3,+,3,x,3] -> returns 12

It is a calculator that adheres to the proper order of operations, therefore multiply/divide behaves differently than addition/subtraction. I have one redundant line of code:

userEntry = calculatorOperations
    .calculationSequence(operationsMD[i], indexOfOperand, userEntry);

is similar to:

 userEntry = calculatorOperations
    .calculationSequence(userEntry[1], indexOfOperand, userEntry);

With different behavior between operations, is it possible to remove this redundant line or is it neccesary?

function operateOnEntry(userEntry) {
//this is where the calculations occur when hitting =
const operationsMD = ['x', '/'];
let indexOfOperand;
let operation;

while (userEntry.includes('x') || userEntry.includes('/')) {
    let i = 0;
    if (!userEntry.includes('x')) {
        i++;
    }
    indexOfOperand = userEntry.indexOf(operationsMD[i]);
    userEntry = calculatorOperations
        .calculationSequence(operationsMD[i], indexOfOperand, userEntry);
}
while (userEntry.includes('+') || userEntry.includes('-')) {
    indexOfOperand = 1;
    userEntry = calculatorOperations
        .calculationSequence(userEntry[1], indexOfOperand, userEntry);
}
return userEntry;
}

let calculatorOperations = {
'x': (arg1, arg2) => {
    return arg1 * arg2;
},
'/': (arg1, arg2) => {
    return arg1 / arg2;
},
'+': (arg1, arg2) => {
    return arg1 + arg2;
},
'-': (arg1, arg2) => {
    return arg1 - arg2;
},
returnIndexOfEntry: (index, userEntry) => {
    let arg1 = Number(userEntry[index - 1]);
    let arg2 = Number(userEntry[index + 1]);
    return [arg1, arg2];
},
returnSpliced: (index, newTotal, userEntry) => {
    userEntry.splice((index - 1), 3, newTotal);
    return userEntry;
},
calculationSequence: (operation, indexOfOperand, userEntry) => {

    let getArgs = calculatorOperations.returnIndexOfEntry(indexOfOperand, userEntry);
    let newTotalForEntry = calculatorOperations[operation](getArgs[0], getArgs[1]);
    let newUserEntry = calculatorOperations.returnSpliced(indexOfOperand, newTotalForEntry, userEntry);
    return newUserEntry;
}
};

Overview

If you are using ES6, you should use the full ability of arrow functions: (args) => returnVal
In addition, some of the methods don't need to be defined as a property.
calculatorOperations should be a constant.
You don't need to access each element in an array like this: (getArgs[0], getArgs[1]), you can do (...getArgs)
I agree with @SamOnela in that you can iterate over the operations. However, their method pollutes global namespace

Questions

Why are you using x not *?

Rewrite

    function operateOnEntry(userEntry) {
        let indexOfOperand,
            operation;
        Object.keys(calculatorOperations.ops).forEach(function(functionName) {
            while (userEntry.includes(functionName)) {
                indexOfOperand = userEntry.indexOf(functionName);
                userEntry = calculatorOperations.calculationSequence(functionName, indexOfOperand, userEntry);
            };
        });
        return userEntry;
    }

    const calculatorOperations = {
        ops: {
            'x': (arg1, arg2) => arg1 * arg2,
            '/': (arg1, arg2) => arg1 / arg2,
            '+': (arg1, arg2) => arg1 + arg2,
            '-': (arg1, arg2) => arg1 - arg2
        },
        returnIndexOfEntry(index, userEntry) {
            let arg1 = Number(userEntry[index - 1]),
                arg2 = Number(userEntry[index + 1]);
            return [arg1, arg2];
        },
        returnSpliced(index, newTotal, userEntry) {
            userEntry.splice((index - 1), 3, newTotal);
            return userEntry;
        },
        calculationSequence(operation, indexOfOperand, userEntry) {

            let getArgs = calculatorOperations.returnIndexOfEntry(indexOfOperand, userEntry),
                newTotalForEntry = calculatorOperations.ops[operation](...getArgs),
                newUserEntry = calculatorOperations.returnSpliced(indexOfOperand, newTotalForEntry, userEntry);
            return newUserEntry;
        }
    };
    var userEntry = [3, '+', 3, 'x', 3];
    console.log(operateOnEntry(userEntry));


Attempting to run multiple simulations of the Gillespie algorithm for a set of stochastic chemical reactions in less than 10 minutes I have written Python code that generates a plot. In the code below, when I set maxtime = 0.1, the program takes ~50s and when I set maxtime = 1, it takes ~420s. I need to run it for maxtime = 1000.

I am familiar with Python syntax and writing "Matlabic" code, but am lost in writing natively "Pythonic" code. As a result, I need help in optimizing this code for runtime, specifically in the two outer for loops and inner while loop.

How can I make the code suitable for use with Numba or Cython?
If that's not possible, do I need to use functions, or map, or lambda statements?

Unfortunately, my Spyder IDE for Python is freezing up every time I try to profile the code. I would include those details if I could!

import numpy as np
import matplotlib.pyplot as plt
import pylab as pl
import math

maxtime = 1
Qpvec = np.logspace(-2,1,101,10)
lenQp = len(Qpvec)
delta = 1e-3
P0vec = Qpvec/delta
SimNum = 100
PTotStoch = np.zeros((SimNum,lenQp))
k_minus = 1e-3
k_cat = 1-1e-3
k_plus = 1e-3
zeta = 1e-4
D0 = 10000
kappa_M = (k_cat+k_minus)/zeta
QpDeg = np.true_divide(1000*D0*Qpvec*k_cat,1000*Qpvec + kappa_M)

for lenQpInd in range(0,lenQp):
    for SimNumInd in range(0,SimNum):
        Qp = Qpvec[lenQpInd]
        P0 = P0vec[lenQpInd]
        DP0 = 0
        P = math.floor(P0)
        DP = DP0
        D = D0
        time = 0

        while time < maxtime:
            u_time = pl.rand()
            u_event = pl.rand()
            rates=np.array([Qp,zeta*P*D,k_minus*DP,k_cat*DP])
            PTot = P + DP
            kT = np.sum(rates)
            tot = np.cumsum(rates)
            deltaT = -np.log(1-u_time)/kT
            time += deltaT
            if time > maxtime:
                PTotStoch[SimNumInd,lenQpInd] = PTot
                break
            elif u_event*kT < tot[0]:
                P += 1
            elif u_event*kT < tot[1]:
                P -= 1
                DP += 1
                D -= 1
            elif u_event*kT < tot[2]:
                P += 1
                DP -= 1
                D += 1
            elif u_event*kT < tot[3]:
                DP -= 1
                D += 1

PMean = PTotStoch.mean(axis=0)
PStd = PTotStoch.std(axis=0)

plt.figure(0)
plt.plot(Qpvec,PMean,marker=".")
plt.errorbar(Qpvec,PMean,yerr = 2*PStd, xerr = None)
plt.show()

Pythonic code follows PEP8, mostly you need to stick to one type of variable format.
In Python they are:

snake_case variables. (functions are variables).
UPPER_SNAKE_CASE constants.
CammelCase classes.

There are a few white space conventions that you didn't follow, such as spaces around operators, and one space after commas.
But all in all you have some nice style.

First things first, don't use pylab. It's not supported,
in-fact I could only find one archive page about it that said to use matplotlib instead.

I assume that pylab.rand is the same as numpy.random.

Due to the above style concerns I found your code hard to read.
But there are also a lot of variables that are written and read once.
This makes the code harder to read, as then you have to remember senseless names such as u_event or kT.

I found the inner part of the while loop hard to read, instead split, the time changes and changes in p, d and dp, into two sections.
This made it easier for me to read and understand.

After doing the above I ran a profiler over the code, and using numpys array and cumsum were dramatic performance sinks.
To resolve this I implemented cumsum in Python, and used a Python array.

This lead to a 260% increase in speed on max_time = 0.1.
Note that I don't test the generation of P0_VEC as I made it a global variable in my code, but didn't in yours.
(So mine has an unfair advantage).

Also as @oliverpool says in the comments, you can get a speed up from using functions. (I put both my code and your code in a function, so the timings wern't extreemly unfair.)

I doubt that you can make the code much faster, as it's seems to be \$O(n)\$ already.
And with my change you are doing 309057 function calls, or 214921 a second.

import numpy as np
import matplotlib.pyplot as plt
random = np.random

# testing functions remove them from this script.
PROFILE = True
TIME = True

# Global constants
DELTA = 1e-3
SIM_NUM = 100
K_MINUS = 1e-3
K_CAT = 1 - 1e-3
ZETA = 1e-4
D = 10000
QP_VEC = np.logspace(-2, 1, 101, 10)
KAPPA_M = (K_CAT + K_MINUS) / ZETA
P0_VEC = QP_VEC / DELTA

def gillespie(max_time=0.01):
    p_tot_stoch = np.zeros((SIM_NUM, len(QP_VEC)))
    for len_qp_ind in range(len(QP_VEC)):
        qp = QP_VEC[len_qp_ind]
        p0 = int(P0_VEC[len_qp_ind])
        for sim_num_ind in range(SIM_NUM):
            p = p0
            d = D
            dp = time = 0

            while True:
                tot = [qp, ZETA * p * d, K_MINUS * dp, K_CAT * dp]
                for i in range(3):
                    tot[i + 1] += tot[i]
                kt = tot[-1]
                time += -np.log(1 - random.random()) / kt
                if time > max_time:
                    p_tot_stoch[sim_num_ind, len_qp_ind] = p + dp
                    break

                event_kt = random.random() * kt
                if event_kt < tot[0]:
                    p += 1
                elif event_kt < tot[1]:
                    p -= 1
                    dp += 1
                    d -= 1
                elif event_kt < tot[2]:
                    p += 1
                    dp -= 1
                    d += 1
                elif event_kt < tot[3]:
                    dp -= 1
                    d += 1

    return p_tot_stoch

if __name__ == '__main__' and PROFILE:
    import cProfile
    cProfile.run('gillespie()')

if __name__ == '__main__' and TIME:
    import timeit
    def gillespie_dot_one():
        return gillespie(0.1)
    def gillespie_one():
        return gillespie(1)
    print('gillespie(0.01)*100 = ', timeit.timeit(gillespie, number=100))
    print('gillespie(0.1)*10 = ', timeit.timeit(gillespie_dot_one, number=10))
    print('gillespie(0.1) = ', timeit.timeit(gillespie_one, number=1))

if __name__ == '__main__':
    p_tot_stoch = gillespie()
    p_mean = p_tot_stoch.mean(axis=0)
    p_std = p_tot_stoch.std(axis=0)

    plt.figure(0)
    plt.plot(QP_VEC, p_mean, marker=".")
    plt.errorbar(QP_VEC, p_mean, yerr=2 * p_std, xerr=None)
    plt.show()


Operations on nested data Problem description.
I have JSON which comes in bad shape:
data = [
    {"ids": [1]},
    {"ids": [3, 4]},
    {"ids": [1, 2]},
    {"ids": [4]},
    {"ids": [3]},
    {"ids": [2]},
] # LD. List of dictionaries.

I want it to get in shape, like this:
expected = [
  [{"ids": [1]}, {"ids": [2]}],        # Length = 1
  [{"ids": [3, 4]}, {"ids": [1, 2]}],  # Length = 2
]  # LOLD. List is now list-of-lists of dictionaries.

To simplify the problem, we can remove the dictionaries of a single kv-pair, keeping in mind that we must reconstruct it later:
# in
[
    [3, 4], [1, 2],
    [1], [4], [3], [2]
]

# out
[
    [[3, 4], [1, 2]]
    [[1], [4], [3], [2]]
]

This is very easy. assemble . op . disassemble $ data:
def main(ids):
    return [list(x) for x in assemble(cardinality_groups(disassemble(ids)))]

def cardinality_groups(lol):
    return [list(group) for _, group in groupby(sorted(lol, key=len), key=len)]

def assemble(data):
    return [tag_datum(x) for x in data]

def tag_datum(datum):
    return [{"ids": x} for x in datum]

def disassemble(ids):
    return [x['ids'] for x in ids]

but
I insist, it must be simpler, purer! Although, I am not sure if Python sports the amenities to make things prettier. So please suggest functionality present in other languages.
I am curious about two ways the program can expand here, and some other things:

By the complexity and nesting of the data. Data can take the form of any JSON found in the wild. Here we simply descend a few levels down.
By the operation performed: Here grouping by cardinality solved the issue. In another world we want no two sets to intersect. Is there any more complex operations, what are they, and do they break anything?
Assembly-disassembly symmetry. The two should be each other's inverse, so can I deduce one from the other, thus not having to code it. Does any language provide such tools?
Beyond typing, are there any languages that support describing how data looks when it comes in, and how it will look when it comes out? Not just the top-level type, but the shape of the data the code works with at that level of abstraction. My presumption is that many programs lend well to this type of reasoning.
I don't like f(g(h(x))). I like f . g . h $ x--it's purer. It really bothers that I can't do something like this in Python, or JavaScript--two of the most popular languages! Consequently, I frequently find myself doing either:

someValue = dostuff(someInput)
valueIsNowSlightlyChanged = doMoreStuff(someValue)
iAmLosingTrack = doStuffMoreNow(valueIsNowSlightlyChanged)
final = wtf(iAmLosingTrack)

return final

Or variations thereof. At this point I don't feel like using either language. Doing things this way isn't, of course, isn't always going to be possible, but I don't even get the opportunity. Am I confused, or do I have a point, and you possibly a solution to my supposed confusion?
\0.
This code was originally written for a question on StackOverflow. I believe that I found a neat way to do it in comparison to the rest. Regardless, I must assume that I don't have the only good solution. Do you have an example?
Feel free to interpret my questions liberally. Apologies if some is beyond the scope of this forum. I appreciate any pointers to literature on these topics, as well as all your impressions.
Please also let me know if I am unclear. Thank you!

You can avoid assemble, disassemble, tag_datum if you leverage on key param of both itertools.groupby and sorted and condense them into one simple function.
Build a custom function for key param
Write a function to return the length of each value in the dictionary.
val_len = lambda x: len(x['ids'])

Now pass this as argument for key in both itertools.groupby and sorted
from itertools import groupby

def transform(data):
    sorted_data = sorted(data, key=val_len)
    return [list(group) for _, group in groupby(sorted_data, key=val_len)]

transform(data) # data taken from question itself.
# [[{'ids': [1]}, {'ids': [4]}, {'ids': [3]}, {'ids': [2]}],
#  [{'ids': [3, 4]}, {'ids': [1, 2]}]]

Details
sorted_data = sorted(data, key=val_len) # sorts data based the length of
                                        # the value of 'ids'
# [{'ids': [1]}, ---|
#  {'ids': [4]},    |-- group with length 1
#  {'ids': [3]},    |
#  {'ids': [2]}, ---|

#  {'ids': [3, 4]},---|-- group with length 2
#  {'ids': [1, 2]}]---|

groupby(sorted_data, key=val_len) # groups data based on the length of
                                  # the value of 'ids'


Deserializing a byte array from Java in C++ I am writing a byte array value into a file, consisting of up to three of these arrays, using Java with big Endian byte order format. Now I need to read that file from a C++ program.

short employeeId = 32767;
long lastModifiedDate = "1379811105109L";
byte[] attributeValue = os.toByteArray();

I am writing employeeId, lastModifiedDate and attributeValue together into a single byte array. I am writing that resulting byte array into a file and then I will have my C++ program retrieve that byte array data from the file and then deserialize it to extract employeeId, lastModifiedDate and attributeValue from it.

This writes the byte array value into a file with big Endian format:

public class ByteBufferTest {

    public static void main(String[] args) {

        String text = "Byte Array Test For Big Endian";
        byte[] attributeValue = text.getBytes();

        long lastModifiedDate = 1289811105109L;
        short employeeId = 32767;

        int size = 2 + 8 + 4 + attributeValue.length; // short is 2 bytes, long 8 and int 4

        ByteBuffer bbuf = ByteBuffer.allocate(size);
        bbuf.order(ByteOrder.BIG_ENDIAN);

        bbuf.putShort(employeeId);
        bbuf.putLong(lastModifiedDate);
        bbuf.putInt(attributeValue.length);
        bbuf.put(attributeValue);

        bbuf.rewind();

        // best approach is copy the internal buffer
        byte[] bytesToStore = new byte[size];
        bbuf.get(bytesToStore);

        writeFile(bytesToStore);

    }

    /**
     * Write the file in Java
     * @param byteArray
     */
    public static void writeFile(byte[] byteArray) {

        try{
            File file = new File("bytebuffertest");

            FileOutputStream output = new FileOutputStream(file);
            IOUtils.write(byteArray, output);

        } catch (Exception ex) {
            ex.printStackTrace();
        }
    }
}

Now I need to retrieve the byte array from that same file using this C++ program and deserialize it to extract employeeId, lastModifiedDate and attributeValue from it. I am not sure what the best way is on the C++ side.

int main() {

    string line;

    std::ifstream myfile("bytebuffertest", std::ios::binary);

    if (myfile.is_open()) {

        uint16_t employeeId;
        uint64_t lastModifiedDate;
        uint32_t attributeLength;

        char buffer[8]; // sized for the biggest read we want to do

        // read two bytes (will be in the wrong order)
        myfile.read(buffer, 2);

        // swap the bytes
        std::swap(buffer[0], buffer[1]);

        // only now convert bytes to an integer
        employeeId = *reinterpret_cast<uint16_t*>(buffer);

        cout<< employeeId <<endl;

        // read eight bytes (will be in the wrong order)
        myfile.read(buffer, 8);

        // swap the bytes
        std::swap(buffer[0], buffer[7]);
        std::swap(buffer[1], buffer[6]);
        std::swap(buffer[2], buffer[5]);
        std::swap(buffer[3], buffer[4]);

        // only now convert bytes to an integer
        lastModifiedDate = *reinterpret_cast<uint64_t*>(buffer);

        cout<< lastModifiedDate <<endl;

        // read 4 bytes (will be in the wrong order)
        myfile.read(buffer, 4);

        // swap the bytes
        std::swap(buffer[0], buffer[3]);
        std::swap(buffer[1], buffer[2]);

        // only now convert bytes to an integer
        attributeLength = *reinterpret_cast<uint32_t*>(buffer);

        cout<< attributeLength <<endl;

        myfile.read(buffer, attributeLength);

        // now I am not sure how should I get the actual attribute value here?

        //close the stream:
        myfile.close();
    }

    else
        cout << "Unable to open file";

    return 0;
}

Can anybody take a look on C++ code and see what I can do to improve it, as I don't think it is looking much efficient? Any better way to deserialize the byte array and extract relevant information on the C++ side?

Obviously the code isn't portable to big-endian machines. I'll use C syntax, since I'm more familiar with that than C++.

If you have endian.h, you can use the functions in there; if not, you should have arpa/inet.h which defines functions for swapping network byte order (big-endian) to host byte order, but lacks a function for 64-bit values. Look for either be16toh (from endian.h) or ntohs (from arpa/inet.h) and friends.

Why not read directly into the values:

fread((void *)&employeeId, sizeof(employeeId), 1, file);
employeeId = be16toh(employeeId);

Since you can manipulate pointers in C, you just need to provide a universal pointer (void *) to the read function where it should place the results. The & operator takes the address of a value. Once that is done, you can manipulate the value directly, as above.

Using this Java test code:

import java.io.*;

public class write {
  public static void main(String... args) throws Exception {
    final FileOutputStream file = new FileOutputStream("java.dat");
    final DataOutputStream data = new DataOutputStream(file);

    final long time = System.currentTimeMillis();
    final short value = 32219;

    //  fill a table with a..z0..9
    final byte[] table = new byte[36];
    int index = 0;
    for (int i = 0; i < 26; i++) {
      table[index++] = (byte)(i + 'a');
    }
    for (int i = 0 ; i < 10; i++) {
      table[index++] = (byte)(i + '0');
    }

    data.writeLong(time);
    data.writeShort(value);
    data.writeInt(table.length);
    data.write(table);
    data.close();

    System.out.format("wrote time: %d%n  value: %d%n  length: %d%n  table:%n", time, value, table.length);
    for (int i = 0; i < table.length; i++) {
      System.out.format("%c ", (char)table[i]);
    }
    System.out.println();
  }
}

The output from this code is:

wrote time: 1380743479723
  value: 32219
  length: 36
  table:
a b c d e f g h i j k l m n o p q r s t u v w x y z 0 1 2 3 4 5 6 7 8 9

You can read the values in with this C code:

#include <stdio.h>
#include <stdlib.h>
#include <endian.h>
#include <sys/types.h>

int main(int argc, char **argv) {
  int64_t time;
  int16_t value;
  int32_t length;
  u_int8_t *array;

  FILE *in = fopen("java.dat", "rb");

  fread(&time, sizeof(time), 1, in);
  time = (int64_t)be64toh( (u_int64_t)time);

  fread(&value, sizeof(value), 1, in);
  value = (int16_t)be16toh( (u_int16_t)value );

  fread(&length, sizeof(length), 1, in);
  length = (int32_t)be32toh( (u_int32_t)length );

  array = (u_int8_t *)malloc(length);
  fread(array, sizeof(array[0]), length, in);

  fclose(in);

  printf("time: %ld\nvalue: %d\narray length: %d\narray:\n", time, value, length);
  for (int i = 0; i < length; i++) {
    printf("%c ", array[i]);
  }
  printf("\n");

  free(array);
  return 0;
}

I compiled this on Ubuntu x64 with clang. Its output was:

./a.out
time: 1380743479723
value: 32219
array length: 36
array:
a b c d e f g h i j k l m n o p q r s t u v w x y z 0 1 2 3 4 5 6 7 8 9

Keep in mind that the only unsigned types in Java are byte (8 bits) and char (16-32 bits).


Seeking the seeds for writing tidy event handlers in JS I'm seeking a critique of the simple program below, which displays two counters and increments them through a button, using D3.js. In real life of course no one has a need for displaying buttons that increment counters. It's a stand-in to talk about how to craft the code over the scaffold of a trivial problem.

I would like to remain within function invocations, and to generally fit in seamlessly with D3. In particular, I do not wish to use constructor invocation or prototypal patterns. More concretely, I would like to use neither class nor new.

I would also like to avoid constantly asking myself "what is this in this instant?" by initializing that, as you see in the code (let's leave arguing that this is sometimes entirely compelling for another occasion).

function Incrementer(name, value) {
    let that = {};

    that.init = function() {
        that.name = name;
        that.span = d3.select('body')
                      .append('span')
                      .attr('id', '#num' + name)
                      .text(value);
    }
    that.init();

    that.increment = function() {
        let i = parseInt(that.span.text()) + 1;
        that.span
            .text(i);
    }
    return that;
}
var incrementer1 = Incrementer('one', 10);
var incrementer2 = Incrementer('two', 20);

function double_increment() {
    incrementer1.increment();
    incrementer2.increment();
}

d3.select('#inc')
  .on('click', double_increment);

// d3.select('#inc')
//   .on('click', incrementer1.increment);
// d3.select('#inc')
//   .on('click', incrementer2.increment);
span { margin-left: 40px; }
<script src="https://cdnjs.cloudflare.com/ajax/libs/d3/5.7.0/d3.min.js"></script>
<button id="inc">Increment</button><br />

At this point I'm lacking a nice way to avoid the function double_increment. Of course without that function—were we to use the commented out pair of on-click handlers instead—we would have a bug. Only one on-click handler can be set. We have no mechanism for adding multiple addEventListeners.

So to summarize:

We define that.init as our initializer. That seems tidier than letting the initialization code loose in the body of the function. But defining it then immediately calling it seems odd. Can you improve on this?
How would you avoid the ugly double_increment function within a D3 program?

that.span.text() should probably be replaced by that.span.datum, if I can find the datum in there. But we'd like to avoid duplicating the counter within Incrementer. This way we do not need to worry about the counter and its view going out of sync. The datum itself, once located, is already a bit of a (necessary) duplication.

The simple way of avoiding needing to define a named function like double_increment is just to use a lambda. You could simply write

d3.select('#inc').on('click',
    () => {
        incrementer1.increment();
        incrementer2.increment();
    });

You could also get fancy and loop through the incrementers too

var incrementors = [incrementer1, incrementer2]

d3.select('#inc').on('click',
    () => {
        for (inc of incrementors) {
            inc.increment();
        }
     };

This lets you easily add new incrementors later if needed.