UVA 10474 - "Where is the marble?" time limit

Your solution is slow because it performs the sorting before every binary search! Just move the sorting to the line above the

printf("CASE# %d:\n",cont+1);

and you'll see the difference.

I also add my solution just to see what's possible to optimize (it uses custom int reader, without sort and the binary search):

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>

#define SIZE 16*1024
#define N    10001
static unsigned int values[N];
static int  buffer_size = 0;
static int  buffer_pos  = -1;
static char buffer[SIZE];

unsigned int readInt()
{
    unsigned int value  = 0;
    unsigned int digits = 0;
    char b;

    if (buffer_pos == -1)
    {
        buffer_size = fread(buffer, 1, SIZE, stdin);
        buffer_pos  = 0;
    }

    while(buffer_size > 0)
    {

        if (buffer_pos == buffer_size)
        {
            buffer_size = fread(buffer, 1, SIZE, stdin);
            buffer_pos = 0;
        }

        while(buffer_pos < buffer_size)
        {
                b = buffer[buffer_pos++];
                    if (b == '\n' || b == ' ')
            {
                return value;
            }
                    else
            {
                        value = value*10 + (b - '0');
                digits++;
            }
        }
    }
    return value;
}

int main(int argc, char **argv)
{
    long time = clock();

        char buffer[8192];
        setvbuf(stdout, buffer, _IOFBF, sizeof(buffer));
    unsigned int  n, q, num, max, min, c = 0;

    while(1)
    {
        c++;
        n = readInt();
        q = readInt();

        if(n==0 && q==0) break;

        max = 0;
        min = N;
        memset(values, 0, N * sizeof(unsigned int));

        for(unsigned int i=0; i<n; i++)
        {
            num = readInt();
            values[num]++;
            if(num > max) max = num;
            if(num < min) min = num;
        }

        unsigned int pos = 1, temp;
        for(unsigned int i=min; i <= max; i++)
        {
            if(values[i] !=0){
                temp = values[i];
                values[i] = pos;
                pos += temp;
            }

        }

        printf("CASE# %d:\n", c);
        unsigned qval;
        for(unsigned int i=0; i<q; i++)
        {
            qval = readInt();
            int ret = values[qval];
            if(ret == 0)
                printf("%d not found\n", qval);
            else
                printf("%d found at %d\n", qval, ret);
        }
    }
    //printf("Time (ms): %ld\n", clock()-time);
    return 0;
}


Intentional performance degradation through cache and branch-prediction misses

I don't think the test results will be convincing. Should I be in this situation, I would be interested in not just performance figures, but the miss counts as well. And I would need the latter to be controlled - random access patterns are not enough.

Also, I'd need to know more about cache: line size, ways and associativity, tag width all affect the test setup (e.g., what is the significance of 32767?) and reliability of results.

The guaranteed way to get worst case cache performance is to disable the cache completely. Configure it as writearound, and you'd have a miss on any write or read. To get more granularity (or if writearound is not available on your CPU), play with writethrough and readthrough modes.

I have no say on branch prediction, however I'd insist on more controllable test setup.

As for question 3: it depends on the underlying architecture. If it has virtual memory, page misses are extremely expensive.


I'd say calling self.setattr from __getattr__(self, ) is borderline abuse in itself.

I guess you are trying to implement lazy attributes, right?

However in your case valid attributes are constant and their values appear constant too, so why not set them at class "declaration" time?

Also, python syntax for a tuple with 1 element is (1,) not (1) that you have in valid attrs extensively.


Design Pattern for initializing Properties of a ViewModel which only serves a display purpose

More of a flow problem. Populating the property is necessary for the GET. No getting around that even if you were to create a function to do it for you. The POST however can be restructured to make only one call.

public class LoginController : Controller {
    private readonly ConmaniaSignInManager _signInManager;

    public LoginController(ConmaniaSignInManager signInManager) {
        _signInManager = signInManager;
    }

    private async Task populateLoginViewModel(LoginViewModel model) {
        model.AuthenticationSchemes = await _signInManager.GetExternalAuthenticationSchemesAsync();
    }

    [HttpGet]
    public async Task<IActionResult> Index(string returnUrl = null) {
        var model = new LoginViewModel {
            ReturnUrl = returnUrl
        };
        await populateLoginViewModel(model); //call 1
        return View(model);
    }

    [HttpPost]
    public async Task<IActionResult> Index(LoginViewModel model) {
        if (ModelState.IsValid) {
            var result = await _signInManager.PasswordSignInAsync(model.Email, model.Password, model.RememberMe, true);
            if (result.Succeeded) {
                return LocalRedirect(Url.GetLocalUrl(Url.GetLocalUrl(model.ReturnUrl)));
            }

            ModelState.AddModelError(string.Empty, "Invalid login attempt.");
        }
        //...if we get this far something was wrong. repopulate model and return
        await populateLoginViewModel(model); //call 2
        return View(model);
    }
}

If the model state is not valid then ModelState is already populated. If the login attempt was unsuccessful then we populate the model state and continue to the end of the action.


Reading, echoing, and returning integers in an array

Someone suggested I turn my comment into an answer, so here goes.

First, I simply ran your code through astyle and added line numbers to make it easier to
reference in the text. Hopefully you can see from the output how a little bit of white-space
and consistent formatting/indentation can make your code clearer.

 1  #include <stdio.h>
 2  #include <stdlib.h>
 3
 4  int *myFunction(int size)
 5  {
 6      int i;
 7      int *myArray = malloc(sizeof(int) * size);
 8
 9      for (i = 0; i < size; i++)
10      {
11          printf("Enter a number: ");
12          scanf("%d", &myArray[i]);
13      }
14
15      for (i = 0; i < size; i++)
16      {
17          printf("%d\n", myArray[i]);
18      }
19
20      return myArray;
21  }
22
23  int main()
24  {
25      int *p;
26      int size;
27      printf("Enter the size: ");
28      scanf("%d", &size);
29      p = myFunction(size);
30      return 0;
31  }

First, full marks for lines 23 and 30: many people still code as if main can be declared void main() and
fail to return an exit code. This would be just a little better for line 30.

return EXIT_SUCCESS;

As I said above, there is nothing wrong with your use of pointers (which was your primary concern). But there are
other issues with the code if this was ever to be used for anything other than exploring concepts.

Now, let's deal with dynamic memory.

MORTAL suggested zeroing all dyamically allocated memory. That can be useful (for example, if you are going
to build a c-style string in the buffer) but is not necessarily a "good thing" in all cases. However, we can
clean up line 7 a little and get that for free while we're at it. In this line, you are allocating memory to
hold an array of int - rather than just a chunk of memory.

There's a function just for that purpose, so this

int *myArray = calloc(size, sizeof(int))

very clearly expresses your need for enough memory to store size ints.

It also sets the memory block to all zeroes before returning it.

As ChrisWue mentioned, size could be a very large number such that the call to malloc or calloc could fail.
So it's a best practice to always check the returned pointer to see if it is NULL (that's how memory allocation
routines report failure). So now we have:

int *myArray = calloc(size, sizeof(int))

if (!myArray)
{
    fprintf(stderr, "Failed to allocate memory to hold your input. Exiting.\n");
    exit(EXIT_FAILURE);
}

Note that I simply bail out (indicating an error via the exit status) rather than try and deal with an out of memory
condition. In fact, there's an even chance that the fprintf won't work if your system is this low on resources
but it's worth trying to say something rather than just present the user with a commmand line prompt.

Having got over that hurdle, you should always return any dynamically allocated memory to the pool as soon as you
are finished with it. Most operating systems will release all resources when a process ends ... but what you are
looking at in your code is a classic "memory leak".

As the caller does nothing with the returned array, I would actually make a few changes.

Change the return type of myFunction to void
Replace line 20 with free(myArray); to release the allocated memory
Delete line 25, and modify the call on line 29 not to save the (now non-existent) return value

Also, as a general rule, int is the wrong type for indexing arrays. First, it is signed and
myArray[-1] makes no sense. Second, and int may not be large enough to index all of memory.
The correct type to use is size_t, and indeed that's what calloc and malloc expect.

So your variables size and i should be size_t, and line 28 should change to read

scanf("%zu", &size);

where "%zu" is the correct format code for a size_t.

Your code now looks like this:

 1  #include <stdio.h>
 2  #include <stdlib.h>
 3
 4  void myFunction(size_t size)
 5  {
 6      size_t i;
 7      int *myArray;
 8
 9      myArray  = calloc(size, sizeof(int));
10      if (!myArray)
11      {
12          fprintf(stderr, "Failed to allocate memory to hold your input. Exiting.\n");
13          exit(EXIT_FAILURE);
14      }
15
16      for (i = 0; i < size; i++)
17      {
18          printf("Enter a number: ");
19          scanf("%d", &myArray[i]);
20      }
21
22      for (i = 0; i < size; i++)
23      {
24          printf("%d\n", myArray[i]);
25      }
26
27      free(myArray);
28  }
29
30  int main()
31  {
32      size_t size;
33      printf("Enter the size: ");
34      scanf("%zu", &size);
35      printf("You need to enter %zu numbers\n", size);
36      myFunction(size);
37      return EXIT_SUCCESS;
38  }

Note that I added line 35 to give a little more feedback.

There is a bigger problem with your use of scanf that goes beyond the scope of this review. Just be
aware that scanf (like gets()) is considered EVIL. Just as a simple example, try these
responses to your "Enter the size:" prompt (leave out the quotes)

"a very long string"
"10 followed by some text"
"-1"


Except for some (mostly experience related issues) I commend your efforts. Well done!
The Short Version
Specific Points

use app\models\User; is not needed as the User class is not used (or mocked) in this test.
The comments before your asserts don't always seem to match what you are actually asserting.
I think you'd be better of using those comments as the test method name rather that having them as a comment in the function.
On the subject of names, I'd suggest using words like should and when to make the test method name state exactly what it expects the system under test to do. That way when a test fails you'll rarely have to look in the test to understand what is broken.
The first two test methods are basically testing the same thing. Whether or not the output of the create method matches that of the insertRow mock.
This could be simplified into one single test method fed by a dataProvider, more on this below.
You don't need to catch the result of the create in the methods that test for exceptions. Unused variables in code are bad and test code is still code.

Although it is impossible to tell without the class under test, you seem to have covered all logic paths:

Insert Succeeds
Insert Fails
Valid Arguments Provided (implicitly through previous methods)
Invalid Arguments Provided
DbAdapter Provided (implicitly)
DbAdapter Not Provided

You could consider making the implicit tests more explicit.
Incorporating these points your code would look a bit like this:
<?php

namespace app\models;

