Adding multiple events in gcalcli using a bash script #!/bin/bash

#assigning variables
start="14:00"
duration="150"
title="Exper. Lab"
year="2016"
start2="17:30"
duration2="230"
title2="Calc1"

# Here I refer to dates that the event occurs on piping the variables into gcalcli (Google calendar command line interface) creating a repeated event on my Google calender

for monthday in {09/{1..29..7},09/{6..27..7},10/{4..25..7},10/{6..27..7},11/{1..29..7},11{3..24..7},12/{6..20..7},12/{1..15..7}}
do
(echo $title ; echo "'$monthday/$year $start'"; echo $duration) | ../../../../usr/bin/gcalcli add --calendar 'Class'

done

# Same thing different repeated event

for monthday2 in {09/{12..26..7},10/{3..31..7},11/{7..28..7},12/{5..19..7}}
do
(echo $title2 ; echo "'$monthday2/$year $start2'"; echo $duration2) | ../../../../usr/bin/gcalcli add --calendar 'Class'

done

exit 0

Is there a way I could do this in one loop? Or should I be using another method?

Your main questions

  Is there a way I could do this in one loop?

Yes.
The two loops do essentially the same thing,
but use different values.
The loop should be inside a function,
the range of values and the event details should be parameters of the function.

  Or should I be using another method?

As it is, the dates of the event are very hard to read.
It would be better to have a function that takes start date, end date, and other event details, and use a loop from start date stepping forward one week until the end date is reached.

Furthermore, a "recurring event" is usually not the same event registered n times, but a single event with some parameters. I don't know the API of Google Calendar, but I suggest to check on that, as there might be a much easier method, natural and intuitive method to register a proper recurring event.

Code review

Avoid pointless comments like this one that only state the obvious:

#assigning variables
start="14:00"
duration="150"
title="Exper. Lab"
year="2016"
start2="17:30"
duration2="230"
title2="Calc1"

And there's really no need to define start2, duration2 and title2 at the top of the file.
It's unnecessary information at that point.
You could define these right before the second loop.
You could even reuse the start, duration, title variables.
If you did that, the two loop bodies would look identical,
and converting to a function would be a trivial step.

Be extremely cautious with long lines like this:

for monthday in {09/{1..29..7},09/{6..27..7},10/{4..25..7},10/{6..27..7},11/{1..29..7},11{3..24..7},12/{6..20..7},12/{1..15..7}}

Can you spot the bug?

   Should have been 11/{3..24..7} instead of 11{3..24..7}

A long relative path like this is awful:

... | ../../../../usr/bin/gcalcli add --calendar 'Class'

What if you later need to change the directory of this script or gcalcli?
You will have to recalculate the correct depth,
and update the script at multiple places (the two loops).

It's better to add ../../../../usr/bin to PATH, and use simply gcalcli. Alternatively, you could define a function like this:

gcalcli() {
    ../../../../usr/bin/gcalcli "$@"
}

Actually, since I don't have gcalcli on my PC,
I used this function as a dummy in my tests:

gcalcli() {
    cat
}

Code is easiest to read from top to bottom,
with minimal distractions sideways.
This line is too long,
and I'm forced to scroll to the right to see it:

(echo $title ; echo "'$monthday/$year $start'"; echo $duration) | ../../../../usr/bin/gcalcli add --calendar 'Class'

Avoid multiple statements on one line, break it up like this:

(
echo $title
echo "'$monthday/$year $start'"
echo $duration
) | gcalcli add --calendar 'Class'

Alternative implementation

Consider the alternative implementation below, using some of the suggestions above.

#!/bin/bash

PATH="path_to_gcalcli:$PATH"

add_to_calendar() {
    title=$1
    datetime=$2
    minutes=$3

    (
    echo $title
    echo "'$datetime'"
    echo $minutes
    ) | gcalcli add --calendar 'Class'
}

add_weekly_event() {
    title=$1
    start_time=$2
    minutes=$3
    start_date=$4
    end_date=$5

    event_date=$start_date
    while [[ "$event_date" < "$end_date" ]]; do
        datetime="$(date +%m/%d/%Y -d $event_date) $start_time"
        add_to_calendar "$title" "$datetime" $minutes
        event_date=$(date +%Y/%m/%d -d "$event_date + 1 week")
    done
}

add_weekly_event "Exper. Lab" 14:00 150 2016/09/01 2016/12/21
add_weekly_event "Exper. Lab" 14:00 150 2016/09/06 2016/12/21
add_weekly_event "Calc1" 17:30 230 2016/09/12 2016/12/20

This script is much slower than yours, due to the many calls to date.
But it's a lot easier to understand what dates will be added,
and it's trivially easy to reuse it for other events.

Note that the add_weekly_event function takes the date parameters in %Y/%m/%d format, where month and day are 0-padded. This is to make the loop condition possible, as the alphabetic ordering of dates in this format coincides with the correct chronological ordering.

Also note that before passing the date to add_to_calendar,
it is formatted to (roughly the same) format in your script.
I don't know the Google Calendar API,
but I suspect this is unnecessary,
the %Y/%m/%d format probably works just fine.

For the date parsing and + 1 week calculation,
this script depends on GNU date.
This is the default on Linux,
on other OS you can usually find it in a coreutils package,
named gdate.
To use gdate instead of date,
you can create a function:

date() {
    gdate "$@"
}

But this technique is not my first recommendation.
My first recommendation is to check the Google Calendar API to create proper recurring events, instead of manually looping over dates and adding the same event multiple times.


Converting a 1D Array of Strings to a 2D Array in Python I was solving this question on a site that gives you a 1d array called grid

grid = ['top left',    'top middle',    'top right',
        'middle left', 'center',        'middle right',
        'bottom left', 'bottom middle', 'bottom right']

and you're expected to write a method fire(x,y) that takes in two coordinates and gives you back where you hit the grid.

For example:

fire(0,0) # 'top left'
fire(1,2) # 'bottom middle'

Here is my solution, I used NumPy for it.

import numpy as np
def fire(x,y):
#the grid is preloaded as you can see in description

    oneDArray = np.array(grid)
    twoDArray = oneDArray.reshape(3,3)

    return twoDArray[y][x]

I'm looking to get some feedback on the answer.

A few issues:

Probably better to follow pep8
This isn't a method, it is a function
When doing 2D numpy indexes, you can do twoDArray[x, y]
You don't really need a numpy array, a list of lists works just as well.  Or better yet just just translate the 2D index into a linear index mathematically.  This also avoids the substantial overhead of converting the list to a numpy array.  You could even use np.ravel_multi_index to do the 2D to 1D conversion for you.


Find DivTag element in list and click when innerHTML matches account name I need to click on a "account" button from a table of buttons. If this "account" button is not present, then I will need to click a different "UseAnotherAccount"
 button.

How I do it at the moment is:

Set a flag account_found to be false by default
use a foreach loop to search through the whole _list of buttons
If the expected account button is found, click it, set account_found flag to be true and break away from this foreach loop
an additional if block is used to to click on UseAnotherAccount button if account_found flag is not set.

My code is shown below:

bool account_found = false;
foreach (DivTag element in _list) {
    String account_name = element.GetInnerHtml();
    if (account_name.Equals(_account_name)) {
        element.Click();
        account_found = true;
        break;
    }
}
if (!account_found) {
    _my_repo.SignInToYourAccount.UseAnotherAccount.Click();
}

I have this feeling my way is not the most elegant way to achieve this. Any suggestions?

To eliminate the need for the flag you could do it like this:

var element = _my_repo.SignInToYourAccount.UseAnotherAccount;
foreach (var item in _list)
{
    if (item.GetInnerHtml().Equals(_account_name))
    {
        element = item;
        break;
    }
}
element.Click();

However, your code can be greatly simplified with the Linq extension methods provided in the System.Linqnamespace:

var element = _list.FirstOrDefault(x => x.GetInnerHtml().Equals(_account_name)) ?? _my_repo.SignInToYourAccount.UseAnotherAccount;
element.Click();


Improving the code to turn a string into an array of numbers? I have a string of numbers, like "31654918562314", and I want to convert it into an array of integers. The code I've written is:

string source = "31654918562314";
int[] numbers = new int[source.Length];
for (int i = 0; i < source.Length; i++)
{
    numbers[i] = Convert.ToInt32(source.Substring(i, 1));
}

What improvements can I apply on this code? Is it the best ultimate code I can get to?

Note: I'm using C#. However, I also should implement this code in JavaScript.

I'm partial to using LINQ here.  Jon has a point there on the conversion.

var str = "31654918562314";
var numbers = str.Select(c => c - '0').ToArray();


Read hex from file and convert to decimal I made a small script to read text file that contain hexadecimal and convert to another file in decimal by using command-line as a filename input.

#this is content from hex.txt
0x5B03FA01
0x42018360
0x9FF943B3

and this is what I expected my python to deliver output

#this is content from hex_1.txt
1526987265
1107395424
2683913139

After 3 hour of trial and error. I can see a result in the way I want. The code is follow;

#this was copied from Learn Python the Hard way exercise.
from sys import argv
from os.path import exists

#this is the main function to convert hexadecimal to decimal
def hex2dec (hex):
    result_dec = int(hex, 0)
    return result_dec

#this to accept parameter from command-line
script, from_file = argv

#this to make sure the output generate as a new file with _1
to_file = from_file[:-4] + "_1.txt"
out_file = open(to_file, 'w').close()

#this took me a few hour to figure out a bug. Thanks to stackoverflow
with open(from_file) as in_file:
    lines = in_file.read().splitlines()

for i in lines:
    converted = str(hex2dec (i))
    out_file = open(to_file, 'a+')
    out_file.write(converted + "\n")
    out_file.close()

    #this to print the result for reference on-screen.
    print "converted =" + str(converted)

print "done."

Even though this is done but I feel there are plenty area I could improve to this script e.g. bugs handling in hex2dec function.

However, may I ask you what would you do to enhance this further? and if you could suggest, could anyone suggest me which topic or area I can study further to improve this code or any other about Python?

Python 3

The script currently does not work in Python 3. For example, print "converted =" + str(converted) is invalid syntax in Python 3, use print() instead (as a function).

Comments

It's great that you document the code. Some comments however don't add any value for future reading: #this took me a few hour to figure out a bug. Thanks to stackoverflow. I have no idea what the bug is, and what the answer from StackOverflow was.

Try to use docstrings as well. For instance in the hex2dec function:

def hex2dec(hex):
    """Convert a hexadecimal string to a decimal number"""
    result_dec = int(hex, 0)
    return result_dec

Some other things about the above function:

Rather than hex2dec, perhaps use hex_to_decimal. In this case it's not too big of a deal though.
You're storing the result in a variable and then returning the variable without doing anything else to it. This means you could also return result immediately: return int(hex, 0)
Since it's using a built in function without any other operation, is the function necessary?

Running it!

As said above, using Python 3 failed to run it. Changing the print statements to functions fixed this issue. Python 2 also supports the print() function, so it should have no issue running on Python 2 as well.

The program ran as expected with a correct file, and produced an output file with the converted values.