/**
 * Tests for the UserTable class
 *
 * @coversDefaultClass app\models\UserTable
 * @covers ::<!public>
 */
class UserTableTest extends PHPUnit_Framework_TestCase
{
    ////////////////////////////////// FIXTURES \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    protected $userTable;

    /**
     * create our instance of the class to be tested
     */
    final public function setUp()
    {
        $this->userTable = new UserTable;
    }

    /////////////////////////////////// TESTS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    /**
     * @covers ::create
     * @covers ::setDatabaseAdapter
     *
     * @dataprovider provideBooleans
     */
    final public function testUserTableShouldOnlyCreateUserWhenInsertRowSucceeds($insertRowStatus)
    {
        $userTable = $this->userTable;

        // set mock class
        $mockDbAdapter = $this->getMockAdapter();

        // set the mock object's methods for this test
        $mockDbAdapter->expects( $this->any() )
            ->method('insert')
            ->will( $this->returnValue($insertRowStatus) );

        $userTable->setDatabaseAdapter($mockDbAdapter);

        // perform test

        // assert user is created when valid argument is passed
        $actual = $userTable->create( array() );

        $this->assertEquals($insertRowStatus, $actual);
    }

    /**
     * @covers ::create
     * @covers ::setDatabaseAdapter
     *
     * @expectedException InvalidArgumentException
     *
     * @dataprovider provideInvalidArgumentTypes
     */
    final public function testUserTableShouldOnlyCreateUserWhenGivenValidArguments($invalidArgumentType)
    {
        $userTable = $this->userTable;
        // set mock class
        $mockDbAdapter = $this->getMockAdapter();

        // set the mock object's methods for this test
        $mockDbAdapter->expects( $this->any() )
            ->method('insert')
            ->will( $this->returnValue(false) );

        $userTable->setDatabaseAdapter($mockDbAdapter);

        // perform test

        // pass invalid argument, should be an array of values
        $userTable->create($invalidArgumentType);
    }

    /**
     * @covers ::create
     *
     * @expectedException RuntimeException
     */
    final public function testUserTableShouldOnlyCreateUserWhenGivenDatabaseAdapter()
    {
        $userTable = $this->userTable;
        $userTable->create( array() );
    }

    ////////////////////////////// MOCKS AND STUBS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    final public function getMockAdapter()
    {
        // create mock database adapter
        $mockDbAdapter = $this->getMockBuilder('framework\Db\DatabaseAdapter')
            ->disableOriginalConstructor()
            ->getMock()
        ;
    }

    /////////////////////////////// DATAPROVIDERS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
    final public function provideBooleans()
    {
        return array(
            'true' => array(true),
            'false' => array(false),
        );
    }

    final public function provideInvalidArgumentTypes()
    {
        return array(
            'string' => array('invalid argument type'),
            'boolean - false' => array(false),
            'boolean - true' => array(true),
            'null' => array(null),
        );
    }
}

The Long Version
Should you feel in the mood for a bit more depth and education, I have a more thorough review below.
These are all things I'd point out to my teammates, if this code had passed my desk at work.
This may be a bit much to take in, keep in mind I'm a rather critical developer and I always find room for improvement, even (or maybe especially) in my own code. Feel free to ask for clarity if any point is unclear.
Also, just ignore any points that don't resonate with you. There's no use in taking advice you don't believe in.
Points in General
Don't Start test functions with the word test
Use annotations to tell PHPUnit that this function is a @test. This allow the name of the test function to be more descriptive.

I've actually changed my mind on this one. By starting test method names with test they are both more explicit and recognizable. The same goes for starting provider methods with provides.
Don't create your mock in the setup function
Unless you need to insert the mock into the constructor of your class under test,
don't create your mock until you need it. I'd just create a getDatabaseAdapterMock
function and call that from the test functions. You could consider making specific methods for the various states the mock is in but (depending on the size and complexity of the class under test) that might be a step to far.
Add @covers annotations to your docblocks
Not only does this set you up to understand which parts of your code might still need attention, it also means you don't have to mention the name of the method you are testing in the name of function in your test class.
As of version 4, PHPUnit will tell you if "risky" code has run for a test. That is to say, code that either isn't mentioned by a @covers block or code that is mentioned but didn't run.
Another trick I tend to use is adding a @coversDefaultClass to the class-level docblock along with a @covers ::<!public> (see the @covers docs). That way you only need to specify public API in the test method doc blocks.
Don't use $this->any() when the count is important
A mock is also an assert. You want to be as specific with your assertions of (public) API as you can.
If it does not matter whether insert is called once or a hundred times than it's fine to use any.
In that case you should add a test proving that there is no difference and that no nasty side-effect occur.
If it does matter, for instance double rows might be inserted or existing rows might overwritten several times (causing your application to become slow), then I would strongly advocate using $this->exactly($count). Personally I also prefer using $this->exactly(1) over $this->once() for the sake of things looking the same regardsless of how often a method should be called.
Be specific with native PHP exceptions
Using @expectedException is a good start but, again, being specific really pays of when code breaks and tests start failing.
My advice would be to also add an @expectedExceptionMessage annotation to make sure you are getting the right exception.
If you want to keep things really clean you could put the message of Exceptions you throw yourself in class constants and just use the constant to check against like so (quoting straight from the manual here):
 /**
  * @expectedException        MyException
  * @expectedExceptionMessage MyClass::ERROR_MESSAGE
  */

Misc.
Use a namespace
Your classes live in a namespace, so should your test classes. Some people prefer to have an explicit Test namespace, others (me included) prefer to have the namespace match between the test class and class under test (as the class name already clearly states which one is the test...).
I'd say, pick one and see what you like best.
Avoid $this in the body of test functions.
To some it does not matter, other folks (like me) prefer a "setup" of sorts at the beginning.
That way there are no distracting signatures in the important code. The bit you'll be looking at most will (most likely) look more like it will in
"normal" code (that is to say, non-test code). You can see this in the example code posted above.
Use @dataproviders
If you run into a situation where you feel inclined to use a foreach loop in your test, or feed it just one single value, refactor the code to use a dataprovider instead.
This is especially useful to make a distinction in which datasets are "good" versus "bad".
Another benefit is that developers often only test "happy path", even for things like exceptions.
Instead of merely testing against one invalid argument, for instance, you could use a dataprovider and test against many.
The results may surprise you, especially when testing against scalar values or obtional object parameters when fed a NULL.
It also allows such sets of data to be re-used from several tests functions. (DRY FTW!)


There are a few things that you should refactor to be able to optimize your code better. Currently everthing sits inside the Click event handler and it's too much to handle there.

You are not able to focus on particular task like that. You should separate the operations and create new methods and properties for them. Here's how you could start. It's not 100% perfect (like the hard coded picture box BackColor) yet but it should give you an idea how to start refactoring your current solution.

Create a new panel control with a picture box inside (you can create a property for it so that you can set the image later). In this control you can implement all the logic that should handle the resize.

Give it a few properties that you can set and calculate the size and location values dynamiclly and internally. Use a flag (like IsEnlarged) to track whether a panel is in its normal or enlarged size rather than hard coded sizes. Here you can handle the Click event and take proper actions to modify the panel and the picture box.

class ResizablePanel : Panel
{
    private bool _isEnlarged;

    public ResizablePanel()
    {
        Controls.Add(new PictureBox
        {
            BackColor = Color.PaleGoldenrod
        });

        Click += PanelOrPicktureBox_Click;
        Controls[0].Click += PanelOrPicktureBox_Click;
    }

    public bool IsEnlarged
    {
        get { return _isEnlarged; }
        set
        {
            if (value == _isEnlarged)
            {
                return;
            }

            _isEnlarged = value;
            Size = _isEnlarged ? EnlargedSize : NormalSize;
            ResizePictureBox();
            TranslatePanel();

            if (IsEnlarged)
            {
                Enlarged?.Invoke(this, EventArgs.Empty);
            }
        }
    }

    public Size NormalSize { get; set; }

    public Size EnlargedSize { get; set; }

    public Size BorderSize { get; set; }

    public void Initialize()
    {
        Size = _isEnlarged ? EnlargedSize : NormalSize;
        ResizePictureBox();
        TranslatePanel();
    }

    private void ResizePictureBox()
    {
        var pictureBox = Controls[0] as PictureBox;

        pictureBox.Location = new Point
        {
            X = BorderSize.Width,
            Y = BorderSize.Height
        };

        // multiply by 2 becasue border size specifies only its width and we
        // need to two of them, one for each side
        pictureBox.Size = new Size
        {
            Width = Size.Width - (BorderSize.Width * 2),
            Height = Size.Height - (BorderSize.Height * 2)
        };
    }

    private void TranslatePanel()
    {
        if (IsEnlarged)
        {
            Location = new Point
            {
                X = Location.X - ((EnlargedSize.Width - NormalSize.Width) / 2),
                Y = Location.Y - ((EnlargedSize.Height - NormalSize.Height) / 2),
            };
        }
        else
        {
            Location = new Point
            {
                X = Location.X + ((EnlargedSize.Width - NormalSize.Width) / 2),
                Y = Location.Y + ((EnlargedSize.Height - NormalSize.Height) / 2),
            };
        }
    }

    private void PanelOrPicktureBox_Click(object sender, EventArgs e)
    {
        IsEnlarged = !IsEnlarged;
    }
}

You also can create your panels dinamiclly and give them some initial values.

private void CreatePictureBoxes()
{
    var pictureBoxCount = 4;

    var panelNormalSize = new Size(52, 52);
    var panelEnlargedSize = new Size(66, 66);
    var borderSize = new Size(5, 5);

    for (var i = 0; i < pictureBoxCount; i++)
    {
        var panel = new ResizablePanel
        {
            NormalSize = panelNormalSize,
            EnlargedSize = panelEnlargedSize,
            BorderSize = borderSize,
            IsEnlarged = false,
            // 3 is just a random border width multiplier to create gaps between the controls
            Location = new Point
            {
                X = panelNormalSize.Width * i + borderSize.Width * 3 * (i + 1),
                Y = borderSize.Height * 3
            },
            BackColor = Color.DarkMagenta,
        };
        panel.Initialize();

        Controls.Add(panel);
    }
}

Initialize the panels:

public Form1()
{
    InitializeComponent();
    CreatePictureBoxes();
}

And this is how it looks like:

  When one gets clicked, all other images turn back 'small'

Add an event to your new panel and raise it when it gets enlarged. You can then handle this event in the main form and set all other panels to IsEnlarged = false

public event EventHandler Enlarged;

public bool IsEnlarged
{
    get { return _isEnlarged; }
    set
    {
        // prevents multiple changes
        if (value == _isEnlarged)
        {
            return;
        }

        _isEnlarged = value;
        Size = _isEnlarged ? EnlargedSize : NormalSize;
        ResizePictureBox();
        TranslatePanel();

        // raise the event (using C# 6)
        if (IsEnlarged)
        {
            Enlarged?.Invoke(this, EventArgs.Empty);
        }
    }
}

Subscribe to the event during the initialization:

panel.Enlarged += Panel_Enlarged;

Handle the event:

private void Panel_Enlarged(object sender, EventArgs e)
{
    var otherPanels = Controls.OfType<ResizablePanel>().Where(rp => rp != sender && rp.IsEnlarged);
    foreach (var otherPanel in otherPanels)
    {
        otherPanel.IsEnlarged = false;
    }
}

  when a 'enlarged' image gets clicked, nothing happens

Just modify the PanelOrPictureBoxClick event handler like this:

IsEnlarged = true;


JSON Parser, pulls data from a JSON file for analysis

What a wall of text!  Also known as write-only code, or job security.

You want to write understandable code, that you can come back to in 6 months, and within 5 minutes understand enough to change it, if necessary.  This has a for-loop over a sorted list comprehension, with embedded if-then-else, of a zip of two sorted json queries, with a lambda thrown in to influence sorting.  This isn't functional programming; it's dysfunctional programming.

Let me begin with parsestats, because I can at least begin to understand it.

First, the name parsestats is itself a wall of characters.  How do I pronounce that word?  "par-sest-ats"?  Oh!  "parse stats"!  Write function names with snake_case, to aid the reader find the words in your compound name.  So this function should be called parse_stats.

In this function, you repeat hero['pickBan'] 8 times.  This is a lot of repetition.  It means the Python interpreter is looking up the same element in a dictionary repeatedly, which means the code is not going to win any speed contests.  Look up the value once, and reuse that value, instead of relooking it up over and over again.  This is awkward to do in a list comprehension, so lets use a helper function:

def parse_stats(herodata):

    def stats(hero_id, pick_ban):
        # ... TBD ...

    return [ stats(hero['heroId'], hero['pickBan']) for hero in herodata['heroes']]

A 14-line list comprehension just got turned into a 1 line list comprehension, and hero['pickBan'] got stored in the function argument pick_ban, for reuse multiple times.  I'm liking the direction this is going.

Now, what stats are being collected?  Looks like you've got 8 stats: 4 from this week, and the same 4 from the previous week.  Maybe we could compute those 4 stats in a function, passing in different selectors to grab the correct data?

def parse_stats(herodata):

    def stats(hero_id, pick_ban):

        def week_stats(pick, ban, pick_count, ban_count):

            return pick_ban[pick]['matchCount'] / pick_count * 1000,
                   pick_ban[ban]['matchCount'] / ban_count * 1000,
                   pick_ban[pick]['wins'] * 100,
                   pick_ban[ban]['wins'] * 100

        this_week = week_stats('pick', 'ban', this_week_pick, this_week_ban)
        last_week = week_stats('pickLastWeek', 'banLastWeek', last_week_pick, last_week_ban)

        return hero_id, *this_week, *last_week

    this_week_pick = herodata['matchPickCount']
    this_week_ban = herodata['matchBanCount']
    last_week_pick = herodata['matchLastWeekPickCount']
    last_week_ban = herodata['matchLastWeekBanCount']

    return [ stats(hero['heroId'], hero['pickBan']) for hero in herodata['heroes']]

Is that the clearest way?  Maybe we want to use a when selector, instead:

def parse_stats(herodata):

    def stats(hero_id, pick_ban):

        def stats_for(when):

            pick = pick_ban[f'pick{when}']
            ban = pick_ban[f'ban{when}']

            return pick['matchCount'] / herodata[f'match{when}PickCount'] * 1000,
                   ban['matchCount'] / herodata[f'match{when}BanCount'] * 1000,
                   pick['wins'] * 100,
                   ban['wins'] * 100

        this_week = stats_for('')
        last_week = stats_for('LastWeek')

        return hero_id, *this_week, *last_week

    return [ stats(hero['heroId'], hero['pickBan']) for hero in herodata['heroes']]

Which would make it easier to select 'LastMonth' stats, too.

Is a tuple really the best data structure for parse_stats to return?  The 0th element is the hero id.  What is the 2nd element?  Which element is last week's pick win percentage?  What is [1], or [3]?

How about using a namedtuple?

from collections import namedtuple

Stats = named tuple('Stats',
                    ('id',
                     'pick_rate', 'ban_rate', 'pick_wins', 'ban_wins',
                     'pick_rate_last_week', 'ban_rate_last_week', 'pick_wins_last_week', 'ban_wins_last_week'))

# ...

def parse_stats(herodata):
    def stats(hero_id, pick_ban):
        # ...
        return Stats(hero_id, *this_week, *last_week)

Or maybe make a tuple of tuples.

Stats = namedtuple('Stats', 'id, this_week, last_week')
WeekStats = namedtuple('WeekStats', 'pick_rate, ban_rate, pick_wins, ban_wins')

def parse_stats(herodata):
    def stats(hero_id, pick_ban):
        # ...
        return Stats(hero_id, WeekStats(*this_week), WeekStats(*last_week))

And you can refer to uni.id and imm.this_week.pick_rate instead of uni[0] and imm[1].

Ok, the elephant in the program:

print(f"{'Hero,':<20}{'Value':>7}")
for hero in sorted(ridiculous_list_comprehension, key=lambda tup: tup[1], reverse = True):
    print("{:<20}{:>7.2f}".format(inline_dictionary_definition[hero[0]], hero[1]))

First off, this should be a function, not code executed when Python parses the program text.  Use a if __name__ == '__main__': guard call the function if you want the code executed immediately upon parsing:

def print_mashup():
    print(f"{'Hero,':<20}{'Value':>7}")
    for hero in sorted(ridiculous_list_comprehension, key=lambda tup: tup[1], reverse = True):
        print("{:<20}{:>7.2f}".format(inline_dictionary_definition[hero[0]], hero[1]))

if __name__ == '__main__':
    print_mashup()

Of course, name the function according to what it really is doing.

for hero in sorted(...):

Uhm, no.  The output of sorted() isn't a list of hero objects.  You parsed all that hero data in parse_stats, and this is a much condensed summary of two values of those statistics, combined from two different data sets.  It is a list of two-value tuples.  Give those individual values names:

for hero_id, score in sorted(...):

I'm guess the "value" is some kind of score.  Maybe rating is a better name.  At any rate, this is way more descriptive than the hero[0] and hero[1] references made in the print statement of the body of the loop.

        print("{:<20}{:>7.2f}".format(inline_dictionary_definition[hero[0]], hero[1]))

Is that inline_dictionary_definition being constructed each and every iteration of the body of the for loop, so that one value can be extracted from the dictionary?  Be kind.  Create it once.

def print_mashup():
    print(f"{'Hero,':<20}{'Value':>7}")

    HERO_BY_ID = {1: 'Anti-Mage', 2: 'Axe', ... 129: 'Mars'}

    for hero_id, score in sorted(ridiculous_list_comprehension, key=lambda tup: tup[1], reverse = True):
        print(f"{HERO_BY_ID[hero_id]:<20}{score:>7.2f}")

As for the ridiculous_list_comprehension, I haven't a clue what "simple" is compared to "simple?rank=8", or what the calculated value is supposed to mean.  Or what uni and imm are supposed to be abbreviations for.  There are all sorts of unknowns.  Do the json queries return exactly the same hero_id sets?  Can there be an extra one in either of the sets, in which case sorting and zipping doesn't guarantee the data matches up properly!

I'm happy that you're happy to have written a program that you will personally use.  I'd be happier if someone else could understand it, and use it too.


Am I overlooking anything with this asynchronous read/write loop?

It's not entirely clear what your goals are with this bit of code.  Three possibilities that come to mind:

CopyStream should be asynchronous.  As written, it does not meet this goal since it blocks on the results of EndRead and EndWrite.  If this is your goal, you need to use callback methods and not call the End methods until your callbacks have been called.  You also need to modify CopyStream's signature to return IAsyncResult and take a callback and state as parameters.
CopyStream should hide the details of the underlying asynchronous calls from its users, but is itself synchronous.  Your code accomplishes this goal, but there are a few things that could use cleaning up.
CopyStream should hide the details of the underlying asynchronous calls, and also maximize performance by making sure the writer is kept busy at all times and never sitting around idle waiting for reads, to the extent possible.  It seems like this may have been your goal due to the attempt to read/write in parallel, but by only allowing a single outstanding read or write, you're not really getting much out of it.  In order to implement this correctly, you need to use a reader thread and a writer thread, with a queue of items to be written.  The reader reads as fast as it can and inserts chunks of data into the queue, which the writer then writes as fast as it can or as fast as they show up.

EDIT:

As an example of how to write this code using a queue, see the following.  Note that I didn't need to spin up a thread because I can just make use of the EndRead/EndWrite callbacks and have the original thread wait for a signal to be raised.  Also, this is a lightly-tested first draft, so beware that there could be bugs lurking in there.  This type of code is complex and difficult to get right, and should only be used when really necessary for valid perf reasons.  If the complexity is not justified, I would just use the synchronous Read and Write methods to simplify the code as much as possible.

public static void CopyStream(Stream input, Stream output)
{
    AutoResetEvent completed = new AutoResetEvent(false);
    BeginRead(new CopyStreamState(completed, input, output));

    completed.WaitOne();
}

private static void BeginRead(CopyStreamState state)
{
    const int bufferSize = 8192;

    state.InputBuffer = new byte[bufferSize];
    state.Input.BeginRead(state.InputBuffer, 0, bufferSize, ReadCallback, state);
}

private static void ReadCallback(IAsyncResult ar)
{
    CopyStreamState state = (CopyStreamState)ar.AsyncState;

    int bytesRead = state.Input.EndRead(ar);
    if (bytesRead > 0)
    {
        byte[] dataToWrite = state.InputBuffer;

        if (bytesRead < state.InputBuffer.Length)
        {
            dataToWrite = new byte[bytesRead];
            Array.Copy(state.InputBuffer, dataToWrite, bytesRead);
        }

        EnqueueWriteData(state, dataToWrite);
        BeginRead(state);
    }
    else
    {
        state.FinishedReading = true;
    }

    BeginWriteOrComplete(state);
}

private static void EnqueueWriteData(CopyStreamState state, byte[] data)
{
    lock (state)
    {
        state.WriteQueue.Enqueue(data);
    }
}