Running it with an invalid file, or no arguments at all, crashes the program with either a FileNotFoundError or ValueError respectively. If a file was given which exists but has an invalid hexadecimal value in it (for instance a random string), the program also crashes:

Traceback (most recent call last):
  File "hex2dec.py", line 22, in <module>
    converted = str(hex2dec (i))
  File "hex2dec.py", line 7, in hex2dec
    result_dec = int(hex, 0)
ValueError: invalid literal for int() with base 0: 'random word'

The above errors tells us we need some error checking.

The first error to solve would be the ValueError of not giving a filename as argument when running the program. This can be solved by using a CLI module such as argparse, click, or perhaps another one you like. I personally use argparse, since it's pretty simple and it's part of the standard library.

The arguments:

import argparse

def get_cli_parser():
    parser = argparse.ArgumentParser(description='Describe what your program does')
    parser.add_argument('input_file', type=str,
                        help='The file to read the hexadecimal values from'
    return parser

if __name__ == '__main__':
    parser = get_cli_parser()
    args = vars(parser.parse_args())

At this point you can access the command line arguments args as a dictionary.

The if __name__ == '__main__': part of the code is only applicable if it's run as a script. This means that if the file is imported as a module, this will not run. See also this answer for more info.

When we run the above code without any arguments, we get the following:

usage: hex2dec2.py [-h] input_file
hex2dec2.py: error: the following arguments are required: input_file

Next up is the FileNotFoundError. This one is quite easy to solve: before opening the file, check if it exists using the already imported os module:

if not os.path.exists(args['input_file']):
    print('Input file does not exist, please try again')
    return

The last ValueError can be solved in a number of ways. One would be to use regex on the current line of the file to find any hexadecimal values. Assuming only the hexadecimal values should be on a line, it becomes a lot simpler though. We could just check if the line starts with 0x (e.g. a hexadecimal value).

Reading the file should also be done line by line as an iterator, to prevent huge files from crashing your program by too high memory consumption. Right now the program reads all the text in the file into memory. This can be done as described in the following question:

with open(input_file) as input_file:
    for line in input_file:
        # rest of code here.

Placing this in a function of its own also prevents you from opening the file continuously.

The final product

import argparse
import os

def write_and_print_result(args, result):
    """Write every result to the output file
    and print each converted value
    """
    out_file = args['input_file'][:-4] + "_1.txt"
    with open(out_file, 'a+') as output:
        for converted in result:
            output.write(converted + "\n")
            print('Converted: {}'.format(converted))

def read_and_convert_file(args):
    """Read the given input file and convert each hexadecimal value
    to decimal value
    """
    result = []
    with open(args['input_file']) as input_file:
        for line in input_file:
            if (line.startswith('0x')):
                result.append(str(int(line, 0)))
    write_and_print_result(args, result)

def get_cli_parser():
    parser = argparse.ArgumentParser(description="Describe program here")
    parser.add_argument('input_file', type=str,
                        help='The file to read the hexadecimal values from')
    return parser

def cli_runner():
    parser = get_cli_parser()
    args = vars(parser.parse_args())

    if not os.path.exists(args['input_file']):
        print('Input file does not exist, please try again.')
        return

    read_and_convert_file(args)

if __name__ == '__main__':
    cli_runner()

As you can see, most of it is either error checking or command line arguments. The actual conversion is only one line of code.

What's next

One issue I still found:

If the input file is anything but *.txt, the output file might have a weird name. For example, a file called hex would simply become _1.txt, which isn't very descriptive. To fix this, as pointed out by @Mathias Ettinger, you could use os.path.splitext. It is used as such:

filename, extension = os.path.splitext(args['input_file'])
outfile = filename + "_1.txt"


How can I refactor for handling multiple proto generated service clients by eliminating double await and the DRY code? I'm having doubts about which is the best strategy to manage the many service clients in this web app.
"Best" in terms of a good compromise between user's device RAM and Javascript execution speed (main thread ops).
This is what I'm doing right now, this is the main file:

main.ts:

import type { PlayerServiceClient } from './player.client';
import type { TeamServiceClient } from './team.client';
import type { RefereeServiceClient } from './referee.client';
import type { FriendServiceClient } from './friend.client';
import type { PrizeServiceClient } from './prize.client';
import type { WinnerServiceClient } from './winner.client';
import type { CalendarServiceClient } from './calendar.client';

let playerService: PlayerServiceClient;
export const player = async (): Promise<PlayerServiceClient> =>
    playerService ||
    ((playerService = new (await import('./player.client')).PlayerServiceClient()),
    playerService);

let teamService: TeamServiceClient;
export const getTeamService = (): TeamServiceClient =>
    teamService ||
    ((teamService = new (await import('./team.client')).TeamServiceClient()),
  teamService);

let refereeService: RefereeServiceClient;
export const getRefereeService = (): RefereeServiceClient =>
    refereeService ||
  ((refereeService = new (await import('./referee.client')).RefereeServiceClient()),
  refereeService);

let friendService: FriendServiceClient;
export const getFriendService = (): FriendServiceClient =>
    friendService ||
  ((friendService = new (await import('./friend.client')).FriendServiceClient()),
  friendService);

let prizeService: PrizeServiceClient;
export const getPrizeService = (): PrizeServiceClient =>
    prizeService ||
  ((prizeService = new (await import('./prize.client')).PrizeServiceClient()),
  prizeService);

let winnerService: WinnerServiceClient;
export const getWinnerService = (): WinnerServiceClient =>
    winnerService ||
  ((winnerService = new (await import('./winner.client')).WinnerServiceClient()),
  winnerService);

let calendarService: CalendarServiceClient;
export const getCalendarService = (): CalendarServiceClient =>
    calendarService ||
  ((calendarService = new (await import('./calendar.client')).CalendarServiceClient()),
  calendarService);

// and so on... a lot more...

As you can see there are many service clients.
I'm using this code because I thought it was better given my web app structure based on routes almost overlapping with client services:
I mean, if the player goes from /home to /players page I can use it like this:

components/players.svelte

import { getPlayerService } from "main";

const playerService = await getPlayerService();
const players = await playerService.queryPlayers();

In this way, if the PlayerService does not exist, it is imported at the moment and returned, otherwise it returns the one imported and instantiated before.
Since the user switches pages frequently this way I can avoid the sudden creation and destruction of those clients, right?
But in this way I am using global variables which I don't like to use and I'm using verbose, DRY and long code in each component.
Is there a way to use the below code in components instead?
import { playerService } from "main";

const players = await playerService.queryPlayers();

What do you suggest me to do?

Angular
If you use Angular you are able to use store. For example ngxs or ngrx modules for your application.
One more way in angular use resolvers.

Interface that classes can implement to be a data provider. A data
provider class can be used with the router to resolve data during
navigation. The interface defines a resolve() method that is invoked
when the navigation starts. The router waits for the data to be
resolved before the route is finally activated.

//player.resolver.ts
class PlayerResolver implements Resolve<number> {
   private req = false;
   private repSubject = new ReplaySubject(null);

   constructor(private service: PlayerServiceClient) { }

   public resolve(route: ActivatedRouteSnapshot): Observable {
        if (!this.req) {
            this.req = true;

            this.service.get().subscribe(res => this.repSubject.next(res));
        }
        return this.subject.pipe(first());
    }
}

//app-routing.module.ts
const routes: Routes = [
...
    {
     path: 'player'
     resolve: {data: PlayerResolver}
    },
    {
     path: 'friend'
     resolve: {data: FriendResolver}
    }
]

ReactiveX
If you use RxJs in your app, you would to do caching, create BaseSender:
//base.sender.ts
let hash = require('object-hash');

class BaseSender {

  ...
  protected httpSenderWithCache(service: Observable<any>,param): Observable<any> {
    const id = hash(param);

    if (!this.cache$[id]) {
      this.expireCache(id, time);
      this.cache$[id] = service.pipe(
        shareReplay(1), // last value
        catchError((err) => console.log(err))
      );
    }

    return this.cache$[id];
  }

}

shareReplay(1) gets last result.  const id = hash(param); generate unique hash for caching.
//player.service.ts
export class PlayerService extends BaseSender {
   ....
   public get(param = null): Observable<any> {
    param = param || {};

    const data = {
        url: 'https://example.com/player',
        options: {
          params: param,
        },
      };
    const request$ = this.http.get(data.url, data.options);

    return this.httpSenderWithCache(request$, data);
  }
}

General
If you don't use it all.

Install and configire inversify-binding-decorators with decorators for DI in app. In main.ts create container.

//main.ts
    import { injectable, Container } from "inversify";
    import { provide, buildProviderModule } from "inversify-binding-decorators";
    import "reflect-metadata";

    var container = new Container();
    // Reflects all decorators provided by this package and packages them into
    // a module to be loaded by the container
    container.load(buildProviderModule());

Create interface ClientServiceInterface

//client-service.interface.ts
interface ClientServiceInterface {
  public get(param = null);
}

Create services

    @provide(PlayerServiceClient)
    export class PlayerServiceClient implements ClientServiceInterface {

      private result;

      public function get() {
         if(!result) {
           return fetch('https://ex.com/player')
                   .then(res => {
                      this.result = res;
                      return Promise.resolve(res)
                    })
         }
         return Promise.resolve(result);
      }
    }

in your components you have to inject service

  class PlayerComponent {
       constructor(private playerService: PlayerServiceClient) {
         playerService.get().then(...)
       }
    }


Find smallest multiple Algorithm which returns a number which is divisible by all the numbers from \$1\$ to \$n\$ without remainders.

public static void smallestMultiple(long l){
    int count = 1;
    boolean flag = true;
    while(true){
        flag = true;
        for(int i=1; i<=l; i++){
            if( count % i != 0){
                count++;
                flag = false;
                break;
            }
        }
        if(flag){
            break;
        }
    }
    System.out.println(count);
}

Any better approach is there?

Your approach works but it is hard to read: all the logic is concentrated in the single smallestMultiple method, which is responsible for:

looping over the numbers
determining if one is divisible by all the number between 1 and l
printing the result

Dedicated methods
Looking at the points above, we clearly need a method whose goal will be to say if one number n is divisible by all the numbers between 1 and max.
private static boolean isDivisible(long n, int max) {
    for (int i = 2; i <= max; i++) {
        if (n % i != 0) {
            return false;
        }
    }
    return true;
}

This is very clear. It loops over all the numbers between 2 (no need to start at 1, thanks to Konrad Morawski that pointed it out) and max. If one does not divide n, then it returns false. If none were found, it means all divide n, so we return true.
Take note also of the spacing: adding a single space after the for, before the curly braces, after each semi-colon in the loop condition, greatly contributes to clarity.
long vs int
You are using inconsistently long and int. smallestMultiple takes a long as parameter but the inner loop uses an int:
//                                  vvvv
public static void smallestMultiple(long l){
    // ...
    while(true){
        //  vvv
        for(int i=1; i<=l; i++){

Also, count, which will be the result of the result is an int.
You need to clarify: what can be big enough to warrant using a long? Clearly, it is not the upper bound, which is 20 here. So l doesn't need to be a  long. count, on the other hand, is expected to be big so we could use a long for this variable. In this case, it turns out that the result fits in an int, but if we increase a little the upper bound, it won't fit anymore.
Namings

count is a bad name here. This is really the result of the calculation. It doesn't really count anything. It is merely an temporary variable that we increment. A generic result would be more explicit.
l is not very clear also. We could rename it upper or max to signify that this is the upper bound for the numbers to consider.

Main method
With the above dedicated method, we then have:
public static long smallestMultiple(int max) {
    long result = 1;
    while (!isDivisible(result, max)) {
        result++;
    }
    return result;
}

With such a method, one can clearly see what is happening: we are incrementing result while it is not divisible by all the numbers from 1 to max.
Note that it also returns the result, instead of printing it: you should let whoever is calling printing the result, printing is not the task of that method.
The big advantage here: no need to break any loop and no need for flag variables: having such flags is generally a code-smell indicating that the method is doing too much.
Faster methods
There are indeed faster methods:

taking the prime factorization of the numbers between 1 and the `max, as shown in this answer.
using a property of the lowest common multiple, as shown in this answer.


Determine the number of factors of a large numbers efficiently I am trying to determine the number of factors of a large number efficiently. I have written this function but how can it be improved upon?

long numOfFactors(long long n)
{
    long i=0,num=0;
    for(i=1;i<=n/2;i++)
    {
        if(n%i==0)
            num++;
    }
    return num;
}

Your program actually gives the wrong output for every number. You're forgetting that n is a factor of n! The easy fix is to simply initialize num to 1. Even better to initialize num to 2 and start your loop at i=2. Once you fix that, your program is fine. It's just that you can just do a lot better performance-wise.

Let's start with the general formula. Any number can be expressed as the product of primes raised to some power: \$n = \prod_i p_i^{k_i}\$. The number of factors of \$n\$ in this case is \$\prod_i(1+k_i)\$. So what we need to do is find all the prime factors.

How do we do that? We can actually reduce n as we go. Everytime we find a factor, if we do it right, we can be sure it's prime, so we reduce it as far as possible:

exp = 0;
while (n%i == 0) {
    ++exp;
    n /= i;
}

So if we're doing 80, as an example, we will divide by 2 until we're no longer divisible by 2 (that is, n == 5). All we have to do then is multiply out all the exps:

long numOfFactors(long long n)
{
    long num_factors=1;
    long i;
    long exp;

    for(i=2; i*i<=n; ++i)
    {
        exp=0;
        while (n%i == 0) {
            ++exp;
            n /= i;
        }
        num_factors *= (exp + 1);
    }

    return num_factors * (n > 1 ? 2 : 1);
}

The last check is there because if n>1 at the end, we know it's prime and we haven't counted it yet, so that's another doubling of factors.


Encoding special characters in URLs with Ruby I have the following Ruby code snippet:

  url = "http://somewiki.com/index.php?book=#{author}:#{title}&action=edit"
  encoded = URI.encode(url)
  encoded.gsub(/&/, "%26").sub(/%26action/, "&action")

The last line is needed since I have to encode the ampersands (&) in the author and title but I need to leave the last one (&action...) intact.

Consider this example for clarity:

author = "John & Diane"
title = "Our Life & Work"
url = "http://somewiki.com/index.php?book=#{author}:#{title}&action=edit"
encoded = URI.encode(url)
encoded.gsub(/&/, "%26").sub(/%26action/, "&action")

# => "http://somewiki.com/index.php?book=John%20%26%20Diane:Our%20Life%20%26%20Work&action=edit"

Although this result is satisfactory, I'd like to clean the original code with the gsub/sub calls. Any ideas?

PS: I could "clean" both params before passing them to the url = ... line but then the % characters will be re-encoded as %25 by the URI.encode call so I don't think that's an option. I'd love to be proved wrong here though.

Unless you need the unencoded url (for logging?), I would just:

encoded_url = "http://somewiki.com/index.php?book=#{CGI.escape(author)}:#{CGI.escape(title)}&action=edit"


Deep copy linked list with a random pointer I had this as an interview question through phone through an online text editor.

  The input is a linked list and in addition to a next pointer there is
  a random pointer that can point to any node in the list. Create a deep
  copy of the linked list and return the head of the new copy.

This my code below, which does the above Can some one review the code provide some feed back. Also is 28 mins too much for this code for a phone interview? I took my time but still made a few mistakes which has been fixed here already.

public class Node {
 public int val;
 public Node next;

 public Node random;

 public Node(int val) {
  this.val = val;
  next = null;
  random = null;
 }
}

class CopyUtil {

 Node deepCopy(Node head) {
  if (head == null) return head;
  Dictionary < Node, Node > nodePool = new Dictionary < Node, Node > ();
  Node current = head;
  Node newHead = new Node(head.val);

  nodePool.Add(current, newHead);

  newHead.random = getNode(nodePool, head.random);

  Node returnHead = newHead;

  while (current.next != null) {
   newHead.next = getNode(nodePool, current.next);
   newHead.next.random = getNode(nodePool, current.next.random);

   current = current.next;
   newHead = newHead.next;
  }

  return returnHead;

 }

 Node getNode(Dictionary < Node, Node > nodePool, Node current) {
  Node ret = null;
  if (current==null) return ret;
  if (nodePool.ContainsKey(current))
   ret = nodePool[current];
  else {
   ret = new Node(current.val);
   nodePool.Add(current, ret);
  }
  return ret;
 }
}

Mistakes:

1) Had a return type for constructor.
2) Had private (unspecified protection levels) Node values.
3) I was working with newHead.random instead of newHead.next.random
inside loop.
4) Missed new keyword in one of initialization of
Dictionary.
5) Missed null check when I moved to code to a util function.

Tips:

1) The editor was in plain text and 1/2/4 were stupid and silly mistakes which should have been caught if I patiently reviewed the code.
2) Practice on a plain notepad editor.
3) Have a headphone. Its important to collect your thoughts without worrying about dropping your phone.
4) Silly mistakes creeped in when I moved some code lines into a function and was doing some copy/paste. Be careful when you refactor your code during interviews.
5) Think your are submitting your code to codereview in stackexchange. lol.

public class Node {
 public int val;
 public Node next;

 public Node random;

 public Node(int val) {
  this.val = val;
  next = null;
  random = null;
 }
}

I understand that this was for a demo rather than production code, but it's really neither idiomatic C# nor good OO. I would have preferred

To use properties rather than fields;
To make the setters private, and move the clone method into Node;
Since the specification doesn't say anything about the contents, to parameterise as Node<T> rather than forcing the contents to be int (although maybe you clarified this with the interviewer and forgot to include it in the question).

I'm also not keen on exposing the Node class directly, although I'm not sure what API the enclosing LinkedList class would have with respect to the random links.

class CopyUtil {

 Node deepCopy(Node head) {

Node is public: why is CopyUtil internal and deepCopy private?

Why is neither the class nor the method static?

  if (head == null) return head;
  Dictionary < Node, Node > nodePool = new Dictionary < Node, Node > ();
  Node current = head;
  Node newHead = new Node(head.val);

  nodePool.Add(current, newHead);

Stylistically, I think there's far too much whitespace in Dictionary<Node, Node>. I suspect that you're applying C++ style guides designed to avoid parsing ambiguity with >> vs > >.

Why not Node newHead = getNode(nodePool, head)?

There is plenty of room for debate around when to use var, but I think few people nowadays would favour writing out an explicit type twice when calling a constructor.

  newHead.random = getNode(nodePool, head.random);

I think that with a bit of thought you could remove the special case head == null and this line and treat them as part of the general case in the loop.

  Node returnHead = newHead;

This is the first warning sign that newHead is a bad name. It's the head when first assigned, but later on it's really newCurrent.

 Node getNode(Dictionary < Node, Node > nodePool, Node current) {

I could have mentioned this above when the dictionary was created, but this is a better place: code to the interface, not the implementation. nodePool should be an IDictionary<Node, Node>.

Why current? That makes sense in deepCopy, but here a better name would be oldNode.

  Node ret = null;
  if (current==null) return ret;

I don't understand the point of ret given that you're not following the single exit principle.

  if (nodePool.ContainsKey(current))
   ret = nodePool[current];

This searches through the dictionary's data structures twice. The .Net API has a nicer way of doing it:

Node newNode;
if (nodePool.TryGetValue(current, out newNode)) return newNode;


Command-line-based capacitance decoding I am an amateur programmer and have just written a simple Capacitor Identifier mostly to practice. I have not had many people review my coding and thought this would be a good time to start. It is completed and works. Is there a better way I should have went about it? What should I look at in what and how I am writing that might improve my approach?

#!/usr/bin/env python3

import sys

# Get the arguments list
cmdargs = sys.argv

cmdargs = cmdargs[1:]

tolerances = {'B':'.10%','C':'.25%','D':'.5%','E':'.5%','F':'1%','G':'2%','H':'3%','J':'5%','K':'10%','M':'20%','N':'.05%','P':'+100% to 0%','Z':'+80% to -20%'}

# Get the total number of args passed
total = len(cmdargs)

# Templete String for output
capTemp = '{} = {}'

# output container
result = []

# cycle through passed arguments and decode each capacitor
for arg in cmdargs:
    case = len(arg)

    if(case == 2):
        temp = arg + ' pf, ' + str(float(arg) / 1000000) + 'uf'
        result.append(capTemp.format(arg, temp))
    elif(case == 3):
        cap = int(arg[:2]) * 10**int(arg[2:])
        temp = str(cap) + ' pf, ' + str(float(cap) / 1000000) + 'uf'
        result.append(capTemp.format(arg, temp))

    elif(case == 4):
        cap = int(arg[:2]) * 10**int(arg[2:3])
        temp = str(cap) + ' pf, ' + str(float(cap) / 1000000) + 'uf, ' + tolerances[arg[3:].upper()] + ' tolerance'
        result.append(capTemp.format(arg, temp))

# print the decoded capacitor values
for i in result:
    print(i)

Variable usage

Assign cmdargs once, and get it right the first time.  Put a more insightful comment there.  Rename it to cmd_args for readability.

# Skip argv[0], which is the name of this program
cmd_args = sys.argv[1:]

The total variable is never used.

The variable name case is not descriptive enough.  Naming a variable temp is almost always a bad idea as well.  I suggest names like arg_len and decoded_value, respectively.

tolerances is a constant, and should be named TOLERANCES by convention.  Format it using one entry per line for readability.

Decoding technique

It is good practice to separate your calculations from your output routines.  Start by defining a function decode_capacitance(code):

def decode_capacitance(code):
    '''
    Decodes the capacitance as a (value, tolerance) pair.
    The value is a float representing the nominal number of picofarads.
    The tolerance is a free-form string, and may be None.
    '''
    code_length = len(code)
    if code_length == 2:
        return float(code), None
    elif code_length == 3:
        return float(code[:2]) * 10 ** int(arg[2:]), None
    elif code_length == 4:
        return float(code[:2]) * 10 ** int(arg[2:3]), TOLERANCES[code[3:].upper()]

In Python, it is idiomatic to write if conditions without parentheses.

However, there is a lot of redundancy there, and not much insight either.  An improvement would be:

def decode_capacitance(code):
    '''(Same docstring)'''
    base_pF, scale, tol = float(code[0:2]), int(code[2:3] or 0), code[3:4]
    multiplier = 10 ** scale
    return base_pF * multiplier, TOLERANCES.get(tol.upper())

You fail to interpret the third digit properly.  If the third digit is 8 or 9, it is supposed to be interpreted as a multiplier of 0.01 or 0.1, respectively.  To fix that, change the middle line to:

    multiplier = 10 ** (scale if scale <= 5 else scale - 10)

As @vnp noted, you have no validation or error handling (and I haven't added any for you).

Formatting technique

Define another function to display the capacitance and tolerance in the desired format.

It would be nice, I think, to automatically select the most natural units.

The standard symbol for the farad is F — a capital letter.

Putting it all together

With the appropriate functions defined, the main loop is quite simple.  Print results as you go; there's no need to store results.

import sys

TOLERANCES = {
    'B': '.10%',
    'C': '.25%',
    'D': '.5%',
    'E': '.5%',
    'F': '1%',
    'G': '2%',
    'H': '3%',
    'J': '5%',
    'K': '10%',
    'M': '20%',
    'N': '.05%',
    'P': '+100% to 0%',
    'Z': '+80% to -20%',
}

def decode_capacitance(code):
    '''
    Decodes the capacitance as a (value, tolerance) pair.
    The value is a float representing the nominal number of picofarads.
    The tolerance is a free-form string, and may be None.
    '''
    base_pF, scale, tol = float(code[0:2]), int(code[2:3] or 0), code[3:4]
    multiplier = 10 ** int(scale if scale <= 5 else scale - 10)
    return base_pF * multiplier, TOLERANCES.get(tol.upper())

def format_capacitance(pF, tolerance):
    if pF > 1000000:
        mantissa, unit = pF / 1000000, 'uF'
    else:
        mantissa, unit = pF, 'pF'

    output = '{} {}'.format(mantissa, unit)
    if tolerance is not None:
        output += ', ' + tolerance + ' tolerance'
    return output

# Skip argv[0], which is the name of this program
cmd_args = sys.argv[1:]

# Cycle through passed arguments and decode each capacitor
for arg in cmd_args:
    print('{} = {}'.format(arg, format_capacitance(*decode_capacitance(arg))))


Sum of array of strings to bytes I'm trying to generate the sum of a collection of strings by calcuating each character's byte value into a sum:

$sum = 0;

foreach( $array as $item ) {
    $bytes = unpack( 'C*', $item );

    for( $i = 1; $i <= count( $bytes ); $i++ ) {
        if( isset( $bytes[$i+1] ) ) {
            $sum += $bytes[$i] - $bytes[$i+1];
        } else {
            $sum -= $bytes[$i];
        }
    }
}

return $sum;

The goal here is to compare past sums with newly generated sums (that is to say, check if there has been a new addition to the collection, suppose Item4) and perform actions if yes.

As such, it's very important that:

The algorithm can compute the sum irrespective of the order of the items.
The algorithm doesn't get confused by a case where let's say Item3 now becomes temI3 (and therefore the sum value is still the same, even though it's clearly not the same Item).

The whole secondary loop is to check against exactly that: loop through each byte  (character) from each Item$i and to the final summ, add the differene between the first and second bytes. If there isn't a next one and we are at the end of the string, simply subtract if from the whole sum.

As such, the following:

Input(s): ['Item1', 'Item2', 'Item3'] / ['Item1', 'Item2', 'Item3'] , output: the same int. Where as ['Item1', 'Item2', 'Ite3m'] outputs a different int.

The performance as of now is as follows:

100000 items across 100 runs: 0.18s / 1000000 items across 100 runs: 2.05s

And although I understand that to parse and do these calculations for a million items in 2s is rather fast for PHP, I still think that, if you look at what the thing does, it's still slow.

Any way to speed this up?

Dont call functions in loop conditions.

Calling count as condition of a for is a common mistake. It has the same behaviour but it is slower because the count function has to be called repeatedly although it returns the same value all the time. Unless the count changes during iterations, prefetch the count before the loop.

Iterate only where body is same

In your for you always check if the element is the last one, to do something else in that case. You better exclude it from the for, and handle the last element separately.

Unpack is not necesary

I wasnt really sure until I benched it, but just accessing individual characters of the strings with the [] operator and using the ord function to get the numeric value of bytes seems quite faster.

Not sure your algorithm is correct

As I've shown in me2 implementation, you are actualy adding just the first charracter of each word and subracting last character of each word twice.
It means that none of the characters except first and last in each word is contributing to the resulting sum. Therefore item Item1 is the same as Ixxx1. (Try for $input2 in the code below and see for yourself).

<?php

function op(array $array)
{
    $sum = 0;

    foreach( $array as $item ) {
        $bytes = unpack( 'C*', $item );

        for( $i = 1; $i <= count( $bytes ); $i++ ) {
            if( isset( $bytes[$i+1] ) ) {
                $sum += $bytes[$i] - $bytes[$i+1];
            } else {
                $sum -= $bytes[$i];
            }
        }
    }

    return $sum;
}

function op2(array $array)
{
    $sum = 0;

    foreach( $array as $item ) {
        $bytes = unpack( 'C*', $item );

        $length = count( $bytes );
        for( $i = 1; $i < $length ; $i++ ) {
            $sum += $bytes[$i] - $bytes[$i+1];
        }
        if ($length > 0) {
            $sum -= $bytes[$length];
        }

    }

    return $sum;
}

function me(array $array)
{
    $sum = 0;

    foreach( $array as $item ) {
        $length_1 = strlen($item) - 1;
        for ($i = 0; $i < $length_1; ++$i) {
            $sum += ord($item[$i]) - ord($item[$i + 1]);
        }
        if ($length_1 >= 0) {
            $sum -= ord($item[$length_1]);
        }

    }

    return $sum;
}

function me2(array $array)
{
    $sum = 0;

    foreach( $array as $item ) {
        $length = strlen($item);
        if ($length == 1) {
            $sum -= ord($item[0]);
        } elseif ($length > 1) {
            $sum += ord($item[0]) - 2 * ord($item[$length-1]);
        }
    }

    return $sum;
}

function bench(callable $callback, array $input, int $reps = 10000)
{
    $total = 0;
    for ($i = 0; $i < $reps; ++$i) {
        $start = \microtime(true);
        $callback($input);
        $total +=  \microtime(true) - $start;
    }
    return $total / $reps;

}

$input1 = ['Item1', 'Item2', 'Item3', 'X', ''];
$input2 = ['Ixxx1', 'Ixxx2', 'Ixxx3', 'X', ''];

$outOp = op($input1);
$outOp2 = op2($input1);
$outMe = me($input1);
$outMe2 = me2($input1);

echo 'OP: ' . bench('op', $input1);
echo \PHP_EOL;
echo $outOp;
echo \PHP_EOL;

echo 'OP improved: ' . bench('op2', $input1);
echo \PHP_EOL;
echo $outOp2;
echo \PHP_EOL;

echo 'No unpack: ' . bench('me', $input1);
echo \PHP_EOL;
echo $outMe;
echo \PHP_EOL;

echo 'Me2: ' . bench('me2', $input1);
echo \PHP_EOL;
echo $outMe2;
echo \PHP_EOL;

OP: 3.5921573638916E-6
-169
OP improved: 3.3084869384766E-6
-169
No unpack: 1.4432907104492E-6
-169
Me2: 5.5739879608154E-7
-169


Client categorisation based on hourly payment I am currently working on a small internal rating system for clients to be able to prioritise my work based on what client pays best. The input data is the hourly rate they are paying and based on that they should be divided into categories in are the range from A-D. The first setup looks like this:

def category_for(client)
  hourly_rate = client.hourly_rate
  case client.country
  when :germany
    return 'A' if hourly_rate >= 35
    return 'B' if hourly_rate >= 30
    return 'C' if hourly_rate >= 25
    return 'D' if hourly_rate < 25
  when :sweden
    return 'A' if hourly_rate >= 400
    return 'B' if hourly_rate >= 350
    return 'C' if hourly_rate >= 300
    return 'D' if hourly_rate < 300
  end
end

This is ofc only part of it to show the general setup.

This code, although working fine, really really hurts my eyes, so I was wondering if someone here had a good idea on how to refactor it in a nice way?

I like @SergeiiK's first solution, but we can make this more declarative:

GRADES = {
  germany: {
    "A" => 35...Float::INFINITY,
    "B" => 30...35,
    "C" => 25...30,
    "D" => 0...25,
  },
  sweden: {
    "A" => 400...Float::INFINITY,
    "B" => 350...400,
    "C" => 300...350,
    "D" => 0...300,
  },
}

def category_for(client)
  GRADES[client.country].find do |_, range|
    range.cover?(client.hourly_rate)
  end.first
end

Client = Struct.new(:country, :hourly_rate) # just for demonstration

p category_for(Client.new(:germany, 26))
# => "C"

p category_for(Client.new(:sweden, 7000))
# => "A"

You'd probably want to add some error handling to category_for for invalid inputs.

Note that the inner Hashes are Hashes for aesthetics only. Using Enumerable#find on a Hash will iterate over its key-value pairs one by one just as it would an array of key-value pairs ([ [ "A", 35...Float::INFINITY ], ... ]), making lookup O(n) instead of O(1) (ish) like normal Hash lookup. This is just prettier than nested arrays. This won't be as fast as a case solution, either, but unless you're, say, "grading" hundreds of thousands of clients at once you won't notice a difference.


Optimizing and clarifying nested list appending I have such part in my script, how can I optimize such parts?  I don't want to use many local variables.

size_variants = []
for size in sizes_list:
    size_variants.append({'size': size})

Also here:

variants_tmp = list()
all_color_list = list(get_all_colors())
[variants_tmp.extend(var) for var in all_color_list]

Using a list comprehension means you can eliminate all local variables:

size_variants = [{'size': size} for size in sizes_list]

Generally, anything of the form:

<new list initialization>
for <variable> in <list>:
    <new list>.append(<variable>)

can be written much more cleanly in list comprehension form.

Edit: If you want to extend an already existing list, and you have something like a nested list, then you'll probably want to use itertools.chain:

import itertools
x = [1, 2, 3]
y = [[4, 5], [6, 7, 8], [9, 10]
x = x + [i for i in itertools.chain.from_iterable(y)]

which will have x == [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].


Compile .tex and remove .div file from Haskell As a practice for interacting with system commands, I wrote a Haskell program that compiles a .tex file specified by the argument.

When one execute this program:

If no argument is specified, print "Specify a file!" then exit
If the specified file does not exist, print "File not found." then exit
If the specified file does not have the extension .tex, then print "Not a .tex file!" then exit
If a .tex file is specified (report.tex), execute the following commands

$ uplatex report.tex -o report.dvi
$ dvipdfmx report.dvi
$ rm report.dvi

The program does not have to handle errors in the execution of tex-related commands.

This is what I wrote:

import qualified Control.Shell as S
import Data.Char (toLower)
import Data.List (dropWhileEnd)
import qualified System.Environment as E

main = do
    args <- E.getArgs
    case args of
        [] -> putStrLn "Specify a file!"
        (filename:_)  -> handler filename

isTeXFile :: S.FilePath -> Bool
isTeXFile = ( == ".tex") . map toLower . S.takeExtension

handler :: S.FilePath -> IO ()
handler f = do
    (Right isfile) <- S.shell $ S.isFile f
    if not isfile
        then putStrLn "File not found."
        else
            if isTeXFile f
                then do
                    result <- compile f
                    case result of
                        (Left l) -> putStrLn l
                        (Right _) -> return ()
                else
                    putStrLn "Not a .tex file!"

compile :: S.FilePath -> IO (Either String ())
compile f = S.shell $ do
    S.run_ "uplatex" [f, "-o", dvifile] ""
    S.run_ "dvipdfmx" [dvifile] ""
    S.rm dvifile
    where
        basename = dropWhileEnd (/= '.') f
        dvifile = basename ++ "dvi"

I appreciate any kind of feedback. I'm especially concerned about:

The body of handler seems unnecessarily complicated. How can I clean it up?
Am I using Control.Shell in the right way? Is there other function I should use instead? Maybe I should consider using System.Process?

Side note: I don't use pdflatex because it is not as good as uplatex at Japanese typesetting and I haven't been catching up with the latest LaTeX families.

When you are using packages, please make sure to say which ones. I guessed shellmate btw. As for the code, it's quite readable except for the handler definition which I've rewritten as follows:

handler :: S.FilePath -> IO ()
handler f = do
  (Right isfile) <- S.shell $ S.isFile f
  if      not isfile        then putStrLn "File not found."
  else if not $ isTeXFile f then putStrLn "Not a .tex file!"
  else compile f >>= either putStrLn return

Given that you are quickly dismissing the cases you are not interested in, I inspected not $ isTeXFile f and systematically written the if (...) then clauses on the same line. This avoids the crazy right-leaning nesting.

Using the either combinator of type (a -> c) -> (b -> c) -> Either a b -> c lets you rewrite the final case expression in a more concise fashion.

Finally, I don't think it's good practice to test a file's type based on its extension. Given that you are already running external tools, you may want to use ̀file for that job. It would look something like this:

isTeXFile :: S.FilePath -> IO Bool
isTeXFile fp = do
  (Right ans) <- S.shell $ S.run "file" [fp] ""
  return $ " LaTeX " `isInfixOf` ans

handler :: S.FilePath -> IO ()
handler f = do
  (Right isfile) <- S.shell $ S.isFile f
  if not isfile        then putStrLn "File not found."
  else do
  testTexFile    <- isTeXFile f
  if not $ testTexFile then putStrLn "Not a .tex file!"
  else compile f >>= either putStrLn return

The rest is pretty much perfect as far as I'm concerned (modulo some cosmetic preferences which I won't bore you with).


Is this an good OOP Design in MVC PHP for getting User Details? I was given a API project by my co-worker to work on half created a system in slim PHP. The project consists of the Models and controllers. When the route is called it calls the Controller with the necessary function in the Controller, then query data from the database in Model functions and get back the response.

I want to know is this the correct way to implement OOP in PHP API applications? If not what are the changes and components I have to do to make it OOP MVC app?

User Controller -> UserController.php

public function getUser($request, $response, $args)
{
    $userId= $request->getAttribute('id');

    $user = $this->User->getUserById($userId);

    if (!$user) {
        return $response->withJSON([
          "error" => true,
          "message" => "cannot get user info",
          "data" => null
        ]));
    }

    return $response->withJSON([
        "error" => false,
        "message" => "cannot get user info"
        "data" => $user
    ]);
}

User Model -> User.php

public function getUserById($userId)
{
    $sql = "SELECT id, name, email
            FROM users
            WHERE id= :id";

    try {
        $stmt = $this->db->prepare($sql);
        $result = $stmt->execute([
            'id' => $userId
        ]);
        if ($result && $stmt->rowCount() == 1) {
            return $stmt->fetchAll()[0];
        } else {
            return false;
        }
    } catch (\PDOException $e) {
        $this->logger->info("cannot get user by id from DB " . $e);
        return false;
    }
}

There is not much OOP to review it. Actually that's just two functions, so one can review only the code, not the object structure. All in all there is a lot of repetitive or outright useless code

public function getUser($request, $response, $args)
{
    $userId= $request->getAttribute('id');
    $user = $this->User->getUserById($userId);
    return $response->withJSON([
        "error" => !$user,
        "message" => $user ? "success" : "no such user"
        "data" => $user ?: null
    ]);
}

as you can see, there could be just one return statement

public function getUserById($userId)
{
    $sql = "SELECT id, name, email FROM users WHERE id= :id";
    $stmt = $this->db->prepare($sql);
    $result = $stmt->execute(['id' => $userId]);
    return $stmt->fetch();
}

I took out all the useless code, namely

it is useless to verify $result. In case of error an exception will be thrown
it is always useless to check the rowCount()
why use fetchAll() if you need only one row?
it is useless to add any condition because fetch() will return FALSE already if no record found.
the same goes for the try-catch. There should be a site-wide error handler that will log the error and do something else like show a generic 500 error in the browser


Extracting watermark from a video using Python I'm extracting a few frames from a video:

Comparing the similarity or equality of pixel color (from first two images)
Saving to a new image
Comparing the new image (conjunction of first two images) and the next image, etc.

Can you review my codes for efficiency and best coding practices?

Code

import sys
import os
import numpy as np
from PIL import Image, ImageDraw

def main(obr1,obr2):
    img1= Image.open("%s" %(obr1))
    img2= Image.open("%s" %(obr2))
    im1 = img1.convert("RGBA")
    im2 = img2.convert("RGBA")
    pix1 = im1.load()
    pix2 = im2.load()
    im = Image.new("RGBA", (im1.width, im1.height), (0, 0, 0, 0))
    draw = ImageDraw.Draw(im)
    x = 0
    y = 0
    while y != im1.height-1 or x != im1.width-1:
        if pix1[x,y] == pix2[x,y]:
            draw.point((x,y),fill=pix1[x,y])
        else:
            p1 = np.array([(pix1[x,y][0]),(pix1[x,y][1]),(pix1[x,y][2])])
            p2 = np.array([(pix2[x,y][0]),(pix1[x,y][1]),(pix1[x,y][2])])
            squared_dist = np.sum(p1**2 + p2**2, axis=0)
            dist = np.sqrt(squared_dist)
            if dist < 200 and pix1[x,y] !=(0,0,0,0) and pix2[x,y] != (0,0,0,0):
                color = (round(pix1[x,y][0]+pix2[x,y][0]/2), round(pix1[x,y][1]+pix2[x,y][1]/2), round(pix1[x,y][2]+pix2[x,y][2]/2), round(pix1[x,y][3]+pix2[x,y][3]/2))
                #color=pix1[x,y]
                draw.point((x,y),fill=color)
            else:
                draw.point((x,y),fill=(0,0,0,0))
        if x == im1.width-1:
            x=0
            y=y+1
        else:
            x=x+1
    im.save('test%s.png' %(z), 'PNG')
    print("Zapisano obraz test%s.png" %(z))

imglist = sys.argv[1:]
z=0
while imglist != []:
    exists = os.path.isfile("./test%s.png" % (z-1))
    if exists:
        obr1="test%s.png" % (z-1)
        obr2=imglist.pop()
        print("Porównywanie obraza %s i %s" % (obr1,obr2))
        main(obr1,obr2)
        print("Analiza skończona")
        z=z+1
    else:
        obr1=imglist.pop()
        obr2=imglist.pop()
        print("Porównywanie obraza %s i %s" % (obr1,obr2))
        main(obr1,obr2)
        print("Analiza skończona")
        z=z+1

Best practices

A collection of general best practices for Python code can be found in the infamous Style Guide for Python Code (also called PEP8). While your code looks quite reasonable, there a two major points from the Style Guide I would like to point out to you.
First, add documentation to your functions (and maybe choose a more descriptive name than main, more on that shortly). Future-you will be very grateful for this.
Second, always use a single whitespace before and after = when assigning to a variable (no whitespace if used as keyword arguments in a function ! Relevant section of PEP8 here). The code is also generally better to read if you add a trailing whitespace after , like so: main(obr1, obr2) instead of main(obr1,obr2).

Another thing that I would consider a Python best practice, is to wrap code that is to be executed in a "scripty" manner in a if __name__ == "__main__": clause (also see the official documentation on that topic). That would allow you reuse/import the function currently named main into other functions without running the while loop. Therefore, I would like to suggest the following coarse code-level structure:

# imports would go here ...

def compare_images(filename1, filename2):
    """Compare two images and store the comparison to file"""
    # function logic would go here

def main():
    """Process arguments from command line"""
    imglist = sys.argv[1:]
    z = 0
    while imglist != []:
        # ...

if __name__ == "__main__":
    main()

I would also recommend to give some of the variables a more descriptive name (what do obr1 and obr2 stand for?). Also keep in mind that most of the people reading your code (including me) do not speak your mother tongue, so it's always nice to translate console output to English before posting it here.

Efficiency

.load() should probably not be necessary as per the documentation (this assumes your actually using Pillow fork and not the old and crusty PIL).

The most striking point in terms of efficiency is that Python is often terribly slow at loops. So the easiest way to gain performance is to get rid of them. But how? NumPy to the rescue! NumPy does all those pesky loops in C and is therefore orders of magnitudes faster compared to looping over array data in Python "by hand".

So what you would generally do to benefit from this is to get your image data as NumPy array (see this SO answer for a hint) and then work on those NumPy arrays with array operations, like masking. I will try to convey what I mean by that in a short example, maybe I can fully adapt it to your example later.

im1_np = ... # get data as numpy array, see SO post
im2_np = ... # get data as numpy array, see SO post
result = np.zeros_like(im1_np)   # same dtype and shape as input
matching_pixels = im1_np == im2_np   # boolean mask with true where pixels match exactly
result[matching_pixels] = im1_np[matching_pixels]   # this is your if clause

As you can see, there a no "manual" loops involved, everything is done by NumPy in the background.

Now to the else path. First, I think there might be some errors here, feel free to comment if I'm wrong. What (I think) you basically want to do, is to compute the difference between corresponding pixels and set them to a certain color if they are below a given threshold. Mathematically this would be expressed similar to this:

$$ \sqrt{(r_1-r_2)^2 + (g_1-g_2)^2 + (b_1-b_2)^2} < 200 $$

Your code does the following at the moment:

$$ \sqrt{r_1^2 + r_2^2 + g_1^2+g_2^2 + b_1^2+b_2^2} < 200 $$

When working from my definition above, the code becomes as follows:

dist_mask = np.sum(im1_np-im2_np, axis=2) < threshold
# remove pixels already set in the if clause
dist_mask = np.logical_and(dist_mask, np.logical_not(matching_pixels))
# remove all-zero pixels
dist_mask = np.logical_and(dist_mask, np.sum(im1_np, axis=2) > 0)
dist_mask = np.logical_and(dist_mask, np.sum(im2_np, axis=2) > 0)
# set color in result image as mean of both source pixels
result[dist_mask] = (im1_np[dist_mask]+im2_np[dist_mask]) / 2.

I leave threshold as variable since I'm not sure your original computation works the way you expect it and the threshold as chosen by you is meaningful. (Note: You can simply leave out the sqrt if you square the threshold value).
Apart from that, the code is a relatively strict transformation of your original conditions, it's just that instead of looping over the images pixel by pixel, everything is done in array operations.

Under the assumption that you actually want to assign the average pixel value of both source images, this can be optimized further, since the if condition of exact pixel equality is a subset of distance < threshold. This would save you a mask computation (matching_pixels would not be needed anymore) and the negation/and operation with the dist_mask. In case of exact equality, summing both values and dividing them by two should leave you with the original value (Warning: Watch out for quirks with floating point values and/or range-limited integer values).

To be fully compatible with your original code you would then have to go back to PIL to store the image to disk. This should also be described in the SO post linked above.

Other things

You are sometimes using string formatting in a weird way. If you just want make sure that a variable is a string, pass it to str(...) instead of using string formatting. If you really need string formatting such as where you create the output filename, it is often recommended to use .format(...) (Python 2, Python 3) or f-strings (Python 3) to format string output. There is a nice blog post here that compares all ways of doing string formatting in Python I mentioned.


Find the odd int challange this is a function given an array of numbers to find the one that appears an odd number of times.
what I did is:

Sort the array
Check if the array is odd.
If the array length is 1 then return this value.
Assign every two values to a new array, and check if it is a number.
Check if every two values are equals.
If not equals then the result will be the first element on the new array

function findOdd(A) {

  let copA = Array.from(A).sort((a, b) => a - b);
  let res = 0;

  if (copA.length % 2 === 0) return;
  if (copA.length == 1) return res = copA[0];

  let newArr = [];
  for (let i in copA) {
    // if not a integer break
    if (!Number.isInteger(copA[i])) {
      res = 0;
      break;
    };

    newArr.push(copA[i]);
    if(newArr.length == 2) {
      const [a,b] = newArr;
      if (a === b) {
        newArr = [];
      } else {
        res = a;
        newArr[0] = b;
        break;
      }
    } else if (newArr.length == 1) {
        res = newArr[0];
    }
  }
  return res;
}

// test the code
findOdd([20,1,-1,2,-2,3,3,5,5,1,2,4,20,4,-1,-2,5]); // 5
findOdd([1,1,2,-2,5,2,4,4,-1,-2,5]); // -1
findOdd([20,1,1,2,2,3,3,5,5,4,20,4,5]); // 5
findOdd([10]); // 10
findOdd([1,1,1,1,1,1,10,1,1,1,1]); // 10
findOdd([5,4,3,2,1,5,4,3,2,10,10]); // 1

My code works well, but I looking to improve it.

Review and Complexity
This answer is in two parts.

A review of the code and a rewrite based on the review. There are what might be bugs. This addressed in the second part.
A look at complexity, how to reduce complexity. An example function is given with lower complexity.

Review

Do not use un-delimited code blocks eg if (copA.length == 1) return res = copA[0]; should be if (copA.length == 1) { return res = copA[0]; }

Use === (Strict equality) rather than == (Equality) The same would apply for !== (Strict inequality) rather than != (Inequality)

Avoid using for...in without checking if the key is a property or index of the array/object you are iterating. Use for...of for arrays and if the item being iterated is an object use Object.keys to create an array of property names.

Use const for variables that do not change. Eg the array copA never changes and should be a const.
On the same point it is preferred to use const when possible, If you use newArr.length = 0 rather than newArr = [] you can then make newArr a const. This is also more efficient at run time as zeroing an array is quicker than creating a new array.

The general rule for line ends is use a semicolon only when ASI (Automatic Semicolon Insertion) is ambiguous (The assumption is you may not know what the next line of code is eg when using bundlers, pre-parse parser, anything that changes the code before parsing).
A } that delimits a code block is an unambiguous line terminator and does not require a semicolon.
A } at the end of a object literal can be ambiguous and does requires a semicolon.

Most comments are just code noise, often they assume the reader does not know how to read code.
Comments that are a Human readable expressions of intent should be avoided.
If you find that some code is a little hard to understand from just reading the code it is likely that some renaming and or restructuring will make your intent clear rather than having to explain your intent with a comment

Use modern syntax.
To copy an array use the spread operator ...(Spread syntax) rather than  Array.from
Names
Names should be what the value represents rather than its type.

A (for array) can be values or numbers The plural means its an array

copA function arguments are not constants and you can reuse them. See rewrite.

Across many languages we use i, j, and sometimes k as counters. You should only use these names when they are counters. The loop for (let i in copA) { should use a name that represent what the value holds. Eg for (let num in copA) { or for (let val in copA) {

Efficient code
Always be efficient and avoid redundancy.

The line return res = copA[0] is a redundant assignment and should be return copA[0]

The second else if(newArr.length == 1) { is not required and can just be else {

Array.push returns the length of the array after the push.
Chrome, Edge, Nodejs and many APPs that use JS engines use V8. V8 has a quirk that makes indexed references eg newArr.length slower than direct or immediate references. Thus it would be better to use the statement if (newArr.push(copA[i]) === 2) {`` rather than if(newArr.length == 2)`
Please note that this is trivial and as such would be something to you do when you are comfortable with your coding knowledge.

Don't use arrays to count occurrences . See rewrite. Because I see this type of thing so often will boldly yell 'DON'T USE A ARRAYS TO GET COUNTS'

The line  newArr[0] = b; is followed by a break thus you never use the array newArr again making the assignment redundant.

Rewrite
The rewrite does not create a new algorithm. It behaves the same as you code and passes the provided tests. This does not mean it will do so on all tests but I am limited by only what you provide.
The rewrite is just a clean up of the code in regards to the points outlined above.

The  array use to count numbers has been replaced with a counter.
To keep the previous value a variable called prevNum is used.
As prevNum is the value you return via res, res has been removed with prevNum.

The code
    function findOdd(numbers) {
        let count = 0, prevNum;
        numbers = [...numbers].sort((a, b) => a - b);
        if (numbers.length % 2 === 0) { return }
        if (numbers.length === 1) { return numbers[0] }

        for (const num of numbers) {
            if (Number.isInteger(num)) {
                if (num === prevNum) {
                    count ++;
                } else if (prevNum !== undefined) {
                    if (count % 2) { break }
                    count = 1;
                    prevNum = num;
                } else {
                    count ++;
                    prevNum = num;
                }
            } else { return 0 }
        }
        return prevNum;
    }

Complexity
This problem  has a complexity of \$O(n)\$.
Your code has a complexity of \$O(n log( n))\$. Why? because you sort the array. Sorts are complex. If you can avoid using a sort do so.
I understand the reasoning behind the use of a sorted array, to group numbers.
We must find a better way to  group values but first your code has some inconsistent results (or bugs?)
Bugs???
Note: that in the rewrite (above) I pointed out that it only passed the tests provided. Any working code provides all that is needed to rewrite it and  pass any test. However I strongly suspect that you have bugs that are not detected by the tests you provided.
Test are useless if they do not check every edge case. Edge cases are more often than not the source of bugs. Just looking at the code an I see inconsistency in the return values.
Inconsistencies
The inconsistent results due to

The early return of even sized arrays.
The early break if a non integer is found.
The problems description is incomplete as it does not clearly state what should be returned in some edge cases

These Inconsistencies mean that the following inputs give or (may be giving) unexpected results
findOdd([1,0.1,2,1,0.1])); // returns 0, would expect 2?
findOdd([0.1,1]));         // returns undefined, would expect 1?
findOdd([0.1,1,1]));       // returns 0, The return has not been defined

There are also 2 unexplained return values, each the contra of the other

Returning the first and lowest value if the array length is 1. Why the lowest?
Returning zero. Why zero and not the lowest as above?

Reducing complexity
I will reduce the complexity to \$O(n)\$ by using a hash map to count groups. JS provides several hash maps Map, Set, WeakMap and WeakSet. Only the first two can help in this case as Number is not a referenced value. (exception when number is created using Number(123))
First some assumptions
I must make some assumptions about how it behaves in edge cases.

Even sized arrays may still contain odd groups of values.
If there are no odd groups the first integer value is returned.
If there are no integer values or the array is empty undefined is returned.
All values in the array are of type Number

Steps
An explanation of the new solution
Create a hash map to count occurrences in each group.
Iterate each value in input,
   If a value is an integer
       If the values is in the map add one to the count
       If not in the map add to the map with a count of 1
Iterate the completed map.
   If an entry is an odd count return that maps value as the result
If the size of the hash map is zero return undefined
return the first value in the hash map.

Getting the first entry from a hash map can be done using [...map.keys()][0] however that is is overly complex \$O(n)\$. Rather you can use the iterator protocols Iteration protocols next().value with a complexity of \$O(1)\$
The \$O(n)\$ solution
function findOdd(numbers) {
    const groupsByVal = new Map();
    for (const num of numbers) {
        if (Number.isInteger(num)) {
            const counter = groupsByVal.get(num);
            if (counter) {
                counter.count ++;
            } else {
                groupsByVal.set(num, {count: 1});
            }
        }
    }
    for (const [num, {count}] of groupsByVal.entries()) {
        if (count % 2) {
            return num;
        }
    }
    return groupsByVal.keys().next().value;
}

Or I prefer
function findOdd(numbers) {
    const grpByVal = new Map();
    for (const num of numbers) {
        if (num % 1 === 0) {
            const grp = grpByVal.get(num);
            if (grp) { grp.c ++ }
            } else { grpByVal.set(num, {c: 1}) }
        }
    }
    for (const [num, {c}] of grpByVal.entries()) {
        if (c % 2) { return num }
    }
    return grpByVal.keys().next().value;
}