private static void BeginWriteOrComplete(CopyStreamState state)
{
    lock (state)
    {
        if (!state.WriteInProgress)
        {
            if (state.WriteQueue.Count > 0)
            {
                byte[] outputBuffer = state.WriteQueue.Dequeue();
                state.WriteInProgress = true;
                state.Output.BeginWrite(outputBuffer, 0, outputBuffer.Length, WriteCallback, state);
            }
            else if (state.FinishedReading)
            {
                state.Completed.Set();
            }
        }
    }
}

private static void WriteCallback(IAsyncResult ar)
{
    CopyStreamState state = (CopyStreamState)ar.AsyncState;
    state.Output.EndWrite(ar);

    lock (state)
    {
        state.WriteInProgress = false;
        BeginWriteOrComplete(state);
    }
}

private class CopyStreamState
{
    public CopyStreamState(
        AutoResetEvent completed,
        Stream input,
        Stream output)
    {
        this.Completed = completed;
        this.Input = input;
        this.Output = output;
        this.WriteQueue = new Queue<byte[]>();
    }

    public AutoResetEvent Completed { get; private set; }
    public Stream Input { get; private set; }
    public Stream Output { get; private set; }
    public Queue<byte[]> WriteQueue { get; private set; }

    public byte[] InputBuffer { get; set; }
    public bool FinishedReading { get; set; }
    public bool WriteInProgress { get; set; }
}


Daemonize a process under Microsoft Windows

It looks good: very good, assuming it works (i.e. has the run-time behaviour that you want: I don't know the effect of calling CreateProcess in that way).

You don't free(arguments) but that doesn't matter because memory will be reclaimed by the O/S when the process exits. Similarly, in theory I'd expect an API call to release message.

I don't see why you need the (LPSTR) cast. Using a cast can hide compiler warnings/errors, which should be fixed instead of their causing errors at run-time.

Instead of int idx; it's better to not define a local variable until the moment at which you initialize it: instead you could say, for (int idx = 1; idx < argc; idx++).

Also, shouldn't it be for (int idx = 2; idx < argc; idx++)? Because, idx=1 is the 'command' parameter, not the first 'argument' parameter.

If when you say "C/C++" you mean "C++", then it's more conventional these days to use std::string instead of C-style strings (but this is an unusually short program; perhaps although I don't know why you want to avoid the hassle of linking to the STL, especially if you know enough to avoid the many pitfalls of C-style string manipulation, which you seem to have).

For example, using std::string you could write the following, which is simpler than the string-manipulation code you have at the moment:

std::string s;
for (int idx = 1; idx < argc; idx++)
{
    s += "\"" + argv[idx] + "\"";
    if (idx < argc - 1) {
        s += " ";
    }
}
char* arguments = strdup(s.c_str());

The reason for strdup in the above is that MSDN says ...

  The Unicode version of this function, CreateProcessW, can modify the contents of this string. Therefore, this parameter cannot be a pointer to read-only memory (such as a const variable or a literal string). If this parameter is a constant string, the function may cause an access violation.