Implementing an asynchronous mutex in C# I wrote a simple synchronization primitive that I can use with async operations on an external REST service so that I don't call it twice from different threads and have one call fail due to not having the latest state. Despite obvious limitations of that design decision I think this should solve my problem...

struct DumbAsyncMutex
{
    int busy;

    public async Task WithMutexAsync(Func<Task> t)
    {
        while (Interlocked.CompareExchange(ref busy, 1, 0) != 0)
        {
            await Task.Delay(100);
        }

        try
        {
            await t();
        }
        finally
        {
            while (Interlocked.CompareExchange(ref busy, 0, 1) != 1)
            {
                throw new Exception("Releasing the mutex failed - but this should never happen");
            }
        }
    }
}

Of course this is a hack... did I just overlook a better way to do this using the standard framework?

A better alternative to the busy wait loop would be to use a Semaphore - in this case a SemaphoreSlim would probably be the most appropriate. Semaphores are not reentrant. This would simplify the implementation to:

struct DumbAsyncMutex
{
    private SemaphoreSlim _Semaphore = new SemaphoreSlim(1, 1);

    public async Task WithMutexAsync(Func<Task> t)
    {
        await _Semaphore.WaitAsync();

        try
        {
            await t();
        }
        finally
        {
            _Semaphore.Release(1);
        }
    }
}

The advantage is that it has async support and you can abandon the waiting by using cancellation tokens.


A recursive Boggle solver `Boggle is a word game in which letters are randomly placed in a 4x4 grid
e.g:

A D Q P
N L E M
O S R T
V K J H

Words can be started from any letter and are formed by finding a sequence of connecting letters. Letters may connect diagonally, vertically or horizontally.

Example words on the above board are "son", lad" and "land". Letters may not be re-used.

Below is a recursive solution to the boggle board.
The problem with my solution is it is very slow. I had to limit word length to 8 otherwise it takes far too long.

Please can you comment on general style improvements and if you can think of another method to solve this game, a hint as to what I should do for my next attempt.

The dictionary is from here

"""Boggle game solver"""
import copy

def words_from(board, row, column, running_string="", list_words=[]):
    """Calculate all possible words from a given starting position [row, column]"""
    if row in (4, -1) or column in (4, -1):
        return
    if len(running_string) > 4:
        return
    if board[row][column] != "-":
        new_string = running_string + board[row][column]
        new_board = copy.deepcopy(board)
        new_board[row][column] = "-"
        # Add new word
        if len(new_string) >= 3:
            list_words.append(new_string.lower())

        # Find next word
        next_move = [
            (1, 1),
            (-1, -1),
            (1, -1),
            (-1, 1),
            (1, 0),
            (0, 1),
            (-1, 0),
            (0, -1),
        ]
        for dx, dy in next_move:
            words_from(new_board, row + dx, column + dy, new_string, list_words)
        return list_words

def get_permutations(board):
    """Get all permutations """
    set_permutations = set()
    counter = 0
    print("Working..", end = "")
    for row in range(4):
        for column in range(4):
            print(".", end="")
            counter += 1
            words = words_from(board, row, column, list_words=[])
            if words:
                for word in words:
                    set_permutations.add(word)
            words = None
    return sorted(list(set_permutations))

def dictionary_check(set_permuations):
    """Check set_permutations for valid English words"""
    dictionary = {}
    with open("en-dict.txt", "r", encoding="utf8") as file:
        for line in file:
            dictionary[line.strip()] = 0

    counter = 0
    for word in set_permuations:
        if word.lower() in dictionary:
            counter += 1
            print(word)
    print(f"======\n{counter} words")

def find_words(board):
    """Find words on the boggle board"""
    set_permutations = get_permutations(board)
    print("Performing dictionary check....")
    dictionary_check(set_permutations)

def build_board(string):
    """Build board from string"""
    if len(string) != 16:
        print("Error. Must enter 4*4 grid (16 characters)")
        return
    board = [[*string[0:4]], [*string[4:8]], [*string[8:12]], [*string[12:16]]]
    find_words(board)

if __name__ == "__main__":
    string_board = "playthiswordgame"
    build_board(string_board)

The problem that you observe with this program is speed, so let's look at that.

Running the program, I immediately noticed that the get_permutations section was slow, and the dictionary_check section was many times faster. That immediately tells me that it's not worth looking for faster ways to do the dictionary_check until get_permutations is much faster. After all, even if we could make dictionary_check take no time at all, the program would take almost as long to run!

Of course, I've been a bit naughty there. I went with my internal clock, when what I should do is use a tool. This is the result of running cprofile.

python -m cProfile -s cumtime boggle.py

             116983186 function calls (93930898 primitive calls) in 32.455 seconds

   Ordered by: cumulative time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        1    0.000    0.000   32.455   32.455 {built-in method builtins.exec}
        1    0.052    0.052   32.455   32.455 boggle.py:1(<module>)
        1    0.009    0.009   32.403   32.403 boggle.py:62(find_words)
        1    0.085    0.085   31.945   31.945 boggle.py:34(get_permutations)
5763088/16    4.231    0.000   31.726    1.983 boggle.py:15(words_from)
15128064/720384   12.915    0.000   27.119    0.000 copy.py:132(deepcopy)
3601920/720384    5.565    0.000   25.605    0.000 copy.py:210(_deepcopy_list)
 30256128    3.207    0.000    3.207    0.000 {method 'get' of 'dict' objects}
  3601920    1.764    0.000    2.288    0.000 copy.py:252(_keep_alive)
 23052288    1.619    0.000    1.619    0.000 {built-in method builtins.id}
 18009500    1.261    0.000    1.261    0.000 {method 'append' of 'list' objects}
 11526144    0.840    0.000    0.840    0.000 copy.py:190(_deepcopy_atomic)
        1    0.289    0.289    0.448    0.448 boggle.py:50(dictionary_check)
  4431757    0.324    0.000    0.324    0.000 {built-in method builtins.len}
   720284    0.131    0.000    0.131    0.000 {method 'add' of 'set' objects}
      173    0.076    0.000    0.076    0.000 {built-in method builtins.print}
   712738    0.067    0.000    0.067    0.000 {method 'lower' of 'str' objects}
   178691    0.017    0.000    0.017    0.000 {method 'strip' of 'str' objects}
      240    0.000    0.000    0.003    0.000 cp1252.py:22(decode)
      240    0.003    0.000    0.003    0.000 {built-in method _codecs.charmap_decode}
        1    0.000    0.000    0.000    0.000 {built-in method io.open}
        1    0.000    0.000    0.000    0.000 _bootlocale.py:11(getpreferredencoding)
        1    0.000    0.000    0.000    0.000 {built-in method _locale._getdefaultlocale}
        1    0.000    0.000    0.000    0.000 boggle.py:5(check_board)
        1    0.000    0.000    0.000    0.000 codecs.py:259(__init__)
        1    0.000    0.000    0.000    0.000 {method 'disable' of '_lsprof.Profiler' objects}

The first few lines are just the call sequence in: for example there's a lot of time (cumtime) spent in find_words but almost all of it is in functions that it's calling and very little in the function directly (tottime). That's not where you want to cut down.

Instead, a huge amount of time is spent within deepcopy: 27 of 32 seconds. That is genuine time expenditure, and a good place to start hitting. Two options occur to me: either look for a simpler board representation that is cheaper and easier to copy, or try to avoid the copies.

For option 1, the obvious simpler thing is a flat list or tuple with sixteen elements, which you then index into as [row * 4 + column]. The data would be the same, but you'd avoid the overhead of copying all the extra lists.

For option 2, you'd want to use one board and keep track of what you're changing (and, depending on your implementation, perhaps exactly one copy of the board you never change). When you use a letter you'd stub it out; when you come back up the tree you'd replace the stub symbol with the original letter.

I haven't done it myself and it's always dangerous guessing at performance, but I would be optimistic about getting four to five times faster with that second change.

The above is trying for efficiency gains with minimal changes to the underlying algorithm. If you want to get much faster, however, you'll need to change your approach to the problem. The first rule of getting a job done faster is "The fastest work is the work you don't do."

Although I said earlier and stand by that you don't need to start optimising dictionary_check, there may be some opportunities to benefit from knowing your word list while you explore the grid. For example, there are no words that start with "plt". If, then, your running_string is "plt" then all future strings you find are guaranteed to get filtered out at the end. One option would be to read your word list at the start, and prepare a dictionary of all the prefixes that appear. As you recursively call words_from, if the running_string is not in the prefix dictionary, return immediately. That would probably offer enough gains that you could remove your limit to length 8 words.

I notice that the question and code have been editted several times since I started this answer. I'm just going to post it as is, and hope that except in the most fiddly details it is still helpful.


Large number to many characters My goal is to take any size number, and convert it to a string of smaller characters that represent the concatenation of the binary expansion of the number. To illustrate what I am talking about, here is an example using the number 4820:

4820's binary:   0001001011010100   #   16 bits of binary, enough to store the number 4820
Separated:     00010010  11010100   #   Simply split the binary into two char size pieces
Characters:      '\x12'       'Ô'   #   Converted the binary snippets into their respective characters
Together:                 '\x12Ô'   # The concatenation of the characters, this is what I need

I wrote a algorithm in Python to do the job for me, but I'm not sure if there is a easier way to do it, or if my algorithm is as efficient as it can be.

def to_chars(number, length):                             #takes number and expected binary length
    string = bin(number)[2:]                              #built-in binary conversion
    expanded = ('0' * (length - len(string))) + string    #expand the binary
    chars = ''
    for pos in range(int(length / 8)):                    #iterates over chunks of 8 bits
        section = expanded[8 * pos : 8 * (pos + 1)]       #gets the binary snippet
        chars += chr(int(section, 2))                     #converts the binary into a character
    return chars                                          #return the concatenated string

There has to be a lower level, possibly built in way to do the same thing, but faster. In a way, this is just the conversion from one data type to another, but I can't figure out a faster way.

Here are a few benchmarks:

%timeit to_chars(4820, 16)
100000 loops, best of 3: 3.04 µs per loop

%timeit to_chars(4820, 32)
100000 loops, best of 3: 4.6 µs per loop

%timeit to_chars(690655640, 32)
100000 loops, best of 3: 4.59 µs per loop

Well, it's not a huge performance improvement, but at least it's a lot cleaner:

def to_chars2(number, length):
    bin_num = bin(number)[2:].zfill(length)
    return ''.join([chr(int(bin_num[s:s+8], 2)) for s in range(0, len(bin_num), 8)])

Benchmarks:

>>> timeit(lambda: to_chars2(4820, 16))
1.590714931488037
>>> timeit(lambda: to_chars(4820, 16))
1.6366891860961914


Equals method for a class representing an origin and destination Have I over-ridden GetHashCode properly in the below code?

public class ODPair
{
    #region Constructors and Destructors

    public ODPair(string origin, string destination)
    {
        if (string.IsNullOrEmpty(origin))
        {
            throw new ArgumentNullException(origin, "Origin of an ODPair should be non-null");
        }

        if (string.IsNullOrEmpty(destination))
        {
            throw new ArgumentNullException(destination, "Destination of an ODPair should be non-null");
        }

        this.Origin = origin;
        this.Destination = destination;
    }

    #endregion

    #region Public Properties

    public string Destination { get; private set; }

    public string Origin { get; private set; }

    #endregion

    #region Public Methods and Operators

    public override bool Equals(object obj)
    {
        if (obj == null)
        {
            return false;
        }

        var pair = obj as ODPair;
        if (pair == null)
        {
            return false;
        }

        return this.Origin == pair.Origin && this.Destination == pair.Destination;
    }

    public bool Equals(ODPair pair)
    {
        if (pair == null)
        {
            return false;
        }

        return this.Origin == pair.Origin && this.Destination == pair.Destination;
    }

    public override int GetHashCode()
    {
        return this.Origin.GetHashCode() ^ this.Destination.GetHashCode();
    }

    #endregion
}

Equals

You have a bit of repetition between your Equals methods. Since no instance is equal to null, you can replace your entire overridden Object.Equals method with:

return Equals(obj as ODPair);

If the obj is not an ODPair, this will just pass in a null which will then return false.

At a pinch, XOR is a passable way of combining hash-codes. But it has collision issues:

All instances where the origin and destination are the same will have the hash code of 0 (you mentioned this will never be the case for this class, but it's worth keeping in mind in general)
Two instances where the origin and destination are swapped will have the same hash code.
As Simon mentions in his answer, if the hashcodes being XORed occupy a small subset of the integers, there will be frequent collisions. This too is not likely to be much of a problem for you because the codes getting combined are from strings, so are likely to span the integers pretty well.

So of those it's likely to be the second, if any, which is of concern in this particular case.

My preference is to avoid the complexity of trying to implement my own hashing algorithm and let .NET do it for me:

return Tuple.Create(Origin, Destination).GetHashCode();

or:

return new {Origin, Destination}.GetHashCode();

Only if this presents a noticeable performance issue should you worry about trying to implement your own- premature optimization is still premature optimization even inside a GetHashCode method!


Compare two arrays of objects and get the objects based on ids I have two arrays of objects. One has an id property with a list of ids and the second has an object with a unique id property. I want to filter the ids in the second array with the first list and get the data.

const data1 = [{
  name: 'A',
  ids: [1, 2, 3]
}, {
  name: 'B',
  ids: [4, 5, 6]
}, {
  name: 'C',
  ids: [7, 8, 9]
}]

const data2 = [{
    id: 1,
    color: 'red'
  }, {
    id: 2,
    color: 'black'
  }, {
    id: 3,
    color: 'blue'
  }, {
    id: 4,
    color: 'yellow'
  }, {
    id: 5,
    color: 'green'
  }, {
    id: 6,
    color: 'pink'
  },
  {
    id: 7,
    color: 'orange'
  }, {
    id: 8,
    color: 'white'
  }, {
    id: 9,
    color: 'teal'
  }
]

const arrayToObject = (array) =>
  array.reduce((obj, item) => {
    obj[item.id] = item
    return obj
  }, {})

console.log(arrayToObject(data2))

const newData = data1.map(item => {
  return {
    name: item.name,
    data: item.ids.map(i => arrayToObject(data2)[i])
  }
})

console.log(newData)

Expected Output:
[
  {
    "name": "A",
    "data": [
      {
        "id": 1,
        "color": "red"
      },
      {
        "id": 2,
        "color": "black"
      },
      {
        "id": 3,
        "color": "blue"
      }
    ]
  },
  {
    "name": "B",
    "data": [
      {
        "id": 4,
        "color": "yellow"
      },
      {
        "id": 5,
        "color": "green"
      },
      {
        "id": 6,
        "color": "pink"
      }
    ]
  },
  {
    "name": "C",
    "data": [
      {
        "id": 7,
        "color": "orange"
      },
      {
        "id": 8,
        "color": "white"
      },
      {
        "id": 9,
        "color": "teal"
      }
    ]
  }
]

I did manage to achieve this, but I guess there might be a better clean and performant solution. Please advice me on that.
P.S: I am also open to using lodash.

Use Map to reduce search complexity
Currently your method is rather complex at \$O(n^2)\$. The reason is that for each Id you iterate each item to locate the one with the Id you want.
You can use a Map to reduce the complexity. Map uses a hash to locate items, you can add each id to a map linking ids to each category.
Then just iterate the items array using the map to locate the correct category and add the item.
The result is achieved in a time complexity of \$O(n)\$.
The example shows how this can be implemented.

const categories = [{name: 'A', ids: [1, 2, 3]}, {name: 'B', ids: [4, 5, 6]}, {name: 'C', ids: [7, 8, 9]}]
const items = [{ id: 1, color: 'red'}, { id: 2, color: 'black'}, { id: 3, color: 'blue'}, { id: 4, color: 'yellow'}, { id: 5, color: 'green'}, { id: 6, color: 'pink'},{ id: 7, color: 'orange'}, { id: 8, color: 'white'}, { id: 9, color: 'teal'}];

function categorizeById(cats, items) {
    const idMap = new Map();
    const result = cats.map(cat => {
        const category = {name: cat.name, data: []};
        cat.ids.forEach(id => idMap.set(id, category));
        return category ;
    },[]);
    items.forEach(item => idMap.get(item.id).data.push({...item}));
    return result;
}
console.log(categorizeById(categories, items));


Queue By Using Stack I have implemented a queue by using a stack data structure. How could I improve it?

package _amazonAskedQuestions.dataStructures;

import java.util.Iterator;
import java.util.Stack;

public class Queue_Stack<T> implements  Iterable<T>{

    Stack<T> stack = new Stack<>();

    public void enqueue(T item){
        Stack<Integer> temp = new Stack<Integer>();
        while(!stack.empty()){
            temp.push((Integer) stack.pop());
        }
        stack.push(item);
        while(!temp.empty()){
            stack.push((T) temp.pop());
        }
    }

    public T dequeue(){
        return stack.pop();

    }

    @Override
    public Iterator<T> iterator() {
        return (Iterator<T>) stack.lastElement();
    }

    public static void main(String args[]){

        Queue_Stack<Integer>  qs = new Queue_Stack<>();

        qs.enqueue(9);
        qs.enqueue(8);
        qs.enqueue(1);
        qs.enqueue(3);
        qs.enqueue(4);
        qs.enqueue(5);

       System.out.println( qs.dequeue());

    }
}

Your Queue_Stack only works with Integer.  I tried using String and got a ClassCastException.

I made some modifications to your code.  I added a test to make sure you don't get an error if you dequeue more than you enqueue.  I used the underlying iterator to get rid of the temp Stack.

import java.util.Iterator;
import java.util.Optional;
import java.util.Stack;

public class Queue_Stack<T> implements Iterable<T> {

    Stack<T> stack = new Stack<>();

    public void enqueue(T item) {
        stack.push(item);
    }

    public Optional<T> dequeue() {
        if (!stack.empty()) {
            T item = stack.iterator().next();
            stack.remove(0);
            return Optional.of(item);
        } else {
            return Optional.empty();
        }
    }

    @Override
    public Iterator<T> iterator() {
        return (Iterator<T>) stack.iterator();
    }

    public static void main(String args[]) {
        Queue_Stack<String> qs = new Queue_Stack<>();

        qs.enqueue("zeta");
        qs.enqueue("alpha");
        qs.enqueue("beta");
        qs.enqueue("gamma");

        Iterator<String> iter = qs.iterator();
        while (iter.hasNext()) {
            System.out.println(iter.next());
        }

        System.out.println(qs.dequeue());
        System.out.println(qs.dequeue());
        System.out.println(qs.dequeue());
        System.out.println(qs.dequeue());
        System.out.println(qs.dequeue());
    }
}


Function to scan input to string buffer in C I have written a function that reads standard input, ignores initial whitespace and fills the buffer with that data.
I think that it is a bit large and cumbersome, can you please tell me what can be improved?

int dscan(char *where, int size) {
    int found = 0;
    int i = 0;
    int j = 0;
    for (i = 0; i < size - 1; i++) {
        char c;
        scanf("%c", &c);
        if (c == '\n') {
            where[j] = '\n';
            where[j + 1] = '\0';
            return j;
        }
        if (isspace(c)) {
            if (!found) {
                continue;
            }
        } else {
            found = 1;
        }
        where[j] = c;
        j++;
    }
    return j;
}

You must check the return value of scanf. If you don't, you run into an endless loop when reaching the end of the file. And, even worse, you invoke undefined behavior.

You could split the loop into two: first, skip all whitespace, then read the remaining text.

int dscan(char *buf, int bufsize) {
    int ch;

    while ((ch = getc()) != EOF && ch != '\n' && isspace(ch)) {
        continue;
    }

    int idx = 0;
    while (ch != EOF && ch != '\n' && idx < bufsize) {
        buf[idx++] = ch;
        ch = getc();
    }

    if (ch == '\n' && idx < bufsize) {
        buf[idx++] = '\n';
    }
    if (idx < bufsize) {
        buf[idx++] = '\0';
    }

    return idx;
}

I changed several things:

EOF is handled correctly. On EOF, the string is returned, properly null-terminated. (This was not the case in your code.)
Leading whitespace does not count towards the buffer, it is simply ignored, like you said in your description. Your code counted it.
Each call to an I/O function is checked for an error result.
It's easy to strip the newline character from the string by removing the corresponding if sentence. Depending on whether the calling code needs this or not.
If dscan(buf, size) < size, the buffer is a string, i.e. it is properly null-terminated.
Calling isspace does not invoke undefined behavior. In your code, if I enter a negative char, the behavior is undefined. See the <ctype.h> documentation.


How optimize set of nested 'if' in one function? I have such part of function:

$token = $this->getToken($user);

    if ('main' !== session('type') && is_null($token)) {
        $instagramProfile = $this->findInstagramProfile($user);
        $token = $instagramProfile->profile->user->api_token ?? null;
    } else {
        if (!$instagramProfile = $this->findInstagramProfile($user)) {
            $profile = create_instagram_profile($user);
            $user = $this->createUser($profile);
            $token = $user->api_token;
        } else {
            if($instagramProfile->profile->user) {
                $token = $instagramProfile->profile->user->api_token;
            } else {
                $user = $this->createUser($instagramProfile->profile);
                $token = $user->api_token;
            }
        };
    }

I have many scenarios:

param type which save in session;
instagram profile exist;
user exists.

In worlds - very simple, but in code it stay not understandable.

How I can optimize such part of code?

Maybe create more abstract function?

I am not sure I understand what is supposed to happen in

    if ('main' !== session('type') && is_null($token)) {
        $instagramProfile = $this->findInstagramProfile($user);
        $token = $instagramProfile->profile->user->api_token ?? null;
    }

if by any chance $instagramProfile->profile->user happens to be null. Otherwise, refactoring is very straightforward. Just lift the common actions out of the if/else one by one:

    $instagramProfile = this->findInstagramProfile($user);

    if (!$instagramProfile) {
        $instagramProfile = create_instagram_profile($user);
    }

    user = $instagramProfile->profile->user;
    if (!user) {
        $user = $this->createUser($instagramProfile->profile);
    }

    $token = $user->api_token;

Observe that it is just a mechanical rewrite.

PS: I presume that create_instagram_profile can be safely called with null argument; you are doing it anyway.


Handling two buttons that submit a form using AJAX I am currently handling forms by binding submit event and then calling the ajax() function:

// Code to handle add post form

$('#add_post').on('submit', function (e) {
    e.preventDefault();

    $.ajax({
        type: 'POST',
        url: "add",
        dataType: 'json',
        data: $(this).serialize(),
        success: function (data) {},
        error: function (data) {}
   })

// Code to handle update post form

$('#edit_post').on('submit', function (e) {
    e.preventDefault();

    $.ajax({
        type: 'POST',
        url: "edit",
        dataType: 'json',
        data: $(this).serialize(),
        success: function (data) {},
        error: function (data) {}
   })

})

As you can see I have a lot repeating code. How can I refactor my code?

If it's the exact same call you could abstract the Ajax call into its own function and pass the url. For example:

const doAjax = (url) => {
    $.ajax({
        type: 'POST',
        url: url,
        dataType: 'json',
        data: $(this).serialize(),
        success: function (data) {},
        error: function (data) {}
    });
}

doAjax('add');
doAjax('edit');

This is an ES6 function expression but would also work with ES5 function(){} syntax.

function doAjax(url) {
    $.ajax({
        type: 'POST',
        url: url,
        dataType: 'json',
        data: $(this).serialize(),
        success: function (data) {},
        error: function (data) {}
    });
}