... which helps to explain why arguments must be char* not const char*. Sometimes you can ignore these (the windows API sometimes doesn't say its parameters are const when in fact they are), but doing so may be unsafe bad practice (so I use strdup above -- note that strdup'ed memory should be free'ed).


If it is possible that the strides match then you can add a if check to just copy it all in one go:

if(src_stride == dst_stride){
    memcpy(dst, src, count * dst_stride);
} else {
    while (count--) {
       memcpy(dst, src, dst_stride);
       src += src_stride;
       dst += dst_stride;
    }
}


Object-oriented RPG in Java, determining the data of a generated location

Explore is a verb, so your Explore class should be called Exploration, or probably something more descriptive like PlayerStatus. And you are defining state in Location and then just re-exposing it in Explore. Merge the two classes if you notice they are bleeding. You are right to be questioning that and noticing something isn't right. What is the point of Explore? Merge it into Location.


Some things that immediately hit me were not about the implementation of the algorithm but the look of the code, it's not idiomatic Ruby, it looks like Java.

Most Rubyists use 2 spaces instead of 4.
Why all the getters and setters? Setters are quite often a code smell, make sure you really need them (you may do but if you can draw a line under an object's period of mutability - hopefully by the end of initialization - you'll do yourself a favour).
Camel case is the Java-y bit, so no setPointer and friends. pointerData would be pointer_data in idiomatic Ruby.
You only need def pointer for the getter and def pointer= val for the setter, or to use the attr_… helpers.
… and you've used the attr_ helpers, so why is there even a setPointer at all? Just use previous_node.pointer = next_node (to give one example) and cut out the middle man. Same goes for all the others, why write a getter/setter to wrap another getter/setter?
self.node = nil in the intializer is unnecessary and if you were going to do it you'd probably use @node = nil.
self.node = node, again, why not access the instance variable directly if you know there's no extra processing going on? @node = node is more idiomatic.
No need to write return unless you want an early return, everything's an expression so the last expression in a method will become its return value.
pointer_data = current_node.pointer.nil? ? nil : current_node.pointer.data - why use a ternary where an && or || will do? pointer_data = current_node.pointer && current_node.pointer.data is terser.
To be even more up to date and terse you could use the new safe-navigation operator, &., e.g. pointer_data = current_node.pointer&.data
You're returning true and false from methods that are changing state. Return either self or the particular state changed (take a look at what happens when you do [] << 1 or [1,2,3].delete 2 in a REPL). Methods that return true or false are usually suffixed with ? and side effects will probably surprise the consumer.

Once you've cleaned up all of that then the algorithm will be clearer. Some more thoughts on the code that are stylistic but not about Ruby per se.

Firstly, I'd allow the head node to be set in the initialize method - why make a 1 line operation into 2? Secondly, I'd be clearer in my naming, node is a bit abstract, if you're really talking about the position of a node - the head, then call it head:

def initialize head=nil # <- this makes it optional
  @head = head # <- this takes care of setting to nil anyway

while ! current_node.pointer.nil? is the same as while current_node.pointer but the latter is much easier to understand. No one likes a double negative.

def initialize(data = nil, pointer = nil) Optional arguments are nice until you start using them in multiple positions, something like def initialize(data = nil, pointer: nil) is better, or def initialize(data: nil, pointer: nil), it depends on what you think is most likely to be used / easier on the consumer of the library - think about the interface all the time.

This brings me to my last point - where are the examples for running it? Where are the tests? I've pasted my own version using the advice I've given but does it work? I don't know. Any refactoring can (will!) break things which is why you need examples/tests to fall back on and check your work. It's also where you I'd advise you should start in future. If you don't know how you want the code to be called then you won't do a good job writing it.

I hope that helps.

module LinkedList

  class List
    attr_accessor :node

    def initialize head=nil
      @head = head
    end

    def add(node)
      if @head.nil?
        @head = node
      else
        current_node = @head
        while current_node.pointer do
          current_node = current_node.pointer
        end
        current_node.pointer = node
      end
      self
    end

    def get(node)
      current_node = @head
      data_match = nil
      while current_node.pointer and data_match do
        data_match = current_node.data if node.data == current_node.data
        current_node = current_node.pointer
      end
      current_node
    end

    def remove(node)
      previous_node = nil
      current_node = @head
      next_node = current_node.pointer
      while current_node do
        if current_node.data == node.data and current_node.data == @head.data
          @head = next_node
          return node
        end
        if current_node.data == node.data
          # This looks like it will fail as previous_node is nil afaics
          previous_node.pointer = next_node
          return node
        end
        previous_node = current_node
        current_node = next_node
        next_node = current_node.pointer
      end
      nil
    end

    def print
      current_node = @head
      while current_node do
        pointer_data = current_node.pointer&.data
        puts "data=#{current_node.data}, pointer=#{pointer_data}"
        current_node = current_node.pointer
      end
      puts # puts *what*?
    end

  end

  class Node
    attr_accessor :data
    attr_accessor :pointer

    def initialize(data: nil, pointer: nil)
      @data = data
      @pointer = pointer
    end
  end

end


Web application for recording blog comments

That's easy: You're repeating quite a few things, so don't do it.

disp.forward(req, resp);

This is the easiest, copied in each of the 4 branches, but needn't be. Drop the needless return and move it after the branches.

But in general, deduplication is hard and it's much better to learn to avoid duplications. Just imagine Ctrl-C and Ctrl-V are broken and you must type using one finger only. :D You could write a method accepting statusMessage and handling the problem. But it's simple enough so you can do it inline.

The basic idea is "compute the statusMessage, leave it empty in case there's no problem and handle it":

if (comment.isEmpty()) {
    statusMessage = name.isEmpty()
        ? "Name and comment fields empty"
        : "Comment field empty";
} else {
    statusMessage = name.isEmpty()
        ? "Name field empty"
        : "";
}

if (statusMessage.isEmpty()) {
    CommentBean commentBean = new CommentBean();
    commentBean.setComment(comment);
    commentBean.setName(name);
    req.setAttribute("comment", commentBean);
    disp = req.getRequestDispatcher("acknowledgement.jsp");
} else {
    disp = req.getRequestDispatcher("error.jsp");
}

disp.forward(req, resp);


This is a poor way to test primality:

bool isPrime(int n) {
    for (int i = 2; i < n/2; ++i) {
        if (n % i == 0)
            return false;
    }
    return true;
}

Even sticking with trial division, we can stop at √n, which is generally much less than n/2.  It's also well worth treating 2 as a special case, to reduce the iterations by half (i.e. i += 2 instead of ++i).

Why are we using signed integers, given that we don't need any negative values?

You say the "output is tested", but don't show any of the test code.  It's useful to see how the test works, to identify tests that are missing or erroneously passing.  I mention this because it's notoriously difficult to create reliable tests for multithreaded code.

In PrimeNumberMultiThread::solve, we have opportunity to reduce duplication, and we don't need to hard-code the number of threads to two.  Instead, we can use as many threads as we have cores available, using a for loop to start them and another for loop to join them all.

The mutex m and the commented-out lock_guard should just be removed: these aren't necessary, because only one thread accesses each vector between the worker threads starting and finishing (which points are memory barriers).

Instead of result.insert(), the general case might benefit from std::move algorithm - in any case, it's well worth a reserve before adding the results.

With OpenMP, much of the work can be automated for you - you'll just need a custom reduction to combine vectors - I think that's available from OpenMP 4.0 onwards.

An alternative to using vectors is to use an array of flags to indicate which values are to be included in the result (the flags could even be in the input vector: overwrite the non-primes with zero to discard them).  Then there's no reallocation within either thread, and you can use OpenMP of older versions because we no longer need a custom reduction.

Here's one version that uses the OpenMP approach I've just described:

#include <algorithm>
#include <iostream>
#include <iterator>
#include <random>
#include <vector>

bool isPrime(unsigned n) {
        if (n < 2) return false;
        if (n < 4) return true;
        if (n%2 == 0) return false;
        for (auto i = 3ul;  i*i < n;  i += 2) {
            if (n % i == 0)
                return false;
        }

        return true;
}

std::vector<unsigned> solve(std::vector<int>& source)
{
    auto const len = source.size();
    std::size_t result_size = 0;

#pragma omp parallel for reduction(+:result_size)
    for (std::size_t i = 0;  i < len;  ++i) {
        if (isPrime(source[i])) {
            ++result_size;
        } else {
            source[i] = 0;
        }
    }

    std::vector<unsigned> result;
    result.reserve(result_size);

    std::copy_if(source.begin(), source.end(),
                 std::back_inserter(result),
                 [](unsigned i){ return i; });

    return result;
}

int main(){
    const std::size_t size = 1000000u;
    const unsigned min = 0u;
    const unsigned max = 5000000u;

    std::vector<int> randoms;
    randoms.reserve(size);

    {
        std::uniform_int_distribution<std::mt19937::result_type> dist(min, max);
        std::mt19937 rng;
        rng.seed(0);            // for a reproducible test
        for (std::size_t i = 0;  i < size;  ++i) {
            randoms.push_back(dist(rng));
        }
    }

    auto primes = solve(randoms);
    std::clog << "Found " << primes.size() << " primes "
              << " from " << randoms.size() << " inputs\n";
}

Compile with g++ -fopenmp, or the equivalent for your compiler.

To test a single-threaded version, we just have to remove (manually, or with #ifdef) the #pragma omp line.

Note that since C++17, we're able to specify an execution policy for std::copy_if, so we could reduce that code even further, especially if we're not fussy about the order of the results:

#include <execution>

std::vector<unsigned> solve(std::vector<int>& source)
{
    std::vector<unsigned> result;

    std::copy_if(std::execution::par_unseq
                 source.begin(), source.end(),
                 std::back_inserter(result),
                 [](unsigned i){ return isPrime(i); });

    return result;
}

I haven't been able to test this, as I'm still working with an old standard library that doesn't have <execution>.


Are there ways to speed up this string fuzzy matching in Golang?

Review I - package go-fuzzywuzzy

After a quick read of your Go code, the Go code for package go-fuzzywuzzy, and the Go documentation, it is reasonable to hope for at least a 60% to 95% improvement in Go performance.

For example,

$ go build fuzzy.go && time ./fuzzy
real    0m55.183s
user    0m58.858s
sys     0m0.944s
$

After moving one line in package go-fuzzywuzzy,

$ go build fuzzy.go && time ./fuzzy
real    0m6.321s
user    0m7.211s
sys     0m0.188s
$

The line to move:

r := regexp.MustCompile("[^\\p{L}\\p{N}]")

A diff of package go-fuzzywuzzy:

diff --git a/stringutility.go b/stringutility.go
index 935f6e1..95441cc 100644
--- a/stringutility.go
+++ b/stringutility.go
@@ -5,6 +5,8 @@ import (
        "strings"
 )

+var rxCleanse = regexp.MustCompile("[^\\p{L}\\p{N}]")
+
 func Cleanse(s string, forceAscii bool) string {
        s = strings.TrimSpace(s)
        s = strings.ToLower(s)
@@ -12,8 +14,7 @@ func Cleanse(s string, forceAscii bool) string {
                s = ASCIIOnly(s)
        }

-       r := regexp.MustCompile("[^\\p{L}\\p{N}]")
-       s = r.ReplaceAllString(s, " ")
+       s = rxCleanse.ReplaceAllString(s, " ")
        return s
 }

  Package regexp

 import "regexp"

  func MustCompile

func MustCompile(str string) *Regexp

  MustCompile is like Compile but panics if the expression cannot be
  parsed. It simplifies safe initialization of global variables holding
  compiled regular expressions.

The documentation clearly says global variables.

If I spent more time reading the code, I would expect further performance improvements.

After more reading, package go-fuzzywuzzy is written in "PyGo." Writing the package in Go yields efficiencies.

For example, completely rewriting the go-fuzzywuzzy file stringutility.go in Go.

Before:

$ go build fuzzy.go && time ./fuzzy
real    0m55.183s
user    0m58.858s
sys     0m0.944s
$

After:

$ go build fuzzy.go && time ./fuzzy
real    0m5.735s
user    0m6.601s
sys     0m0.193s
$

stringutility.go:

package fuzzy

import (
    "strings"
    "unicode"
)

func Cleanse(s string, forceAscii bool) string {
    if forceAscii {
        s = ASCIIOnly(s)
    }
    s = strings.TrimSpace(s)
    rs := make([]rune, 0, len(s))
    for _, r := range s {
        if !unicode.IsLetter(r) && !unicode.IsNumber(r) {
            r = ' '
        }
        rs = append(rs, r)
    }
    return strings.ToLower(string(rs))
}

func ASCIIOnly(s string) string {
    b := make([]byte, 0, len(s))
    for _, r := range s {
        if r <= unicode.MaxASCII {
            b = append(b, byte(r))
        }
    }
    return string(b)
}

UPDATE:

The package go-fuzzywuzzy author adopted the suggested stringutility.go changes:

[Fixes #4] Stop reinitializing costly regex in stringutility
https://github.com/paul-mannino/go-fuzzywuzzy/commit/f14294bf5858c8a7fa51b026a9ee9a2802c816bf

Was using regexp.MustCompile within a frequently invoked
method Cleanse. Since go does not cache these calls, it was
incurring a costly regex compilation each time Cleanse was
called. The other changes made in the stackoverflow post that
caught this issue seem to further improve performance by
about 10% on top of the massive gains from fixing this issue,
so incorporating those as well.


Chrono-like clock with different epoch (from 1.1. 2000)

Frankly, there isn't much to say about this class since it is simple enough and relies on another simple clock. The type alias, static members and static methods seem to all rely just like they should on the template parameter Clock so there is no problem. I believe that firda::time_point is nothing really from std::chrono::time_point os this shouldn't be a problem either.

The only remarks I have is about the template parameters: I am not sure whether defaulting Clock to std::chrono::system_clock makes sense. Is it really the default clock amongst the three standard clocks? If you think so, why. But frankly, defaulting Epoch has no sense: adapted_clock<> doesn't tell anything about the epoch to the user and you already provide the meaningful type alias y2k_clock (note that using the defaults is always equivalent to using y2k_clock, which makes them useless). You can't default Clock without defaulting Epoch though because you can't change the order of the template parameters.

I think that the best solution would be to leave undefaulted template parameters in adapted_clock but make y2l_clock an alias template with a possibly default Clock template parameter:

template<typename Clock=std::chrono::system_clock>
using y2k_clock = adapted_clock<Clock, 946681200>;

If you don't like this alias template solution, I would still recommend not to default the template parameters of adapted_clock but instead to write this:

using y2k_clock = adapted_clock<std::chrono::system_clock, 946681200>;


Effectively calculate the result of geometric series

Instead of using a recursive algorithm (which might overflow the stack for large exponents) and calculating the power yourself, use the pow function which is likely pretty optimized.

#include <iostream>
#include <cmath>

using namespace std;

double func(double x, int n)
{
    double x_n;

    x_n = pow(x, n+1);
    return (1-x_n)/(1-x);
}

int main(int argc, const char * argv[])
{

    double result = func(2, 2);
    cout<<result<<endl;
    return 0;
}

Now a few minor remarks about your original code: be sure to be more consistent with your coding style. For example:

if(exponent == 0)
    return 1;
if (exponent == 1)
    return base;

vs.

if (x == 0) return 1;
if (x == 1) return n+1;

Personally, I'd always either write it on one line as in the second example or if there is a newline, I'd always add braces. For reasons see Apple's goto fail;. But it's actually a personal preference, you don't need to do it that way but I highly recommend that you chose one style and stick to it.

(Same with whitespace: there should be a space after the if in if(exponent == 0))

Consistency would also be nice here:

if ((exponent & 0x1) == 1)

Either use 0x1 both times or 1 both times. I feel that mixing these isn't a good idea.


Few suggestions:

Add/use inheritance for packet type (assuming you have something like game::networking::packets::PacketTypeBase that all packet types inherit from. Have GetDescriptor()/GetTypeName()/handle() on this base class
Create a factory to go from int id to packet Type. Something like: game::networking::packets::PacketTypeBase make_packet(int id);
Add a virtual function handleInput(char* data, int len) so your NetHandler::handle simply becomes return make_packet(id).handleInput(data,len). The default can be handled by base class or separate UnknownPacket child class.

I would recommend using inheritance on packet type, not NetHandler. If you need flexibility, you can even pass a functor into the ctor of NetHandler if you want different work done on the PacketType by each instance of NetHandler

In general try to replace switch statement with inheritance and virtual functions. Factory/maker function comes in handy even during testing. Common functionality can be put in base class so you can avoid duplicate code (not need macros) & still do packet specific work.

Instead of having a hard-coded list of 256 (avoid magic numbers), use a std::map / std::unordered_map/HashMap so find / lookup will be faster in getId (use a data structure that  optimizes your most common case)

Hope this helps.


Python code to encrypt and email PDF file using PyPDF2

Your code is easy enough to follow.  All of your open() calls are done using with blocks, which is a good habit.

Your variables — such as REP and EmailDict — are unconventionally named.  Use ALL_CAPS for constants, and lower_case for variables.  Ironically, the one variable that should be a constant, path, is defined several times, and not written as a constant.

Speaking of path, you are misusing os.path.join().  The whole point of the function is to ensure that the path delimiter is automatically inserted for you between the components.  So, you should write os.path.join(path, FileDict[ID]) instead.  Also, use raw strings to make your backslashes less cumbersome.

I doubt that there is any value in writing all of those encrypted versions of the files to disk.  You could just create a transient encrypted copy in memory, mail it off, and move on to the next recipient.  To accomplish that, you can write to a StringIO object instead of a file.

The code to encrypt the PDF and compose the mail message feels like it could belong logically together.  I'd define a function for that:

def message_for(unencrypted_pdf_path, rep_id, email, password):
    """
    Creates a MIME message for a rep containing an encrypted version
    of the specified PDF.
    """
    buf = StringIO()
    …
    encrypted_pdf = buf.getvalue()

    text = MIMEText(… % rep_id)

    pdf_attachment = MIMEApplication(encrypted_pdf, _subtype='pdf')
    pdf_attachment.add_header('content-disposition', 'attachment', filename=('MonthlyPaidCommission_%s.pdf' % rep_id))

    msg = MIMEMultipart(_subparts=[text, pdf_attachment])
    msg['SUBJECT'] = 'Commission Report'
    msg['FROM'] = 'me'
    msg['TO'] = rep['EMAIL']
    return msg

Your code would be more expressive if you defined the dictionary entries "all at once" using a dict comprehension.  I would also avoid storing the e-mail and password as a tuple, since it makes you write more cryptic code like EmailDict[ID][1] later.

I think that you would be better off not defining FileDict at all.  Just process each directory entry as you encounter it.  You can even send multiple messages within the same SMTP connection.  Just make sure you keep a log of what was sent, in case something goes wrong.

PATH = r'C:\Apps\CorVu\DATA\Reports\Monthly Commission Reports\Output\pdcom1'

COLS = ['rep_id', 'email', 'password']
with open('commissionrepemaillist.csv', 'r') as infile:
    email_dict = {
        row[0]: dict(zip(COLS, row)) for row in csv.reader(infile)
    }

smtp = smtplib.SMTP('localhost:587')
smtp.starttls()
smtp.login(…)

for filename in os.listdir(PATH):
    rep_id = re.split('[_.]', filename)[1]
    rep = email_dict.get(rep_id)
    if rep is None:
        continue
    msg = message_for(os.path.join(PATH, filename), *rep)
    smtp.sendmail('me', rep['email'], msg.as_string())
    print("Sent %s to %s" % (filename, rep['email']))

smtp.quit()


Fields are required, except if all fields are empty

You have two situations where input is invalid:

fieldD is empty and one or more are empty
fieldD is not empty and one ore more are empty

This makes that you have to check for different situations and so your if-statement cannot really be shortened to one statement.

What you also can do is loop over the fields and do a count of the empty fields and validate this:

$emptyFields = 0;

foreach(array($fieldA, $fieldB, $fieldC, $fieldD)as $field) {
    if ($field === '') {
        $emptyFields++;
    }
}

if ($emptyFields === 4 || $emptyFields === 0 || ($emptyFields === 1 && $fieldD === '')) {
    //input is VALID
}


Convert decimal to binary and vice-versa in Java

The best way to do it:

static String convertDecimalToBinary(String decimal) {
    return Integer.toString(Integer.parseInt(decimal, 10), 2);
}

static int convertBinaryToDecimal(String binary) {
    return Integer.parseInt(binary, 2);
}

By the way your code looks good except for at least two issues:

Don't work with negative numbers
The method isDecimalOrBinary is ambiguous: what's the result of the 100 input? It's binary of course, but if i meant one hundred? I suggest to delegate the user to specify it.

I would correct it in this way:

import java.util.Scanner;

public class BinaryToDecimalAndBackConverter {

    static String convertDecimalToBinary(String decimal) {
        final boolean isNegative = decimal.startsWith("-");
        if (isNegative) {
            decimal = decimal.substring(1);
        }

        int integer = Integer.valueOf(decimal);
        String result = new String();

        while (integer > 0) {
            result += String.valueOf(integer % 2);
            integer /= 2;
        }

        // use StringBuilder instead of StringBuffer, we are sure about
        // exclusive access, no need to use a thread safe class
        final StringBuilder resultBuilder = new StringBuilder(result);
        if (isNegative) {
            resultBuilder.append("-");
        }

        return resultBuilder.reverse().toString();
    }

    static int convertBinaryToDecimal(String binary) {
        final boolean isNegative = binary.startsWith("-");
        if (isNegative) {
            binary = binary.substring(1);
        }
        int result = 0;

        for (int reverseCounter = 0; reverseCounter < binary.length(); reverseCounter++) {
            final char currentChar = binary.charAt(binary.length() - reverseCounter - 1);
            final int numericValue = Character.getNumericValue(currentChar);
            result += Math.pow(2, reverseCounter) * numericValue;
            // System.out.println(Math.pow(2, reverseCounter) *
            // Character.getNumericValue(binary.charAt(binary.length() -
            // reverseCounter - 1)) + " reverseCounter " + reverseCounter);
        }

        if (isNegative) {
            result = -result;
        }

        return result;
    }

    public static void main(String[] args) {
        final Scanner inputScanner = new Scanner(System.in);

        System.out.print("Please enter the binary/decimal number to convert:");
        // expected input: NUM D|B
        final String[] splittedInput = inputScanner.nextLine().split(" ");
        if (splittedInput.length != 2) {
            // don't forget to close the Scanner
            inputScanner.close();
            throw new IllegalArgumentException("Wrong input format: expected NUM D|B");
        }
        final String input = splittedInput[0];

        switch (splittedInput[1]) {
        case "d":
        case "D":
            System.out.println(convertDecimalToBinary(input));
            break;
        case "b":
        case "B":
            System.out.println(convertBinaryToDecimal(input));
            break;
        default:
            // don't forget to close the Scanner
            inputScanner.close();
            throw new IllegalArgumentException(
                    "Wrong input format: expected NUM D|B, the second argument was not D nor B");
        }

        // don't forget to close the Scanner
        inputScanner.close();
    }
}


Efficiently squaring each element in a sorted array, keeping it sorted

It can be done in O(n) time. Split your array into two logical parts. Positive and negative. Then apply square to each element in both arrays. Then merge the arrays( merging can be done in O(n) ), but merge the array with previously negative integers in reverse order, since its values will be reversed.


grouping values in a list of tuples to a dictionary

I can't think of a builtin that solves your problem, but there is an even simpler solution than the one you provided. Taking Gareth Rees' comment into consideration, I would solve your problem like this:

input_list = [(1,2),(1,4),(3,5),(5,7)]
output_dict = {}

for key, value in input_list:
    #Initialize empty list if key doesn't exist
    if key not in output_dict:
        output_dict[key] = []
    # Now we know that the key always exists
    output_dict[key].append(value)

print(output_dict)
# If you want single items to be integers instead of lists
for key, value in output_dict.items():
    if len(value) == 1:
        output_dict[key] = value[0]

print(output_dict)

Output:

{1: [2, 4], 3: [5], 5: [7]}
{1: [2, 4], 3: 5, 5: 7}

The first for loop creates a dictionary mapping ints to lists. The advantages of this structure compared to the solution you provided is that there are no type-checks required when parsing the data. Checking if a value is a duplicate is a simple if len(value) > 1.

Usually, I'd prefer this implementation in any application that is not extremely dependent on performance. If your application requires a lot of performance, I'd suggest:

Not using Python
Not using dicts

If this is the case, I could elaborate on more ways of improving performance.

In the more likely scenario that a coherent data structure is more important, I'd reduce your code to the first for-loop above, and wrapping it in a function:

def list_to_dict(input_list):
    output_dict = {}
    for key, value in input_list:
        if key not in output_dict:
            output_dict[key] = []
        output_dict[key].append(value)
    return output_dict


Select Records Constituting Percentage of Total

This is very good question and a very hard one because your query is basically sensitive to the ordering. In other SQL database engines, this is easier to solve with use of window functions. Unfortunately, Access database engine does not support window functions.

A possible choice is to use subquery but this can get very expensive quickly because a subquery would have to be correlated and therefore re-calculated for all possible rows, making it O(too much n). For completeness' sake, here's a possible query:

SELECT
  m.Item,
  m.Quantity,
  (
    SELECT SUM(x.Quantity)
    FROM myTable AS x
    WHERE x.Quantity >= m.Quantity
  ) AS RunningSum,
  (
    SELECT SUM(t.Quantity)
    FROM myTable AS t
  ) * 0.8 AS PercentileThreshold
FROM myTable AS m
WHERE (
  SELECT SUM(x.Quantity)
  FROM myTable AS x
  WHERE x.Quantity >= m.Quantity
) <= (
  SELECT SUM(t.Quantity)
  FROM myTable AS t
) * 0.8
ORDER BY m.Quantity DESC;

To avoid paying the expenses of multiple queries being executed several times over, you can combine some VBA with SQL. For instance, you can create the following VBA in a public standard module:

Private Type TPercentile
    PercentileThreshold As Currency
    PercentileValues As Scripting.Dictionary
End Type
Private This As TPercentile

Public Function StartPercentile(PercentileThreshold As Currency) As Boolean
    This.PercentileThreshold = Nz(DSum("Quantity", "myTable") * PercentileThreshold, 0)
    Set This.PercentileValues = New Scripting.Dictionary
    StartPercentile = True
End Function

Public Function Percentile(Key As String, Quantity As Variant) As Currency
    Dim PercentileValue As Currency

    If Not This.PercentileValues.Exists(Key) Then
        PercentileValue = Nz(DSum("Quantity", "myTable", "Quantity>=" & Nz(Quantity, 0)))
        This.PercentileValues.Add Key, PercentileValue
    End If
    Percentile = This.PercentileValues(Key)
End Function

Public Function PercentileThreshold() As Currency
    PercentileThreshold = This.PercentileThreshold
End Function

Your query then becomes:

SELECT
  m.Item,
  m.Quantity,
  Percentile(m.Item,m.Quantity) AS Percentile,
  PercentileThreshold() AS Threshold
FROM myTable AS m
WHERE StartPercentile(0.8)
  AND Percentile([m].[Item],[m].[Quantity]) <= PercentileThreshold()
ORDER BY m.Quantity DESC;

This enables you to handle a larger dataset by populating the dictionary only once and require less code to boot and you can easily generalize for other tables. The downside is that this is limited to only one active running query at a time. If this is unacceptable, then you need to add some kind of identifier that each query can then generate in their invocation to StartPercentile() function so that they have their own "instances".

I also want to call your attention to one possible option for you - If you are reporting, Access already provides you with a Running Sum property on the Report so you could conceivably create a simple query, then apply a filter on a calculated control, rather than doing in the query. That yields much less code.

You may wonder why I use Currency all over the place -- the answer is that I generally avoid using floating numbers such as Double which can cause unstable results due to rounding errors. Currency is a scaled integer and thus is not subject to such problems so it is a great data type to use for where you need fractions less than 4 decimal places. I should note that there is also a Decimal type but IME, it yields more problems than solutions and I never use it within Access/VBA context.


Write log file in nodeJS with setInterval

I'm assuming that you are changing the logText variable to control what's being logged. If you want to keep this same interface, the only optimization that I can see is to make log() check if logText is not empty before it writes it:

var fs = require('fs');
var logText = '';
function writelog(){
    if (logText) {
        fs.appendFile('c:/log.txt', logText, (err) => {
            if (err) throw err;
            logText = '';
        });
    }
}
setInterval(writelog, 5000);

I would also add a function that adds to the log text:

function log(text){
    logtext += ("Log prefix stuff here" + text)
}


Detecting two keys pressed in quick succession

Highlights

Using the libc crate for C data types
No heap allocation of read struct
No (new) panics — better error handling
Event iterator instead of vectors
New types and traits to collect related functionality

Beware - I still haven't gotten this to actually read any events on my machine, but it does compile. That should be good enough, right? ^_^

libc crate

The libc crate should always be used when interoperating with C code. This crate is guaranteed to have the correct sizes for platform-specific types.

Additionally, it already has a definition for timeval, so that was nice.

Less heap allocation

I used mem::uninitialized to create an on-stack version of the struct with no values. I then got a reference to that area as bytes and read directly to that.

Note that I marked that function as unsafe - depending on what T resolves to, it could produce very bad results.

Error handling

Everywhere that had a panic! or assert! was changed into a Result. This allows the caller to decide how to deal with the error. The most awkward place left is in the call to read_c_struct, as it just stops the iterator on failure. Instead, it could return the Result directly and allow its caller to see the failure.

Embrace the Iterator

Rust loves iterators. This seemed like a perfect case for it - you have a stream of events and you want to do something based on them. I reworked your method into an Iterator implementation. This allows the caller to decide how much memory to allocate; in the final code, I believe it's very small. It also allows using the filter method and other iterator adaptors.

New types and traits

Iterators need a type to exist, and I also felt that your hashmap and threshold needed to live together. I also moved the read-specific methods into a new trait that is then implemented for all types that implement Read.

Complete code

There's a few comments scattered about for smaller things, but here it is:

extern crate libc;

use std::{mem, env, process, io, slice};
use std::io::{Read, Write};
use std::fs::File;
use std::collections::HashMap;

// Some helper methods for reading things
trait ReadExtensions : Read {
    /// Completely fill the supplied buffer
    fn read_complete(&mut self, buf: &mut [u8]) -> io::Result<()> {
        let mut total_read = 0;
        while total_read < buf.len() {
            match self.read(&mut buf[total_read..]) {
                // Unable to read anything
                Ok(0) => return Err(io::Error::new(io::ErrorKind::Other, "Unable to read")),
                // Read some amount
                Ok(n) => total_read += n,
                // Might have been interrupted by something
                Err(ref e) if e.kind() == io::ErrorKind::Interrupted => {},
                Err(e) => return Err(From::from(e))
            }
        }
        Ok(())
    }

    /// Directly read a C struct.
    ///
    /// Unsafe because only the concrete type of `T` can determine if
    /// the result is safe. Should only be used to read structs
    /// composed of primitive types without any pointers.
    unsafe fn read_c_struct<T>(&mut self) -> io::Result<T> {
        // avoid allocating on the heap
        let mut val = mem::uninitialized();
        let ptr = &mut val as *mut T as *mut u8;
        let mut buf = slice::from_raw_parts_mut(ptr, mem::size_of::<T>());

        try!(self.read_complete(buf));
        Ok(val)
    }
}

impl<R> ReadExtensions for R where R: io::Read {}

const EV_KEY: u16 = 0x01;

fn to_microseconds(tv: libc::timeval) -> u64 {
    // u64 suffix unneeded, adding _ for readability
    tv.tv_sec as u64 * 1_000_000 + tv.tv_usec as u64
}

// Using libc
#[derive(Copy,Clone)]
#[repr(C)]
struct LinuxInputEvent {
    time: libc::timeval,
    type_: libc::uint16_t,
    code: libc::uint16_t,
    value: libc::int32_t,
}

/// An iterator of Linux events
struct InputSource {
    file: File,
}

impl InputSource {
    fn new(path: &str) -> io::Result<InputSource> {
        let f = try!(File::open(path));
        Ok(InputSource { file: f })
    }
}

impl Iterator for InputSource {
    type Item = LinuxInputEvent;

    fn next(&mut self) -> Option<LinuxInputEvent> {
        unsafe { self.file.read_c_struct() }.ok()
    }
}

/// Tracks when multiple presses of the same key occurs and calls the
/// callback when duplicate events are triggered within a threshold.
struct KeyEventSink {
    last_occurences: HashMap<u16, u64>,
    threshold: u64,
}

impl KeyEventSink {
    fn new(threshold: u64) -> KeyEventSink {
        KeyEventSink {
            last_occurences: HashMap::new(),
            threshold: threshold,
        }
    }

    // Take a closure so we don't have to worry about user interface here
    fn process_event<F>(&mut self, event: LinuxInputEvent, f: F)
        where F: FnOnce(u16, u64)
    {
        let key = event.code;
        let micros = to_microseconds(event.time);

        // if-let for one useful match arm
        if let Some(last_micros) = self.last_occurences.get(&key) {
            let delta = micros - last_micros;
            if delta < self.threshold {
                f(key, delta)
            }
        }
        self.last_occurences.insert(key, micros);
    }
}

fn print_usage_and_exit(program_name: &str) {
    writeln!(&mut io::stderr(), "Usage: {} <microseconds-delta>", program_name).unwrap();
    process::exit(1);
}

fn main() {
    let args: Vec<_> = env::args().collect();
    if args.len() != 2 { print_usage_and_exit(&args[0]); }
    let threshold: u64 = args[1].parse().unwrap_or(0);
    if threshold == 0 { print_usage_and_exit(&args[0]); }

    // just use value of match
    let input = match InputSource::new("/dev/input/event2") {
        Ok(f) => f,
        Err(err) => panic!("Unable to open file: {}", err),
    };

    let mut sink = KeyEventSink::new(threshold);

    let key_events = input.filter(|&event| event.type_ == EV_KEY && event.value == 1);
    for key_event in key_events {
        sink.process_event(key_event, |key, delta| {
            println!("Key code {:3}: event delta: {} μs", key, delta);
        })
    }
}

Followups

  is there any particular reason you added the Copy and Clone traits to LinuxInputEvent?

Basically, LinuxInputEvent is a simple type comprised of Copyable values, and it's also fairly small in the grand scheme of things. Making it Copy simply improves the ergonomics of using it. If you implement Copy, you need to implement Clone.

  Seeing as the function is used at most once, FnOnce would also be better than Fn

Absolutely correct - I updated to use this.

I also updated the read_complete method a bit based on feedback from this Stack Overflow question.


Extract rotation values from css string Javascript

First: A little regexp will do ya just as well, I'd imagine. Something like:

function extractRotationValues(string) {
  var values = {x: 0, y: 0, z: 0};
  string.replace(/\brotate([XYZ])\s*\(\s*([+-]?[\d.]+)\s*deg\s*\)/g, function (_, axis, value) {
    values[axis.toLowerCase()] = parseFloat(value);
  });
  return values;
}

Note that this does a couple of things differently:

parses the string values into Number values
returns an object with x, y, and z properties rather than an array (just because I feel that that's more useful - it can be made to return an array, though)

Basically it looks for rotate[name of axis]([a number]deg) (while skipping over possible whitespace - the \s* stuff - that might be in the string) and captures the axis name and number. Despite the use of replace nothing's actually replaced (or rather we don't care about replacing things). It's just that String.replace takes a function as its 2nd argument, which gets passed the matched/captured substrings, which sort of turns replace into a "scan"-like function.

Anyway, that's what I'd do. Not saying it's bulletproof (spent a minute writing it, and robustly matching a number can be more complicated), but it's certainly less code.

Your code works too, of course, though the lower-level approach of finding indices and slicing and dicing strings is less elegant in my eyes. But it's a valid approach nonetheless.

There are however a few things I want to comment on:

I'd encourage you to always use strict equals (=== and !==). Just because it's a good habit to get into.
closingIndexes is a nice descriptive name by itself, but it gets confusing when you also have rindexes where you've abbreviated, and just plain indexes (which isn't actually used). I'd give all of them equally descriptive names, instead of having one that's descriptive, one that's slightly cryptic, and one that's generic. (I also tend to favor indices for the plural form, but "indexes" is correct English too.)
Why are default zero values inserted into the array in a different way than extracted/found values? The found values you just insert with push, which makes a lot of sense. But when you want to insert 0 for x, y, or z, you use unshift or splice? Why not use push in all cases, as that's the simplest?
I'd rather have a branch that determines the value to insert, and a common push(value) call after that, than branch the array-insertion itself, because in both cases you want to append some value. The branching should be about what to insert, because how and where to insert it is always the same.
Incidentally, if you find a value, you insert it as a string. If you don't find one, you insert zero as a number. Don't mix types like that; keep it consistent. If you'd used null or undefined instead of zero to denote "no value", that'd be fine, but here you're essentially "making up" a rotation that's not present in the CSS string, so it's not "no value", it's "fallback value". And thus it should be the same type as a non-fallback value and vice-versa.
Your code can stumble on malformed CSS. E.g. rotateX(42deg rotateY(23deg) (note the missing closing parenthesis) won't be parsed right. It'll find 42deg rotateY as the value for x-axis rotation, which you might say is better than nothing. However, the CSS was malformed, so technically speaking there is no x-axis rotation; nothing would actually be better.
Most importantly: Why write 3 separate if-else blocks when all that changes in them is an index? That's what loops are for. Write a regular for loop, or call forEach on rindexes, e.g.:

rindexes.forEach(function (rindex, i) {
  var value = 0;
  if(rindex !== -1) {
    value = code.substr(rindex, closingIndexes[i]).split("(")[1].split("deg)")[0];
  }
  values.push(value);
});


Convert an `int` to a C string with size limitation

Use some constants:

This char buf[sizeof x * CHAR_BIT * 10 / 33 + 3];

Would be a lot better if written like this: char buf[MAX_CONV_CHARS]; (or any other name your might prefer for the constant).

Also, do you really care about being that exact with the size of buf? I usually just throw some large number that can't overflow, like 128.

Return early:

Instead of nesting the entire function under an if, return early:

char *int_strnull(int x, char *dest, size_t dest_size) {

    if ((dest == NULL) || (dest_size < MIN_SIZE)) {
        return NULL;
    }

Mind your naming:

xna is an abbreviation for what exactly? Not obvious to me. x is also very vague. Call it input, number or even value.

int_strnull() also seems odd. Why not just int_to_string()?

Local buffer actually needed?

Finally, I don't really get the necessity of the local buffer buf. Why not simply write to dest directly. That would also save a memcpy at the end. I guess you did that to first figure out the total length of the string. But since it is such a small string, I would suggest just requiring that the output buffer be at least the maximum size.


r.Post("/", func(w http.ResponseWriter, r *http.Request) {
    handlePost(w, r)
})

can be simplified to

r.Post("/", handlePost)

since both functions have same signature.

fmt.Fprintf(w, "dummy response will go here")

No need to use printf family functions where you don't need formatting. You can write directly with

w.Write([]byte("dummy response will go here"))

or with

io.WriteString(w, "dummy response will go here")

responsebody, _ := ioutil.ReadAll(resp.Body)
fmt.Println("backend response Body:", string(responsebody))

This one reads the whole response into memory for no reason. Use io.Copy to write it more efficiently:

fmt.Print("backend response Body: ")
_, err := io.Copy(os.Stdout, resp.Body)

  I'm not worried about error handling yet.

Then everything else is totally fine.


Using dynamic methods to build SQL queries in CodeIgniter

In accordance with PSR-1, your method names should be in camelCase. To avoid function name conflicts (native count() with custom count()), it may be best to prefix the method names' functionality with easy or simple, ci, or ar for Active Record.

"Embrace the curly brace."  Always use curly braces to encapsulate your language construct and function bodies -- even if they only have one line of code inside them.  Not using { and } may lead to developer confusion or unintended/incorrect code logic.

For versatility, I recommend passing an array of arrays to your Crud methods.  This will allow you to separate column names from values, enable you to turn off CI's automatic quoting (protect_identifiers flag) when necessary (3rd element in a respective subarray), and effortlessly pass multiple where conditions when required.  There is a multitude of reasons for this utility -- here is just one: https://stackoverflow.com/q/2489453/2943403.

Granted, your Read() method _could_be as simple as passing the table name and an associative array of conditions to get_where(), but that approach doesn't permit the flexibility of turning off the auto-quoting.

Make the default $condition parameter value an empty array.  I don't like the meaningless WHERE 1 in any project.

Use a foreach loop to only add a WHERE clause when there is actually something useful to write inside it.

public function ciRead($tableName, $conditions = []) {
    foreach ($conditions as $condition) {
        $this->db->where($condition);
    }
    $this->db->get($tableName)->result();
}

insert() should have the table name as the first parameter.

public function ciCreate($tableName, $data) {
    return $this->db->insert($tableName, $data) ? $this->db->insert_id() : false;
}

For update() loop the conditions like in read()  I recommend returning the affected rows so that there is a truer representation of success. I often treat the affected rows value as a boolean or explicitly cast it with (bool) if I only need a truthy/falsey value and I want to make the coding intention very clear.

public function ciUpdate($tableName, $data, $conditions = []) {
    foreach ($conditions as $condition) {
        $this->db->where($condition);
    }
    return $this->db->update($tableName, $data) ? $this->db->affected_rows() : false;
}

Same thinking with delete() regarding a more accurate return value...

public function ciDelete($tableName, $conditions = []) {
    foreach ($conditions as $condition) {
        $this->db->where($condition);
    }
    return $this->db->delete($tableName) ? $this->db->affected_rows() : false;
}

When counting, CI has count_all_results().

public function ciCount($tableName, $conditions = []) {
    foreach ($conditions as $condition) {
        $this->db->where($condition);
    }
    return $this->db->count_all_results($tableName);
}

In your controller (assuming all of the routing is set up properly), you should be collecting the slash-delimited input values as parameters of the method call instead of writing out all of those segment() calls.

Using $_GET data is perfectly fine when calling your Read() or Count() methods, but it is not recommended when you are writing to the database.  For Insert(), Update(), and Delete(), you need to be collecting user-supplied data via $_POST data only.

Be sure to validate and sanitize the incoming data in your controller before allowing access to your model methods.

Don't give your users any more access/privilege than absolutely necessary.  I wouldn't be letting people know what my table or column names are called.   I wouldn't want to offer them the flexibility to nominate the table/column that they want to access -- purely on the grounds of security.

Perhaps provide some kind of alias/whitelist to convert the user data to model entity identifiers (translate the table/column names with in the model) or ideally make all of the table/column names static/hardcoded (provided by you, the developer).

public function fetch($tableName, $conditionId) {
    $result = $this->Crud->ciRead($tableName, [['id', $conditionId]]));
    ...
}

or when writing to the db use post().

public function changeStatus() {
    $tableName = $this->input->post('tableName');  // I don't like it
    $status = $this->input->post('status');
    $conditionId $this->input->post('conditionId');
    if ($this->Crud->ciUpdate($tableName, ['is_active' => $status], [['id', $conditionId]])) {
       $this->session->set_flashdata('success', "Success! Changes saved");
    } else {
       $this->session->set_flashdata('danger', "Something went wrong");
    }
    redirect('admin/seller/' . basename($_SERVER['HTTP_REFERER']));
}


You home brewed security hashes and such are a big NO. Please, check here and here for a good read regarding that. Also, please do not hash a hash, that can lead to collisions and should be avoided! Using a method such as password_hash() creates a salt for you, therefore you shouldn't have to make one on your own.

You're using mysql_real_escape_string() which is not the way to go. If possible, move away from that and onto mysqli or PDO.

And then your function htent() is sort of redundant. You don't have anything else in the function, so it shouldn't be needed.


Very slow Project Euler Q3 (largest prime factor of a large number)

Similar questions have been asked before.  You are using the "test the largest candidate factors first" strategy, which does not scale well at all.  You should test the smallest candidate factors first.


Using an offset variable to keep track of the next item in a Rust array

Comparing your variants

Your first variant is a C-style variant. In C, we only have one return value, not multiple, so pointers are the only way to pass more than a single piece of information to the user. Unfortunately, those pointers sometimes get used in a wrong way, which isn't that easy with Rust. Still the following code is somewhat repetitive, isn't it?

let first_u16 = get_next_u16(&input, &mut offset);
let second_u16 = get_next_u16(&input, &mut offset);
let third_u16 = get_next_u16(&input, &mut offset);
let fourth_u16 = get_next_u16(&input, &mut offset);
....
let one_hundred_u16 = get_next_u16(&input, &mut offset);

input and offset are tightly coupled, which shows that there is something amiss. A structured way to handle both at the same time would be great. Or we could search some closure somewhere else.

The latter variant clearly shows the user that there is more information to work with: The usize shouldn't get ignored! After all, it's part of the output. But alas, that's still error prone, since we now can accidentally use the same offset twice:

let (offset2,  first_u16) = get_next_u16(&input, offset);
let (_      , second_u16) = get_next_u16(&input, offset);     // whoops!

That's due to the fact that usize implements the Copy trait.

Iterate with iterators

But let's take a step back: you want a safe way to get the next u16 repeatedly. You want an iterator. Slices already provide a fitting iterator called chunks. Therefore, we can get two u8s easily. All we need to do is to map those chunks:

fn to_u16(input: &[u8]) -> u16 {
    ((input[1] as u16) << 8) + (input[0] as u16)
}

fn main() {
    let input = [1, 2, 3, 4];
    let output: Vec<_> = input.chunks(2).map(to_u16).collect();
    println!("{:?}", output);
}

Note that this will panic if the input doesn't contain an even number of u8s. You can use the unstable chunks_exact feature or return an Option<u16> instead.

Other remarks

Use rustfmt to format your code. That way your code looks similar to the one written by others.


Short analogue of Riffle[] accepting two blocks

There's not too much to say about your implementation, though I'm not sure declaring riffle as a lambda provides any benefit instead of just defining it as a standard function.

Instead of storing the previous element of the iteration and using it to call the block, you should check only call the block if you are not on the last element of the enumerable (if possible, of course, there are some cases when it isn't).

While you can use a mixture of a custom enumerator and a block, it's not the nicest way to do things. I'd recommend instead passing in two lambdas (or a lambda and a block) so that it is immediately clear that the first block follows the second.

def riffle(enumerable, a, b)
  enumerable.each_with_index  do |x, i|
    a.call x
    b.call x unless i + 1 == enumerable.size
  end
end

riffle [1, 2, 3],
  -> i { p [i, :IN] },
  -> i { p [i, :OUT] }

This gives the output:

[1, :IN]
[1, :OUT]
[2, :IN]
[2, :OUT]
[3, :IN]
=> [1, 2, 3]


Using $q in Angular JS to pass navigator geo JSON from factory to controller

You can use resolve feature of ui-router (https://github.com/angular-ui/ui-router/wiki) or ngRoute.
Promise will be resolved and injected into controller automatically, service and controller will be nicely decoupled.
Using ui-router code would look like this:

app.controller('NearestLocationController', function($scope, location) {
}

$stateProvider.state('locationState', {
      resolve: {
         location: function(geolocation){
            return geolocation.getLocation();
         }
      },
      controller: 'NearestLocationController'
})


Order a list of points by closest distance

I don't like any of these answers.  In particular, the accepted answer is simply wrong.

Let's start by critiquing the interface:

List<Point> OrderByDistance(List<Point> pointList)

The contract is: the list must contain at least one element, the list is destroyed (!!!) by the method, the first element is special, and the result is a mutable list.  I like nothing about any of this; this sounds like a ball of potential bugs.  The right contract is:

ImmutableList<Point> OrderByDistance(Point start, ImmutableSet<Point> points)

Look at how many problems this solves. Do we need the set to contain at least one point?  No. We already have the start point. Do we destroy the set of points?  No. It's immutable.  And so on.

Now that we have the signature correct, the algorithm is simple:

ImmutableList<Point> OrderByDistance(Point start, ImmutableSet<Point> points)
{
  var current = start;
  var remaining = points;
  var path = ImmutableList<Point>.Empty.Add(start);
  while(!remaining.IsEmpty)
  {
    var next = Closest(current, remaining);
    path = path.Add(next);
    remaining = remaining.Remove(next);
    current = next;
  }
  return path;
}

Now all you have to do is efficiently implement Closest, which you should be able to do given the previous hints about computing distances cheaply and so on.

If you want a more sophisticated algorithm for Closest, then you need to do some research on this well-studied problem:

https://en.wikipedia.org/wiki/Nearest_neighbor_search


First: Upgrade PHP!!! PHP 5.6 is losing security support on December 31st this year. Ref. All other PHP 5 versions are completely unsupported. You should be running PHP 7 today.

Once you've done that, you can use PHP 7's new null coalescing operator ?? to clean up the loop.

foreach ($elements as $element) {
    $results[] = sprintf('(%d,%d)', $element['x'] ?? 0, $element['y'] ?? 0);
}
unset($element);

Note that I've added unset($element) after the loop. You should always do this to avoid confusing behavior when using the same name in multiple loops. Take a look at the warning on PHP's documentation page for more info. You should also unset $element_x, and $element_y.

The PSR-2 style guide requires braces on control statements, style is a personal(/team) choice, but most people do agree with this rule:

  The body of each structure MUST be enclosed by braces. This standardizes how the structures look, and reduces the likelihood of introducing errors as new lines get added to the body. - PSR-2 Coding Style Guide

Other than this, I believe your code is about as clean as you can get. Good work! I'd probably go for $x instead of $element_x, but there's nothing wrong with your choice.