Proof that two lines intersect each other at precisely one point in Idris I recently attempted to prove that given two lines, they intersect at one and only one point in Idris. Here is what I came up with:

interface (Eq line, Eq point) => Plane line point where
  -- Abstract notion for saying three points lie on the same line.
  colinear : point -> point -> point -> Bool
  coplanar : point -> point -> point -> Bool
  contains : line -> point -> Bool

  -- Intersection between two lines
  intersects_at : line -> line -> point -> Bool

  -- If two lines l and m contain a point a, they intersect at that point.
  intersection_criterion : (l : line) ->
                           (m : line) ->
                           (a : point) ->
                           (contains l a = True) ->
                           (contains m a = True) ->
                           (intersects_at l m a = True)

  -- If l and m intersect at a point a, then they both contain a.
  intersection_result : (l : line) ->
                        (m : line) ->
                        (a : point) ->
                        (intersects_at l m a = True) ->
                        (contains l a = True, contains m a = True)

  -- For any two distinct points there is a line that contains them.
  line_contains_two_points : (a :point) ->
                             (b : point) ->
                             (a /= b) = True ->
                             (l : line ** (contains l a = True, contains l b = True ))

  -- If two points are contained by l and m then l = m
  two_pts_define_line : (l : line) ->
                        (m : line) ->
                        (a : point) ->
                        (b : point) ->
                        ((a /= b) = True) ->
                        contains l a = True ->
                        contains l b = True ->
                        contains m a = True ->
                        contains m b = True ->
                        ((l == m) = True)

  same_line_same_pts : (l : line) ->
                       (m : line) ->
                       (a : point) ->
                       (b : point) ->
                       ((l /= m) = True) ->
                       contains l a = True ->
                       contains l b = True ->
                       contains m a = True ->
                       contains m b = True ->
                       ((a == b) = True)

  -- There exists 3 non-colinear points.
  three_non_colinear_pts : (a : point ** b : point ** c : point **
                           (colinear a b c = False,
                           (a /= b) = True,
                           (b /= c) = True,
                           (a /= c) = True))

  -- Any line contains at least two points.
  contain_two_pts : (l : line) ->
                    (a : point ** b : point **
                    (contains l a = True, contains l b = True))

-- If two lines intersect at a point and they are not identical, that is the o-
-- nly point they intersect at.
intersect_at_most_one_point : Plane line point =>
  (l : line) -> (m : line) -> (a : point) -> (b : point) ->
  ((l /= m) = True) ->
  (intersects_at l m a = True) ->
  (intersects_at l m b = True) ->
  ((a == b) = True)

intersect_at_most_one_point l m a b l_not_m int_at_a int_at_b =
  same_line_same_pts
  l
  m
  a
  b
  l_not_m
  (fst (intersection_result l m a int_at_a))
  (fst (intersection_result l m b int_at_b))
  (snd (intersection_result l m a int_at_a))
  (snd (intersection_result l m b int_at_b))

My main concerns is that I have an "inefficient" (possibly incorrect?) formulation of the axioms. The code runs and compiles, but it feels like for instance, somehow, intersection_criterion and intersection_result could be somehow made into one axiom. Nevertheless, any advice is appreciated.

(This is also on Github here in the file hilbert.idr)

It still may be far from perfect, but I asked a question on Stackoverflow related to this, and received an excellent answer from xash.

I have attempted to cure my Boolean blindness. If I understand correctly, the operator == asks the question "Is the left hand side equal to the right hand side?" Instead, I want to say: "I have a proof that this holds."

To fix this, I changed a lot of my functions like colinear to more closely match the (=) signature:

colinear : point -> point -> point -> Bool

becomes:

Colinear : point -> point -> point -> Type

Also, I removed a /= b and instead replaced it with Not (a = b).

Here was the resulting code:

interface Plane line point where
  -- Abstract notion for saying three points lie on the same line.
  Colinear : point -> point -> point -> Type
  Coplanar : point -> point -> point -> Type
  Contains : line -> point -> Type

  -- Intersection between two lines
  IntersectsAt : line -> line -> point -> Type

  -- If two lines l and m contain a point a, they intersect at that point.
  intersection_criterion : (l : line) ->
                           (m : line) ->
                           (a : point) ->
                           Contains l a ->
                           Contains m a ->
                           IntersectsAt l m a

  -- If l and m intersect at a point a, then they both contain a.
  intersection_result : (l : line) ->
                        (m : line) ->
                        (a : point) ->
                        IntersectsAt l m a ->
                        (Contains l a, Contains m a)

  -- For any two distinct points there is a line that contains them.
  line_contains_two_points : (a : point) ->
                             (b : point) ->
                             Not (a = b) ->
                             (l : line ** (Contains l a, Contains l b))

  -- If two points are contained by l and m then l = m
  two_pts_define_line : (l : line) ->
                        (m : line) ->
                        (a : point) ->
                        (b : point) ->
                        Not (a = b) ->
                        Contains l a ->
                        Contains l b ->
                        Contains m a ->
                        Contains m b ->
                        (l = m)

  same_line_same_pts : (l : line) ->
                       (m : line) ->
                       (a : point) ->
                       (b : point) ->
                       Not (l = m) ->
                       Contains l a ->
                       Contains l b ->
                       Contains m a ->
                       Contains m b ->
                       (a = b)

  -- There exists 3 non-colinear points.
  three_non_colinear_pts : (a : point ** b : point ** c : point **
                           (colinear a b c = False,
                           Not (a = b),
                           Not (b = c),
                           Not (a = c)))

  -- Any line contains at least two points.
  contain_two_pts : (l : line) ->
                    (a : point ** b : point **
                    (Contains l a, Contains l b))

-- If two lines intersect at a point and they are not identical, that is the o-
-- nly point they intersect at.
intersect_at_most_one_point : Plane line point =>
  (l : line) -> (m : line) -> (a : point) -> (b : point) ->
  Not (l = m) ->
  IntersectsAt l m a ->
  IntersectsAt l m b ->
  (a = b)

intersect_at_most_one_point l m a b l_not_m int_at_a int_at_b =
  same_line_same_pts
  l
  m
  a
  b
  l_not_m
  (fst (intersection_result l m a int_at_a))
  (fst (intersection_result l m b int_at_b))
  (snd (intersection_result l m a int_at_a))
  (snd (intersection_result l m b int_at_b))


SQL query for adding column value to compare with other column I have two tables

table_inventory
    List item
    inventory_rack_key(primarykey)
    node_key
    rack_id
    inventory_item_key
    in_coming_qty,locked_qty
    quantity

table_item
    inventory_item_key(primary key)
    item_id,product_zone

The table example are provided here DB TABLES
I need query to find out those items for which (net_qty) i.e difference b/w sum of in_coming_qty & quantity & locked_qty is negative. arranged by node_key,rack_id, item_id,net_qty

Note: each distinct set {node_key,rack_id, item_id,net_qty} will have only 1 row in output.
For ex :{node_key,rack_id, item_id} = {ABD101,RK-01,562879} has 4 rows in table_inventory
but in output net_qty= -78(single row) .

The query I made is giving me result but can we do it in some other way?

SELECT l.node_key,
       l.rack_id,
       i.item_id,
       ( SUM(l.quantity + l.in_coming_qty) - SUM(l.locked_qty) ) AS net_qty
FROM   table_inventory l,
       table_item i
WHERE  l.inventory_item_key = i.inventory_item_key
GROUP  BY l.node_key,
          l.rack_id,
          i.item_id
HAVING SUM(l.quantity + l.in_coming_qty) - SUM(l.locked_qty) < 0

I am not exactly familiar with Oracle syntax, but I know that Oracle supports indexes on functions.  I'm not sure if it will be used, since there is grouping involved, but you may wish to look into creating an index for this query if performance of the query becomes an issue.  A guess at the syntax used to create the index would be:

Create index net_qty on table_inventory(SUM(quantity + in_coming_qty) - SUM(locked_qty));

You can search for "Oracle function based index" for more information.


WCF wrapper implementing dispose-finalize pattern I tried to implement the dispose-finalize pattern on this WCF wrapper.

Do you have any comments about this? Something I missed?

namespace System.ServiceModel
{
    public class WCFClientWrapper<TProxy> : IDisposable where TProxy : class
    {
        private string _endpointAddress;
        public TProxy Service { get; private set; }

        public WCFClientWrapper(string endpointAddress)
        {
            _endpointAddress = endpointAddress;

            var factory = new ChannelFactory<TProxy>(new NetTcpBinding(), new EndpointAddress(endpointAddress));
            Service = factory.CreateChannel();

            (Service as IClientChannel).Faulted += WCFClientWrapper_Faulted;
        }

        private TProxy CreateChannel(string endpointAddress)
        {
            var factory = new ChannelFactory<TProxy>(new NetTcpBinding(), new EndpointAddress(endpointAddress));
            return factory.CreateChannel();
        }

        void WCFClientWrapper_Faulted(object sender, EventArgs e)
        {
            DisposeService();
            Service = CreateChannel(_endpointAddress);
        }

        public void Dispose(bool disposing)
        {
            if (disposing)
            {
                // get rid of managed resources
            }

            DisposeService();
            GC.SuppressFinalize(this);
        }

        private void DisposeService()
        {
            IClientChannel serviceChannel = Service as IClientChannel;
            if (serviceChannel == null)
                return;

            bool success = false;
            try
            {
                (Service as IClientChannel).Faulted -= WCFClientWrapper_Faulted;

                if (serviceChannel.State != CommunicationState.Faulted)
                {
                    serviceChannel.Close();
                    success = true;
                }
            }
            finally
            {
                if (!success)
                {
                    serviceChannel.Abort();
                }
            }
        }

        public void Dispose()
        {
            Dispose(true);
        }

        ~WCFClientWrapper()
        {
            Dispose(false);
        }
    }
}

Given that much of the code seems to rely on Service being an IClientChannel, and the name of the class implies as such, it may be worth simply adding IClientChannel to your type constraints and remove all the type conversion code.

Building upon that thought, I notice that IClientChannel extends IChannel, and everything you use seems to be present on IChannel.  You may consider using that interface instead, and renaming your class to reflect that it is not limited to IClientChannel.


Merging client names into orders using RxJS I have an API which I can't modify. I have to use data as it is.

I want to join data from two separate requests where the second request is based on data from the first one.

I have a working solution already but the code quality, in my opinion, is low so I want to know your opinion (and if you can provide: better, more reactive solution).

So, I have an order API that's receiving data:

/orders response:

[
  {
    id: 1,
    clientId: 201,
    // other data
  },
  {
    id: 2,
    clientId: 201,
  },
  {
    id: 3,
    clientId: 11,
  }
]

In this response, I don't have data about the client name, so I have a second API where I can pass IDs of clients and then I got back:

/clientsData?ids=201,11 response:

[
  {
    id:201,
    name: "Jon Doe"
  },
  {
    id: 11,
    name: "Jack Sparrow"
  }
]

So, now I have a working code but as mentioned, in the beginning, I'm feeling it can be done in a better way. Please, note that the request should be done in the correct order because I need clients IDs before I make a second API call.

Also, want to avoid making multiple (more than two [orders+clients]) requests to check client data.

Working code:

// API Call to get orders:
const result$ = this.http.get('/orders').pipe(
  mergeMap(
    orders => {
      const ids = orders
        .map(item => item.clientId)
        .filter((v, i, a) => a.indexOf(v) === i) // remove duplicates
        .join(','); // get client ids into one string

      // Client IDs: 1,2,99,201
      // API call to check IDs:
      return this.http.get(`/clientsData?ids=${ids}`).pipe(
        mergeMap(clientsData => {
          // map over each client
          clientsData.forEach(client => {
            // then map over each order
            orders.forEach(order => {
              // and assign client name to order
              if (order.clientId === client.id) {
                order.clientName = client.name;
              }
            });
          });

          return of(orders); // observable
        })
      );
    }
  )
);

result$.subscribe(console.log);

I expect to get back Observable with merged data:

[
  {
    id: 1,
    clientId: 201,
    name: "Jon Doe"
    // other data
  },
  {
    id: 2,
    clientId: 201,
    name: "Jon Doe"
  },
  {
    id: 3,
    clientId: 11,
    name: "Jack Sparrow"
  }
]

I want to avoid nested subscriptions and achieve the result in a reactive way.

Looks good. Yeah, sometimes APIs are hard to work with.

Instead of the loop clientsData.forEach, you might want to reduce the client data down to an object, where the keys are the client id (ie. index on the id). Then, map through your orders, and use this object to add in the client name. This will be O(N) vs. O(N^2)

I'd also probably use a different object for orders vs. the orders with the name... just to avoid mutation, which I didn't expect... This is obviously more a style question, though. I favor immutable code, especially when it's more functional like this.


Binary Search Python Tried to make a binary search algorithm, how did I do?

def homogeneous_type(list1):
    i_list = iter(list1)
    listtype = type(next(i_list))
    return True if all(isinstance(x, listtype) for x in i_list) else False

def binSearch(list1, value):

    if not list1:
        raise ValueError("list is empty")
    if not homogeneous_type(list1):
        raise ValueError("list must be one type")

    left = 0
    right = len(list1) - 1
    list1 = sorted(list1)

    while left <= right:

        midIndex, mod = divmod(left + right, 2)
        midIndex = midIndex + 1 if mod else midIndex

        if list1[midIndex] == value:
            return midIndex
        elif list1[midIndex] < value:
            left = midIndex + 1
        elif list1[midIndex] > value:
            right = midIndex - 1

    raise ValueError("value not in List")

There are no docstrings. What do homogeneous_type and binSearch do?
In Python we rarely need to insist that lists be homogeneous. It often makes sense to have heterogeneous lists, provided that all elements support the required operations, for example it can be reasonable to have a list containing a mixture of int and float.
Checking that the list is homogeneous requires looking at every value to check that it is the required type. But if you are going to look at every value, you don't gain anything by doing a binary search, you might as well just call list1.index(value).
Similarly, sorting the list requires looking at every element, so again, there is no point in sorting and then doing a binary search. Skipping the sort and calling list1.index(value) would be faster.
The binary search code maintains a closed interval left <= i <= right that contains the index of the value being searched for. But maintaining a closed interval requires some complexity when computing the middle index:

midIndex, mod = divmod(left + right, 2)
midIndex = midIndex + 1 if mod else midIndex

It would be simpler to maintain a half-open interval left <= i < right by starting with right = len(list1). Then the computation of the midpoint is simple:

midIndex = (left + right) // 2

(In the half-open version of the code the loop condition needs to be left < right rather than left <= right, and you need to assign right = midIndex rather than right = midIndex - 1.)
On each iteration of the binary search loop there are three comparisons:

list1[midIndex] == value:
list1[midIndex] < value:
list1[midIndex] > value:

This can be reduced to one comparison per iteration, like this:

left, right = 0, len(list1)
while left < right:
    mid = (left + right) // 2
    if value < list1[mid]:
        right = mid
    else:
        left = mid + 1
if left < len(list1) and value == list1[left]:
    return left
else:
    raise ValueError("not found")

Note that this version of the code works fine even if list1 is the empty list, allowing you to remove the special case for if not list:.
Binary search is built into Python: see the bisect module.


What's the most optimal way to see if a string is included in a list of strings? Something that has bugged me for a while, is what way is best to perform a conditional where a string is compared to other strings.

Here is some sample code.  This code is just for illustration purposes.

The 3 methods I use are

Do a word for word comparison.  This is probably the most optimal, but ugliest code
Create an array on the fly and do a comparison
Have a predefined array.  This is probably better placed in an external file for easier maintainability.

I just wanted to see what your takes are on this, and if there is an even better way to do it.  I know Enums would probably work well here as well.

    private bool IsWordBad(string word)
    {
        //First Method
        if (word == "Foo" || word == "Bar" || word == "FooBar")
        {
            return true;
        }

        //Second Method
        if ((new[] { "Foo", "Bar", "FooBar" }).Contains(word))
        {
            return true;
        }

        //Third Method
        var badWords = new[] { "Foo", "Bar", "FooBar" };
        if (badWords.Contains(word))
        {
            return true;
        }
        return false;
    }

If you really have just three words, then the performance most likely won't matter. In other words, unless profiling shows that this method is performance-critical, use the method that you find most readable.

Some other options you could consider:

Using a switch:

switch (word)
{
    case "Foo":
    case "Bar":
    case "FooBar":
        return true;
    default:
        return false;
}

This has the advantage that it can use compiler optimizations. (Under certain circumstances, the compiler will turn such switch into a Dictionary lookup.)
Use a HashSet<string> instead of an array. Looking up in a hash set is O(1), but O(n) for an array. This could make sense if you had large number of strings to search for.
Use a trie. This has the advantage of comparing only relevant parts of the input string. This could make sense if you had long input strings or if you huge number of strings to search for.


Financial report query I have been tasked with creating financial reports. I am looking for any suggestions on improving performance, readability/formatting  or best practices.

/*setting dummy parameters */
SET DATEFORMAT dmy
DECLARE @startProduct NVARCHAR = ''
DECLARE @endProduct NVARCHAR = 'zzzzzzzzzzzzzzzzzzzzzz'
DECLARE @startGroup NVARCHAR = ''
DECLARE @endGroup NVARCHAR = 'zzzzzzzzzzzzzzzzzz'
DECLARE @startDepartment NVARCHAR = ''
DECLARE @endDepartment NVARCHAR = 'zzzzzzzzzzzzzzzzzzzzzzz'
DECLARE @startDate DATE = '01/05/2000'
DECLARE @endDate DATE = CAST('31/05/2012' AS DATE)
DECLARE @StartTime TIME  = '08:00:00'
DECLARE @endTime TIME = '17:00:00'
DECLARE @grouping INT = 0
DECLARE @site  INT = 21
/*
ProductCode = 0
ProductGroup = 1
Department = 2
Pump = 3
Date = 4
*/

SELECT
     CASE @grouping
        WHEN 0 THEN P.Code
        WHEN 1 THEN P.[Group]
        WHEN 2 THEN P.Department
        WHEN 3 THEN P.[Description]
        WHEN 4 THEN CONVERT(VARCHAR,RPS.SalesDateTime,103)
    END AS GroupCode,
    CASE @grouping
        WHEN 0 THEN P.[Description]
        WHEN 1 THEN PG.[Description]
        WHEN 2 THEN PD.[Description]
        WHEN 3 THEN 'PUMP: ' + CONVERT(VARCHAR,RPS.PumpId) +' HOSE: ' + CONVERT(VARCHAR,RPS.HoseId)
        WHEN 4 THEN CONVERT(VARCHAR(MAX),CAST(FLOOR(CAST(RPS.SalesDateTime AS FLOAT)) AS DATETIME))
    END AS [GroupDescription],
    SUM(RPS.Quantity) AS Quantity,
    SUM( CASE
            WHEN P.GSTable = 1 THEN  RPS.TotalAmount / 1.1
            ELSE RPS.TotalAmount
         END) AS AmountX,
    SUM(CASE
            WHEN P.GSTable = 1 THEN (rps.TotalAmount * .10)
            ELSE 0 END
        ) AS GST,
    SUM(RPS.UnitLastCost * RPS.Quantity) AS CostXgst,
    SUM(RPS.TotalAmount) AS AmountInc,
    FLOOR(ROUND(((1 -
        (
        SUM(RPS.UnitLastCost * RPS.Quantity)
            /
            NULLIF(SUM
            (
                CASE
                    WHEN P.GSTable = 1 THEN  RPS.TotalAmount / 1.1
                    ELSE RPS.TotalAmount
                END
            ),0))) * 100 ),0))

          AS Margin
FROM
    RetailPosSales RPS
        JOIN Products P ON RPS.ProductCode = P.Code
        LEFT OUTER JOIN ProductGroups PG ON P.[Group] = PG.Code /*need this if the grouping is 1*/
        LEFT OUTER JOIN ProductDepartments PD ON P.Department = PD.Code  /*need this if the grouping is 2 */
WHERE
    ((@grouping <> 3) OR (@grouping = 3 AND RPS.IsFuelSale = 1)) /*When grouping by pump, only allow fuel products */
    AND P.Code BETWEEN @startProduct AND @endProduct
    AND P.[Group] BETWEEN @startGroup AND @endGroup
    AND P.Department BETWEEN @startDepartment AND @endDepartment
    AND CAST(RPS.SalesDateTime AS DATE) BETWEEN @startDate AND @endDate
    AND CAST(RPS.SalesDateTime AS TIME) BETWEEN @StartTime AND @endTime
    AND RPS.SiteId = @site
GROUP BY
    CASE @grouping
        WHEN 0 THEN P.Code
        WHEN 1 THEN P.[Group]
        WHEN 2 THEN P.Department
        WHEN 3 THEN P.[Description]
        WHEN 4 THEN CONVERT(VARCHAR,RPS.SalesDateTime,103)
    END,
    CASE @grouping
        WHEN 0 THEN P.[Description]
        WHEN 1 THEN PG.[Description]
        WHEN 2 THEN PD.[Description]
        WHEN 3 THEN 'PUMP: ' + CONVERT(VARCHAR,RPS.PumpId) +' HOSE: ' + CONVERT(VARCHAR,RPS.HoseId)
        WHEN 4 THEN CONVERT(VARCHAR(MAX),CAST(FLOOR(CAST(RPS.SalesDateTime AS FLOAT)) AS DATETIME))
    END
ORDER BY
    GroupCode,
    GroupDescription

It appears that you're using the same expressions for the GroupCode and GroupDescription columns in the GROUP BY clause.  That's asking for trouble later on if someone needs to modify either expression, especially given their size.  Instead, use a subquery:

SELECT
    GroupCode,
    GroupDescription,
    -- etc
FROM
    (
        SELECT
            CASE /* etc */ END AS GroupCode,
            CASE /* etc */ END AS GroupDescription,
            -- etc
        FROM
            -- etc
        WHERE
            -- etc
    ) AS dummy
GROUP BY
    GroupCode,
    GroupDescription
ORDER BY
    GroupCode,
    GroupDescription

EDIT:

There may be some other opportunities to use subqueries to reduce duplication of expressions.  For example, the expression for the column AmountX is also used in the Margin calculation.


Binary search tree in Python Create a binary search tree representation of a given array of integers.

How can I improve this code?

# Creating Node object class
class Node(object):
    def __init__(self, value, left=None, right=None):
        self.value = value
        self.left = left
        self.right = right

# Creating Tree class
class BST(object):
    def __init__(self, val):
        self.root = Node(val)

    def insert(self, value):
        current = self.root
        while current:
            if value < current.value:
                if current.left == None:
                    current.left = Node(value)
                    break
                else:
                    current = current.left
            else:
                if current.right == None:
                    current.right = Node(value)
                    break
                else:
                    current = current.right

# inputs
t = BST(22)
t.insert(30)
t.insert(10)
t.insert(80)
t.insert(90)
t.insert(9)
t.insert(23)
t.insert(14)
t.insert(6)
t.insert(40)
t.insert(60)
t.insert(3)

You can also consider a very elegant, recursive solution:

def insert(self, value, node=self.root):
   if node == null:
      return Node(value)
   if value < node.value:
      node.left = self.insert(value, node.left)
   else
      node.right = self.insert(value, node.right)

Although, please mind that in case of large trees, efficiency of that solution is slightly lower (because of call stack)


Find the number of substrings of a numerical string greater than a given num string Question:
http://qa.geeksforgeeks.org/9744/vinay-queried-interesting-problem-optimization

  Input

  The first line contains N denoting the length of Target String. This
  line is followed by the Target String. The Target String is followed
  by an integer Q denoting the number of queries asked. After this Q
  lines follow each line containing a Query String.

  Output

  For each query output the total number of distinct Cruel Pairs that
  satisfies the given conditions.

  Constraints

  1 ≤ N ≤ 5000
  1 ≤ Q ≤ 104
  Sum of Lengths of all Query Strings ≤ 106
  All strings are composed only of '0' - '9'

  Sample Input

5
75201
5
78945884875
22
00048
77
501

  Sample Output

0
8
8
6
5

  Explanation

  Query String 1 : No such Pair exist.

  Query String 2 : Pairs are (1,2) , (1,3) , (1,4) , (1,5) , (2,3) ,
  (2,4) , (2,5) , (3,5)

  Query String 5 : Pairs are (1,3) , (1,4) , (1,5) , (2,4) , (2,5)

  Time Limit:1.0 sec(s) for each input file. 5s for python

  Memory Limit:256 MB

  Source Limit:1024 KB

Please review the code and help me with optimization

N = int(raw_input())

target = str(int(raw_input()))

N = len(target)

Q_N = int(raw_input())

queries_list = []
output = []

for x in xrange(Q_N):
    queries_list.append(raw_input())

for x in queries_list:
    d = int(str(int(x))[0])
    x = str(int(x))
    q_l = len(x)
    if q_l > N:
        print 0
        continue
    count = 0
    for i, t in enumerate(target):
        t = int(t)
        temp = 0
        if t > d:
            temp = (N - i) - q_l + 1
        elif t > 0 and t < d:
            temp = (N - i) - q_l
        elif t == d:
            temp = (N - i) - q_l
            if int(target[i:i + q_l]) > int(x):
                temp = temp + 1
        if temp > 0:
            count = count + temp

    print count

Since I misunderstood the question. I offer up a second take. I have not convinced myself that log10 will increase the efficacy of the script but I use it anyway to show the concept of using math instead of len on strings.

The code has two generators, one to truncate the number from the left, or moving the X coordinate of the cruel pair. Another generator to move the Y coordinate.

The generator that moves the Y coordinate starts at the same length as the query string is. If the query string is longer it yields 0.

The main function relies on the fact that if the condition are meet, all following conditions in the current substring are meet and we and we can simply add those. This means that the only check done is the check where the is of equal length + 1.

from math import log10

def iterate_number_ints(num):
    length = int(log10(num))+1
    for i in range(length-1):
        yield num%(10**(length))
        if num // 10 ** (length - 2) % 10 == 0:
            yield 0
        length -= 1

def iterate_sub_number(num, lower_limit):
    if num > 0:
        length = int(log10(num))
        lower_limit_length = int(log10(lower_limit+1))
        for n in range(lower_limit_length, length+1):
            yield num//(10**(length-n))
    yield 0

def main():

    queries = [int(x) for x in ["78945884875", "22", "00048", "77", "501"]]
    target = 75201

    for k in range(len(queries)):
        ans = 0
        for t in iterate_number_ints(target):
            if t == 0:
                continue
            length_t = int(log10(t))
            for l in iterate_sub_number(t, queries[k]):
                if l == 0:
                    continue
                length_l = int(log10(l))
                if l > queries[k]:
                    ans += length_t-length_l+1
                    break

        print(queries[k], ans)

if __name__ == '__main__':
    main()

It might not be a complete solution, i can't test it, but it reduces the numbers of cases that are checked. But i should give some pointers.


php swiftmailer two message instances with one mailer instance? I created a simple function for swiftmailer to send two different messages for support and client. Not sure i did it the rigth way as I am using two different instances for message and one mailer instance. Everything works just fine but am I doing it the right way or I have to create separate mailer instance for each message? Here is my code, if someone could point out if it is ok or could be done better:

<?php

if ((isset($_POST['email'])) && (strlen(trim($_POST['email'])) > 0)) {
    $email = stripslashes(strip_tags($_POST['email']));
} else {$email = 'No data';}

require_once '/home/sitename/public_html/inc/swift/lib/swift_required.php';

function send_email($info){

    $transport = Swift_MailTransport::newInstance();

    $mailer = Swift_Mailer::newInstance($transport);

    //Create the message for support
    $message = Swift_Message::newInstance()
        ->setSubject('Subject 1')
        ->setFrom(array('info@domain.com' => 'Info'))
        ->setTo(array(
            'name1@domain.com',
            'name2@domain.com',
        ))
        ->setBody($info['message_support'],'text/html');

    //Create the message for client
    $message2 = Swift_Message::newInstance()
        ->setSubject('Subject 2')
        ->setFrom(array('info@domain.com' => 'Info'))
        ->setTo(array(
            $info['email']
        ))
        ->setBody($info['message_client'],'text/html');

    if ($mailer->send($message) && $mailer->send($message2))
        return true;
    else
        return false;
}

$info = array(
    'email' => $email,
    'message_support' => '
Dear Support

Hello World
',
    'message_client' => '
Dear Client

Hello World
'
);

if (send_email($info))
{
  header("Location:/thank-you");
}
else
{
  header("Location:/error");
}

?>

Note: this is intended to be a Community wiki since it was taken from a deleted answer by yannis; The suggestion to do this was here

Using two message instances is fine as is most of your code. There is one major issue though, you don't validate the email, you just check if the $_POST variable was set and is not empty but you don't check if the email format is "name@domain.tld". The simplest thing you could do is:
$email =
    isset($_POST["email"]) && !empty($_POST["email"])
    ? filter_var($email, filter_var($_POST["email"], FILTER_SANITIZE_EMAIL) )
    : "No data";

The filter extension greatly helps you sanitize and validate variables. It's on by default since PHP 5.2, if you are working on an earlier version, you can substitute it with an email validation regular expression. By the way, if you are unfamiliar with the conditional format I'm using, it's called ternary operator and it goes something like this:
$variable =
    ( condition )
    ? ( value if condition is true )
    : ( value if condition is false );

When actually sending the email, you should do:
if( $email != "No Data" && send_email($info) ) {
  header("Location:/thank-you");
} else {
  header("Location:/error");
}

That way and since the check $email != "No Data" is first, send_email($info) will never execute for a missing or invalid email. So the 2 instances of a hefty library will never instantiate needlessly.
Lastly, a very minor change, that might be considered a matter of style. Here:
if ($mailer->send($message) && $mailer->send($message2))
    return true;
else
    return false;

You are returning a boolean value based on the boolean return of $mailer->send($message) && $mailer->send($message2). You can simply rewrite as:
return $mailer->send($message) && $mailer->send($message2);


Check if given integer is a palindrome This code checks if the given integer is a palindrome. If it is, prints given integer is a palindrome else, not a palindrome:

from collections import deque
def polindrome(input):
 d = deque()
 d.extendleft(str(input))
 result = int(''.join(d))
 if input == result :
     return input , 'is a polindrome'
 else :
     return (input , 'is not a polindrome')

There's a much easier way to do this. Python slicing allows you to reverse a string with s[::-1]. So you can just do:

def is_palindrome(s):
    return s == s[::-1]

To make this work for integers, you can just str() the int.

Why would you want to do it this way? For one, you're already converting to a string. So this approach shouldn't have any worse performance characteristics. Your use of a deque here is probably a bit overkill. deques make sense when you have lots of prepending to lists. Here, your numbers aren't likely to be that many digits (even though integers in python can be arbitrarily large).  And then on top of this you then convert back to an int, which is an unnecessary step (as you could just compare the strings) and could fail (although it shouldn't).

And beyond the performance, using the deque (and converting back to an int) makes it more difficult to understand what your code is doing. s[::-1] is a well understood idiom that clearly conveys its purpose in context (s == s[::-1]).

I wouldn't return a tuple from the function. It's much more informative to return a bool and then use that to print whatever string you want:

if is_palindrome(str(num)):
    print(f'{num} is a palindrome')
else:
    print(f'{num} is not a palindrome')

You should also look into PEP8 to make your code look more consistent with Python's widely accepted style guide.

Looking at the performance:

$ python3 -m timeit -s 'from funcs import polindrome' 'polindrome(123456654321)'
200000 loops, best of 5: 1.26 usec per loop

$ python3 -m timeit -s 'from funcs import is_palindrome' 'is_palindrome(str(123456654321))'
500000 loops, best of 5: 435 nsec per loop

is_palindrome is about 2.8 times faster.

You may also want to write some unit tests as there are some strange cases that your code may not handle correctly (eg. should 120 be a palindrome of 21 since 021 == 21 or not since they are different lengths?)


Program for binary to decimal and vice versa I started to learn C a week ago and this is my culminating program of what I've learned but I could use a few pointers.

#include <math.h>
#include <stdio.h>
void binary(int n){
    int x = 0, m = 0, z = n;
    while(n >= 1){
            ++x;
            n /= 10;
            }
    n = z;
    int i[x];
    x = 0;
    while (n != 0){
            ++x;
            i[x] = n % 10;
            n /= 10;
            }
    while(x >= 0){
            if (i[x] == 1){
                    if (x == 1)
                            ++m;
                    else m += pow(2,x - 1);
            }
            --x;
    }
    printf("%d", m);
}
void convert(int n){
    int i[16];
    int j, k, p;
    for(k = 15; k >= 0; --k){
            i[k] = n % 2;
            n /= 2;
            }
    int *a, *b;
    a = &i[0];
    b = &i[16];
    for( ;*a == 0; ++a)
            ;
    while(a != b){
            printf("%d", *a);
            ++a;
    }
}
int main()
{
    int n;
    char m;
    printf("\nType b for binary to decimal conversion or d for decimal to binary conversion (max 65535)\n");
    m = getchar();
    printf("\nPrint number to be converted\n");
    scanf("%d", &n);
    if (m == 'b'){
            printf("%d converted to a decimal number is ", n);
            binary(n);
    }
            else{
                    printf("%d converted to a binary number is ", n);
                    convert(n);
            }
}

two sets of comments. IS the functionality sensible or not? Is the code good / open to improvement etc?

Firstly the functionality

Its very odd to put 101 into an int and say that its 5. The number 5 in an int is always 5, it has various string representations depending on the base used to display it. I would have much preferred seeing 2 functions:

one that accepts a string is a given base and gives that returns the valur of it as an int
one that takes an int and displays it as a string using a given base

In effect you would be recoding atoi and itoa

Now about the implementation of the features you implemented

First the code works, ++ for that. Nicely split into functions etc. ++

No Comments -- Gotta have at least some.

commonly accepted c practice is to define and initialize variables on first use. (Old compilers required all variables to be declared at start of scope, not any more)

for exmaple

   char m;
    printf("\nType b for binary to decimal conversion or d for decimal to binary conversion (max 65535)\n");
    m = getchar();

is better as

printf("\nType b for binary to decimal conversion or d for decimal to binary conversion (max 65535)\n");
char m = getchar();

You alignment is somewhat funky. Commonly accepted c styles are

while(n >= 1)
{
    ++x;
    n /= 10;
}

or

while(n >= 1){
   ++x;
   n /= 10;
}

your should '\n' on the last line of your output, its considered polite.

I would have made the conversion functions return something rather than having them do the printing. Imagine you needed to do something else with the converted value

There is no error checking. Entering 42 and asking for a 'b' conversion returns 0. THis is in fact an error, you should say so.

n,z,x are not obvious names. I see that z is a copy of n so you can restore it later, call it saven or something like that

Now for the actual logic. Given that you are doing such a strange thing I find it hard to really comment on it. But its not very clear. Taking convert, I would have converted the input to a string , base 2, then scanned the string back as decimal.


jQuery to construct elements I'm new to jQuery, and am using it to dynamically construct chunks of a page.

The created HTML is what I want, but I'm not sure if the way I'm generating it is sensible or idiomatic.

function setupIndex()
{
  $(document.body).append($("<img/>", {src : 'logo.png'}));
  var div = $("<div/>", {"class" : "content"}).appendTo(document.body);

  div.append("<h2/>", { text :'My Panels'});
  div.append($("<table/>", {id : 'maintable'}));
}

function addPanel(name, desc)
{
  if (!$("#maintable"))
    setupIndex();

  var row = $("<tr/>",{"class":"panels"}).appendTo("#maintable");

  var div = $("<td/>").appendTo(row);
  div.append($("<h3/>").append($("<a/>", { text : name, href : name + "/index.html"})));
  div.append(document.createTextNode(desc));

  div = $("<td/>").appendTo(row);

  var anc = div.append($("<a/>", {href: name + "/index.html"}));
  anc.append($("<img/>", { css : {width : "200px", height : "auto"}, src : name + "/thumbnail.png"}));
}

I know that the following might seem crazy at first but humour me for a minute.  The main improvement is so that variables are not reused (which confuses their meaning) and the subtle .appendTo("#maintable") is at the end (which means the document is altered once).

There is not functional need to store the objects into variables as they aren't re-used. I often try to write in this pattern as it looks more like the way you'd write html.

if (!$("#maintable"))
    setupIndex();

$("<tr/>", {"class":"panels"})
    .append(
        $("<td/>")
            .text(desc)
            .prepend(
                $("<h3/>")
                    .append($("<a/>", { text : name, href : name + "/index.html"})))
            )
    )
    .append(
        $("<td/>")
            .append($("<a/>", {href: name + "/index.html"}))
            .append($("<img/>", { css : {width : "200px", height : "auto"}, src : name + "/thumbnail.png"}))
    )
    .appendTo("#maintable");


Validator and Sanitizer for HTML 5 attribute regex according to current HTML living standard According to https://html.spec.whatwg.org/multipage/syntax.html#attributes-2 an HTML 5 attribute name is defined like this:

Attribute names must consist of one or more characters other than
controls, U+0020 SPACE, U+0022 ("), U+0027 ('), U+003E (>), U+002F
(/), U+003D (=), and noncharacters. In the HTML syntax, attribute
names, even those for foreign elements, may be written with any mix of
ASCII lower and ASCII upper alphas.

Creating a class which can handle or sanitize HTML 5 attribute names I have ended up with the following code - especially the following regex:
class AttributeNameValidator
{
    public const ATTRIBUTE_NAME_MATCHER = "/[\s\x{0000}\x{0020}\x{0022}\x{0027}\x{003E}\x{002F}\x{003D}\x{200B}-\x{200D}\x{FDD0}-\x{FDEF}\x{FEFF}[:cntrl:]]+/u";

    /**
     * Checks if a given string is a valid HTML attribute name.
     * @param string $attributeName
     * @return bool: True if the given attribute name is a valid HTML attribute name.
     */
    public static function isAttributeNameValid(string $attributeName): bool
    {
        return (bool)preg_match(self::ATTRIBUTE_NAME_MATCHER, $attributeName);
    }

    /**
     * Sanitizes a string to be a valid HTML5 attribute name.
     * @param string $attributeName
     * @return string
     * @throws NonSanitizeableException
     */
    public static function sanitizeAttributeName(string $attributeName): string
    {
        $sanitizedAttributeName = preg_replace(self::ATTRIBUTE_NAME_MATCHER, '', $attributeName);
        if(!$sanitizedAttributeName) {
            throw new NonSanitizeableException("Failed to sanitize attribute name");
        }
        return $sanitizedAttributeName;
    }
}

My manual tests seem to work well still I am not sure if the regex exactly matches the standard or if I have forgotten something. Is there still something to improve?

The \x{0020} (SPACE) character is already included in \s, you can omit that part.
I prefer to use  \p{Cc} over [:cntrl:] for consistency and because I don't like to have nested square braces in a character class.
I expect isAttributeNameValid() to be checking that the whole string contains NO blacklisted characters.  If you want to match the entire string then you will need "start of string" and "end of string" anchors and a negated character class in your pattern.  But wait, you have a regular character class and you are returning true if one or more blacklisted characters are in the string -- this seems precisely the opposite of what the method name suggests.  Unless I am confused, you should replace (bool) with ! so that you return false when a match is made and true when there are no blacklisted characters found.
I don't know if I agree with the wording of Failed to sanitize attribute name; it seems more truthful to say that the Attribute name had no salvagable characters.


Payment system for a web shop I've tried making a payment system for an API called Payson, which is a European company similar to PayPal.

I get the right response when I press the button so it seems to work, and I wonder if you can look at my code to see if it looks OK. I didn't test the part receive but the part send i.e. the first part I've testet and it appears to work:

class PaysonHandler(webapp2.RequestHandler):

    def get(self):
        """
............Returns a simple HTML form for Payson
........"""
        logging.info('in payson')
        SellerEmail = 'niklas...@gmail.com'
        Cost = 250
        ExtraCost=0
        GuaranteeOffered=2
        OkUrl = self.request.host+"/payson_okurl" # TO DO
        Key = '319033-6152-402-b95a-37430de6b6'
        text = SellerEmail + ':' + str(Cost) + ':' + str(ExtraCost) + ':' + OkUrl + ':' + str(GuaranteeOffered) + Key
        logging.info('mdtext5: '+text)
        m = hashlib.md5()
        Generated_MD5_Hash_Value = hashlib.md5(text).hexdigest()
        BuyerEmail = 'niklas@.....se'
        AgentID = 11366
        path = os.path.join(os.path.dirname(__file__), 'templates',
                            'payson.html')
        self.response.out.write(template.render(path, {
            'SellerEmail':SellerEmail, 'Cost':Cost,  'Cost':Cost, 'AgentID':AgentID,
                  'ExtraCost':ExtraCost, 'GuaranteeOffered':GuaranteeOffered,  'OkUrl':OkUrl, 'Key':Key,  'Generated_MD5_Hash_Value':Generated_MD5_Hash_Value,  'BuyerEmail':BuyerEmail,    }))

class PaysonReceiveHandler(webapp2.RequestHandler):

    def get(self):
        """
............Receives Payson messages. Not tested
........"""
        logging.info('in payson')
        SellerEmail = 'niklas....@gmail.com'
        Cost = 10
        ExtraCost=0
        GuaranteeOffered=2
        OkUrl = self.request.host+'/payson_okurl' # TO DO
        text = SellerEmail + ':' + str(Cost) + ':' + str(ExtraCost) + ':' + OkUrl + ':' + str(GuaranteeOffered) + Key
        logging.info('mdtext5: '+text)
        m = hashlib.md5()
        Generated_MD5_Hash_Value = hashlib.md5(text).hexdigest()
        AgentID = 11366

        strYourSecretKey = "3190fb33-6152-4052-b95a-379b430de6b6";
        strOkURL = self.request.GET.get('OkURL')
        strRefNr = self.request.GET.get('RefNr')
        strPaysonRef = self.request.GET.get('Paysonref')

        strTestMD5String = strOkURL + strPaysonRef + strYourSecretKey
        strMD5Hash = hashlib.md5(strTestMD5String).hexdigest()

        logging.info('testing MD5 value')
        if strMD5Hash == self.request.GET.get('MD5'):
            logging.info('Korrekt anrop från Payson')
            logging.info('Rätt!')
            logging.info('OkUrl: ') + strOkURL
            logging.info('RefNr: ') + strRefNr
            logging.info('PaysonRef: ') + strPaysonRef
        else:
            #Felaktigt anrop från någon annan.
            logging.info("Felaktig MD5-summa!")

First of all, please make sure that your indentation is correct, especially when posting Python code.  Also, take a look at PEP 8.   You are violating it in at least the following places:

You have tabs, not spaces
Comments should be in English
It is apparently not directly against PEP 8, but giving locals CapsWords names makes the code harder to understand, in my opinion (especially seeing as SE highlights them a different colour).

You also seem to have an awful lot of duplicate code.  Try to factor common operations (such as setting up all those members) into separate functions.

Speaking of which, your functions aren't particularly clear.  Why does a method called get not return anything?  What is it supposed to get?

You've got plenty of hard-coded values in there.  I hope that strYourSecretKey is a placeholder value; the other ones should be passed in as parameters.  Could you make it determine the agent ID based on the email?  As things are, it seems like this is prone to incorrect combinations of input if anything is changed.


Return most common items in list I was wondering about the best way to return the most common items in a list.  So far the best way I could do it was:

//@param string $this->matched_ids_list = 1,11,12,11,12,
$this->matched_ids_list = "," . $this->matched_ids_list;
$this->ids_count = explode(",", $this->matched_ids_list);
$this->ids_count = array_unique($this->ids_count); //See below why this is done
foreach ($this->ids_count as $this->id) {
    $this->appearences[$this->id] = substr_count($this->matched_ids_list, ",{$this->id},");
    //[1] => 1, [11] => 2, [12] => 2
}
$this->most_appearences = max($this->appearences); //2
foreach ($this->appearences as $this->id => $this->times) {
    if ($this->times == $this->most_appearences) {
        $this->top_ids .= $this->id . ",";
    }
}
$this->top_ids = rtrim($this->top_ids, ",");
echo "tops = " . $this->top_ids;

It basically just works for 1,11,12,11,12, and returns 11,12, but it seems a bit overcomplicated. I've been at it for some time now.  Am I doing more steps than necessary, missing a built-in function or something?

I might be dealing with hundreds of ids, so performance is important.

Would this step:

foreach ($this->ids_count as $this->id) {
    $this->appearences[$this->id] = substr_count($this->matched_ids_list, ",{$this->id},");
    //[1] => 1, [11] => 2, [12] => 2
}

do more looping than necessary if array_unique wasn't used?

For instance, if ids_count was

[0] => 1, [1] => 11, [2] => 12, [3] => 11, [4] => 12

it would count how many times 11 and 12 appear on matched_ids_list twice. Instead, I use array_unique so ids_count is

[0] =>, [1] => 1, [2] => 11, [3] => 12

Skip to the Dramatic Update for the best solution
I was curious how to do this myself, because I could clearly tell there had to be an easier way! I started from scratch and I think I've found exactly what this question is asking for.
I don't normally just rewrite code, but since the question asks the best way, I believe I'm obligated to share what I believe is the "bets way".
$input = [1, 10, 11, 12, 11, 12, 18, 18];

$recorded = array_count_values($input);

arsort($recorded);

$output = [];

foreach ($recorded as $key => $value) {
    if ($value == max($recorded)) {
        array_push($output, $key);
    }
}

print_r($output);

I'll explain what's happening here:

We receive the input in $input
array_count_values() returns an array using the values of array as keys and their frequency in array as values. In our case, from $input.
arsort — Sort an array in reverse order and maintain index association
Loop over each pair in the newly sorted array. Create some variables to reference the current pair.
If the current value is equal to the largest value from $recorded
Add that key to the output!

And there we have it!
Update to increase performance
$input = [1, 10, 11, 12, 11, 12, 18, 18];

$recorded = array_count_values($input);

arsort($recorded);

$output = [];

$maximum = max($recorded);

foreach ($recorded as $key => $value) {
    if ($value == $maximum) {
        $output[] = $key;
    } else if ($value < $maximum) {
        break;
    }
}

print_r($output);

We've stored the maximum so we aren't checking every iteration, and we're checking for a value less than the maximum so we can break out of the loop to prevent going through the whole array.
Dramatic Update
So, I completely missed the array_keys function, as pointed out by shudder. The newly improved code would look something like:
$input = [1, 10, 11, 12, 11, 12, 18, 18];

$recorded = array_count_values($input);

$output = array_keys($recorded, max($recorded));

print_r($output);

Crazy improvement compared to the OPs original code!


ThreadSafe IsoStorageManager This is a continuation of the last thread.

I wanted to add a thread-safety and I was considering AwaitableCriticalSection and Task+Mutex.  I ended up with mutex; it can be a bit slower but it would allow different files to be stored independently.

public static async Task<T> ReadJson<T>(String fileName)
{
    if (String.IsNullOrEmpty(fileName)) return default(T);

    return await TaskEx.Run(() =>
    {
        using (var myMutex = new Mutex(false, String.Format("{0}@{1}", AppUid, fileName)))
        {
            try
            {
                myMutex.WaitOne();
                using (var store = IsolatedStorageFile.GetUserStoreForApplication())
                using (var stream = new IsolatedStorageFileStream(fileName, FileMode.Open, store))
                using (var sr = new StreamReader(stream))
                using (var jr = new JsonTextReader(sr))
                    return IsoStorageEx.jsonSerializer.Deserialize<T>(jr);
            }
            finally
            {
                myMutex.ReleaseMutex();
            }
        }
    });
}

public static async Task<bool> WriteJson<T>(String fileName, T content)
{
    if (String.IsNullOrEmpty(fileName))
        return false;

    return await TaskEx.Run(() =>
        {
            using (var myMutex = new Mutex(false, String.Format("{0}@{1}", AppUid, fileName)))
            {
                try
                {
                    myMutex.WaitOne();
                    using (var store = IsolatedStorageFile.GetUserStoreForApplication())
                    {
                        store.CreateFolderIfNecessary(fileName);
                        using (var stream = new IsolatedStorageFileStream(fileName, FileMode.Create, FileAccess.Write, store))
                        using (var sw = new StreamWriter(stream))
                        using (var jw = new JsonTextWriter(sw))
                            IsoStorageEx.jsonSerializer.Serialize(jw, content);
                    }
                }
                finally
                {
                    myMutex.ReleaseMutex();
                }

                return true;
            }
        });
}

Where AppUid is just a random string, so mutexes of different apps would not collide.

Sample can be downloaded here.

Any suggestions?

public static async Task<T> ReadJson<T>(String fileName)

The Task-based asynchronous pattern says that the names of Task-returning methods should end with Async, e.g. ReadJsonAsync.

if (String.IsNullOrEmpty(fileName)) return default(T);

Why return the default value here? Empty fileName seems like a bug that should be discovered as soon as possible, so a more reasonable (and common) response would be to throw an exception.

TaskEx.Run

Usually, calling Task.Run shouldn't be done in a library, it should be done by the user. That way, the user will understand that the method is not truly asynchronous. But this library seems to be specifically meant to be called from the UI thread, so I guess it's okay here.

I don't quite understand the need for the Mutex here. Do you really expect to access the same file multiple times at the same time from one application? That doesn't sound like a common situation to me.


Form every unique combination of the items of a list of lists I've tried looking some stuff up. I thought it was transposition first, but looking at this, that's not exactly it.

I hacked this code together in about 3 hours, stopping frequently due to frustration and I'm pretty sure there's some bad practices in there, I'm just too glad right now I actually got it to work. If anyone could point out a better way of doing this, or a part of it, that would be appreciated.

The idea is you have a variable amount of lists with each a variable amount of items. For instance, lists of String, which I used in my code, which can look like this:

List1["a","b","c"]
List2["d"]
List3["e","f"]

And the result would be:

List1["a","d","e"]
List2["a","d","f"]
List3["b","d","e"]
List4["b","d","f"]
List5["c","d","e"]
List6["c","d","f"]

I left in the sysout statements in case somebody wants to execute it and follow what's happening.

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import com.google.common.collect.Lists;

public class Main {

    public static void main(String[] args) {
        List<String> list1 = new ArrayList<String>();
        list1.add("a");
        list1.add("b");
        list1.add("c");

        List<String> list2 = new ArrayList<String>();
        list2.add("d");

        List<String> list3 = new ArrayList<String>();
        list3.add("e");
        list3.add("f");

        List<List<String>> group = Lists.newArrayList(list1,list2,list3);
        System.out.println(Arrays.toString(group.toArray()));

        List<List<String>> end = makeList(group);

        System.out.println(Arrays.toString(end.toArray()));
    }

    /**
     * Take the original list and output all unique combinations.
     * This method is called recursively.
     */
    private static List<List<String>> makeList(List<List<String>> group) {
        if (group.size() == 1) {
            return splitToSingleItemsLists(group.get(0));
        } else {
            List<List<String>> subList = makeList(group.subList(1, group.size()));
            System.out.println(Arrays.toString(subList.toArray()));
            System.out.println(Arrays.toString(group.toArray()));
            System.out.println("=====");
            List<List<String>> tmpList = Lists.newArrayList();

            List<List<String>> splitToSingleItemsLists = splitToSingleItemsLists(group.get(0));
            System.out.println(Arrays.toString(splitToSingleItemsLists.toArray()));
            System.out.println("----");

            List<String> result = new ArrayList<String>();
            for (List<String> string : subList) {
                result = new ArrayList<String>();
                result.addAll(string);
                for (List<String> tmp : splitToSingleItemsLists) {
                    List<String> result2 = new ArrayList<>();
                    result2.addAll(result);
                    for (String tmp2 : tmp) {
                        result2.add(tmp2);

                        tmpList.add(result2);
                    }
                }
            }

            return tmpList;
        }
    }

    /**
     * Split a list of strings to a list of lists with 1 item of the original list each time.
     */
    private static List<List<String>> splitToSingleItemsLists(List<String> list) {
        List<List<String>> result = Lists.newArrayList();

        for (String str : list) {
            List<String> tmp = new ArrayList<String>();
            tmp.add(str);
            result.add(tmp);
        }

        return result;
    }
}

This seems more complicated than it needs to be.  Consider the following code

        int currentIndex = current.size();
        for (String s : factors.get(currentIndex)) {
            current.add(s);
            multiply(factors, current, results);
            current.remove(currentIndex);
        }

This code adds and removes an element to the current list.  In between, it calls itself recursively.  Then consider

        if (current.size() >= factors.size()) {
            // Don't really need to make a deeper copy with String values
            // but might as well in case we change types later.
            List<String> result = new ArrayList<>();
            for (String s : current) {
                result.add(new String(s));
            }
            results.add(result);

            return;
        }

When we find that we've processed an entire result, we add to the results and return.

Note that we could optimize that a bit by switching from a deep copy to a shallow copy.  With strings, that shouldn't make a difference.  I did the deep copy because we aren't restricted to strings with this kind of operation.

        if (current.size() >= factors.size()) {
            results.add(new ArrayList<>(current));

            return;
        }

That's the shallow version.

The whole thing:

    private static void multiply(List<List<String>> factors, List<String> current, List<List<String>> results) {
        if (current.size() >= factors.size()) {
            // Don't really need to make a deeper copy with String values
            // but might as well in case we change types later.
            List<String> result = new ArrayList<>();
            for (String s : current) {
                result.add(new String(s));
            }
            results.add(result);

            return;
        }

        int currentIndex = current.size();
        for (String s : factors.get(currentIndex)) {
            current.add(s);
            multiply(factors, current, results);
            current.remove(currentIndex);
        }
    }

    private static List<List<String>> multiply(List<List<String>> factors) {
        List<List<String>> results = new ArrayList<>();
        multiply(factors, new ArrayList<String>(), results);
        return results;
    }

I added the second method so that I could call it like

        System.out.println(Arrays.toString(multiply(group).toArray()));

Otherwise I would have needed to use more lines in the caller.

This version outputs the products in the same order as shown in your question, where your code outputs them in reverse order.  I don't know if this matters or not.

I find this to be more straightforward than your version.  It builds up to the answer.  Your version builds the answer on its way back from the recursion, which is why the results are backwards.

It also avoids some things that I didn't like about your version:

            for (List<String> string : subList) {

I'm not sure exactly what it is, but it's not a string.  It's a List.  I'd have probably called it something like strings or pieces in lieu of a better name.

                        result2.add(tmp2);

I'm not a fan of numbered variables.  I find it especially confusing here, as result and result2 are the same type but tmp and tmp2 are not.

I'm not crazy about your use of tmp for a loop index variable.  It seems like there should have been a better name.  That variable is no more temporary than any other.

            List<String> result = new ArrayList<String>();
            for (List<String> string : subList) {
                result = new ArrayList<String>();

You could just say

            for (List<String> string : subList) {
                List<String> result = new ArrayList<>();

The first initialization doesn't do anything before it is overwritten and result is never used beyond a single loop iteration.


Implementing Linked lists in Python from scratch After finishing my ds and algorithms course I wanted to implement what I've learned through the semester so here is my try for Linked lists, both regular (one way) and double one (two way).

code:

from node import ListNode

class LinkedList():
    _length = 0

    ''' Init linked list '''
    ''' double is boolean '''
    def __init__(self, x, doubly=False):
        if x is not None:
            self.head = ListNode(x, doubly)
            self.tail = self.head
            self._length += 1
        else:
            assert False, 'User entered a None value'

    ''' representing the list '''
    def __str__(self, doubly=False):
        st = ''
        arrow = ' <-> ' if doubly else ' -> '
        p = self.head
        while p is not None:
            st += str(p.key)
            p = p.next
            if p is not None:
                st += arrow
        return st

    ''' insert to the end of the list '''
    def insert(self, x, doubly=False):
        if self.head is None:
            self.__init__(x)
            return
        p = self.head
        new_node = ListNode(x, doubly)
        self.tail.next = new_node
        if doubly:
            new_node.prev = self.tail
        self.tail = new_node
        self._length += 1

    ''' insert to the head of the list '''
    def insert_to_head(self, x, doubly=False):
        new_node = ListNode(x, doubly)
        new_node.next = self.head
        if doubly:
            self.head.prev = new_node
        self.head = new_node
        self._length += 1

    ''' delete from list and fix pointers '''
    def delete(self, x, doubly=False):
        ''' deleting first instance of x '''
        # if list is empty
        if self.head is None:
            raise Exception('List is empty')
        # else..
        p = self.head
        # if head is x, delete and fix
        if p.key == x:
            if len(self) > 1:
                if doubly:
                    p.next.prev = None
                self.head = p.next
                self._length -= 1
            else:
                self.head, self.tail = None, None
                self._length -= 1
                return
            del p
            return
        # regular delete from list
        while p.next is not None and p.next.key != x:
            p = p.next
        if p.key != x:
            return None
        tmp = p.next  # grab Node contains `x`
        if self.tail is tmp:
            self.tail = p
        p.next = p.next.next
        if doubly:
            p.next.next.prev = p
        del tmp
        self._length -= 1

    ''' search (linear time O(n)) in list '''
    def search(self, x):
        if self.head is None:
            return None
        p = self.head
        if p.key == x:
            return p
        while p.key != x and p.next is not None:
            p = p.next
        return p if p.key == x else None

class DoublyLinkedList(LinkedList):

    _length = 0

    ''' Init double linked list '''
    def __init__(self, x):
        super().__init__(x, True)

    ''' string with doublt linkedlist '''
    def __str__(self):
        return super().__str__(True)

    def __len__(self):
        return self._length

    def insert(self, x):
        super().insert(x, True)

    def insert_to_head(self, x):
        super().insert_to_head(x, True)

    def delete(self, x):
        super().delete(x, True)

    def search(self, x):
        return super().search(x)

the Node class:

class ListNode():
    def __init__(self, x, double=False):
        self.key = x
        self.next = None
        if double:
            self.prev = None

    def __str__(self):
        try:
            return str(self.key)
        except AttributeError:
            return 'No key for this Node'

thanks in advance for you code review!

Things I want to check:

is the code is understandable?
is it well organized?
is the implementation using abstract class is good or there are conventions I missed/etc ?
any other suggestions?

Review

You should take advantage of the bi-directional nature of a doubly linked list. It's a pitty to let it use navigation of a normal linked list just for the sake of enabling inheritance. And why should a normal linked list be able to branch between normal and doubly mode? This is a code smell.

A practical way to create a doubly linked list, is to create a circular list. You only need to store the head. tail would be head.prev. This also works with a single node head = head.prev = head.next. The advantage is that less if statements are required to perform insert / delete operations. Walking the nodes starts at head and stops until we encounter head again. We could walk both directions if we wish to.

You can initialise a single node:

self.head = ListNode(x, doubly)
self.head.prev = self.head
self.head.next = self.head

The delete would be significantly simplified:

After walking the nodes until you get the proper node p given x, you can call

if p.next == p:
    self.head = None
else:
    p.next.prev = p.prev
    p.prev.next = p.next
    if self.head == p:
        self.head = p.next
del p
self._length -= 1

I don't see a value in using inheritance for these lists.

I also don't get why you store p = self.head in insert. It's an unused variable.


Simplify a complex LINQ query Any suggestions how I could simplify this LINQ query?

from type in assembly.GetTypes()
where type.IsPublic && !type.IsSealed && type.IsClass
where (from method in type.GetMethods()
       from typeEvent in type.GetEvents()
       where method.Name.EndsWith("Async")
       where typeEvent.Name.EndsWith("Completed")
       let operationName = method.Name.Substring(0, method.Name.Length - "Async".Length)
       where typeEvent.Name == operationName + "Completed"
       select new { method, typeEvent }).Count() > 0
select type;

assembly is of type System.Reflection.Assembly.
If you need more information, just ask.

Original code (wrapped up in a method):

public IEnumerable<Type> GetAsyncCompletableTypes(Assembly assembly)
{
    return from type in assembly.GetTypes()
           where type.IsPublic && !type.IsSealed && type.IsClass
           where (from method in type.GetMethods()
                  from typeEvent in type.GetEvents()
                  where method.Name.EndsWith("Async")
                  where typeEvent.Name.EndsWith("Completed")
                  let operationName = method.Name.Substring(0, method.Name.Length - "Async".Length)
                  where typeEvent.Name == operationName + "Completed"
                  select new { method, typeEvent }).Count() > 0
           select type;
}

The first thing I notice is that the overall structure of the query is:

Find me all types in the assembly
Where the type is a public, non-sealed class
And where the type passes a complicated looking filter.

I'd split the complicated looking filter out into a method to start with:

public IEnumerable<Type> GetAsyncCompletableTypes(Assembly assembly)
{
    return from type in assembly.GetTypes()
       where type.IsPublic && !type.IsSealed && type.IsClass
       where IsAsyncCompletableType(type)
       select type;
}

private static bool IsAsyncCompletableType(Type type)
{
    return (from method in type.GetMethods()
        from typeEvent in type.GetEvents()
        where method.Name.EndsWith("Async")
        where typeEvent.Name.EndsWith("Completed")
        let operationName = method.Name.Substring(0, method.Name.Length - "Async".Length)
        where typeEvent.Name == operationName + "Completed"
        select new { method, typeEvent }).Count() > 0;
}

That gives us two simpler queries to look at. The only thing I can see in the first part is that the repeated where can be collapsed into a single one:

public IEnumerable<Type> GetAsyncCompletableTypes(Assembly assembly)
{
    return from type in assembly.GetTypes()
       where type.IsPublic && !type.IsSealed && type.IsClass && IsAsyncCompletableType(type)
       select type;
}

Onto the second part. The lines in the query seem to alternate between being related to the methods and the events - reordering the lines will make it clearer what's going on:

private static bool IsAsyncCompletableType(Type type)
{
    return (from method in type.GetMethods()
        where method.Name.EndsWith("Async")
        let operationName = method.Name.Substring(0, method.Name.Length - "Async".Length)
        from typeEvent in type.GetEvents()
        where typeEvent.Name.EndsWith("Completed")
        where typeEvent.Name == operationName + "Completed"
        select 0).Any();
}

We're now using the method variable up to the let operation, and never using it again, so we can select the operationName in a subquery instead of using let.

private static bool IsAsyncCompletableType(Type type)
{
    var operationNames = from method in type.GetMethods()
        where method.Name.EndsWith("Async")
        select method.Name.Substring(0, method.Name.Length - "Async".Length);

    return (from operationName in operationNames
        from typeEvent in type.GetEvents()
        where typeEvent.Name.EndsWith("Completed")
        where typeEvent.Name == operationName + "Completed"
        select 0).Any();
}

You may notice that the two where lines don't make a lot of sense together at this point:

Pick events
Where the name ends with "Completed"
And where the name starts with operationName and ends with "Completed"

The first line is redundant. So we can remove it:

private static bool IsAsyncCompletableType(Type type)
{
    var operationNames = from method in type.GetMethods()
        where method.Name.EndsWith("Async")
        select method.Name.Substring(0, method.Name.Length - "Async".Length);

    return (from operationName in operationNames
        from typeEvent in type.GetEvents()
        where typeEvent.Name == operationName + "Completed"
        select 0).Any();
}

The only thing we ever do to operationName is add "Completed" to it - we might as well do that when we create the operationName (and rename it appropriately):

private static bool IsAsyncCompletableType(Type type)
{
    var eventNamesFromMethods = from method in type.GetMethods()
        where method.Name.EndsWith("Async")
        select method.Name.Substring(0, method.Name.Length - "Async".Length) + "Completed";

    return (from eventNameFromMethod in eventNamesFromMethods
        from typeEvent in type.GetEvents()
        where typeEvent.Name == eventNameFromMethod
        select 0).Any();
}

We're now asking the computer to iterate over all the events and select its name for every eventNameFromMethod. We can pre-compute these and put them into a fast lookup container - a HashSet:

private static bool IsAsyncCompletableType(Type type)
{
    var eventNamesFromMethods = from method in type.GetMethods()
        where method.Name.EndsWith("Async")
        select method.Name.Substring(0, method.Name.Length - "Async".Length) + "Completed";

    var eventNames = new HashSet<string>(
        from typeEvent in type.GetEvents()
        select typeEvent.Name);

    return (from eventNameFromMethod in eventNamesFromMethods
        where eventNames.Contains(eventNameFromMethod)
        select 0).Any();
}

That last bit is now really just a where and an Any. But Any has an overload that takes a condition, so let's use it:

private static bool IsAsyncCompletableType(Type type)
{
    var eventNamesFromMethods = from method in type.GetMethods()
        where method.Name.EndsWith("Async")
        select method.Name.Substring(0, method.Name.Length - "Async".Length) + "Completed";

    var eventNames = new HashSet<string>(
        from typeEvent in type.GetEvents()
        select typeEvent.Name);

    return eventNamesFromMethods.Any(eventNameFromMethod => eventNames.Contains(eventNameFromMethod));
}

And with a little bit of rearranging to remove the eventNamesFromMethods variable that only gets used once:

private static bool IsAsyncCompletableType(Type type)
{
    var eventNames = new HashSet<string>(
        from typeEvent in type.GetEvents()
        select typeEvent.Name);

    return (from method in type.GetMethods()
        where method.Name.EndsWith("Async")
        select method.Name.Substring(0, method.Name.Length - "Async".Length) + "Completed")
        .Any(eventNameFromMethod => eventNames.Contains(eventNameFromMethod));
}

Personally (and feel free to disagree here), I find the extension method syntax far easier to read and reason about (especially when you have to use things like Any anyway), so here's what it looks like using that:

public static IEnumerable<Type> GetAsyncCompletableTypes(Assembly assembly)
{
    return assembly.GetTypes()
        .Where(type => type.IsPublic && !type.IsSealed && type.IsClass && IsAsyncCompletableType(type));
}

private static bool IsAsyncCompletableType(Type type)
{
    var eventNames = new HashSet<string>(type.GetEvents().Select(typeEvent => typeEvent.Name));

    return type.GetMethods()
        .Where(method => method.Name.EndsWith("Async"))
        .Select(method => method.Name.Substring(0, method.Name.Length - "Async".Length) + "Completed")
        .Any(eventNameFromMethod => eventNames.Contains(eventNameFromMethod));
}


Code review for a collection manipulation tool in Javascript As I was working with .NET, I started wondering how I would implement the LINQ methods syntax in Javascript. So here is my first try; is there a more elegant, or more performant, way to write this code?

// usage : the functions take a predicate function as input and modify the array accordingly
var collection;
(function() {
    "use strict";

    var Collections = function(array) {
        this.array = array;
    };

    Collections.prototype = {
        //The predicate should return a boolean value
        //Keeps all elements that match the given function
        where: function(predicate) {
            var newArray = [];
            this.each(function(index, item) {
                if (predicate(item)) {
                    newArray.push(item);
                }
            });
            this.array = newArray;
            return this;
        },
        //The predicate should return a boolean value
        //Returns true if one element or more matches the predicate
        any: function(predicate) {
            var newArray = [];
            this.each(function(index, item) {
                if (predicate(item)) {
                    newArray.push(item);
                }
            });
            return newArray.length > 0;
        },
        //The predicate should return an object
        //Replaces all elements by the object generated by the predicate
        select: function(predicate) {
            var newArray = [];
            this.each(function(index, item) {
                newArray.push(predicate(item));
            });
            this.array = newArray;
            return this;
        },
        //Returns the current (modified) array
        getArray: function() {
            return this.array;
        }
    };

    collection = function(array) {
        return new Collections(array);
    };
} ());

And here is an exemple of use:

var myArray = [{name: 'chicken', size: 1}, {name: 'cat', size: 2},
{name: 'dog', size: 3}, {name: 'horse', size: 4}, {name: 'skunk', size: 2}];
var newArray = collection(myArray)
    .where(function(animal){
        return animal.size == 2
    })
    .select(function(animal){
        return {
            animalName: animal.name,
            category: 'mammal',
            size: 'small'
        }
    })
    .getArray()
// new array : [{animalName: 'cat', category: 'mammal', size: 'small}, {animalName: 'skunk',
// category: 'mammal', size: 'small'}]

To clarify the use of your global variable "collection" as a namespace for your library, you should assign it directly to a value returned by your Immediately Invoked Function Expression:

var collection = (function(){
  ...

  function collection(array) {
    return new Collections(array);
  }

  return collection;
}());


Sparse linked list insert function I wrote this sparse linked list insert function, and there are nine return statements. Is this a code smell? Is this badly implemented? I fear that it's hard to read, or hard to maintain.

/* NOT BUGS, TESTED AND WORKS CORRECTLY
 * insert an element into a list
 * list is ordered using pos
 * if position pos is already occupied, the value of the node
 * should be updated with val
 * if val=0, then the element should be deleted
 * return 0 if operation is succesfull
 *        1 if malloc failed */
int insert_element(ElementNode_handle *list_handle, int pos, int data)
{
  /*Record the head*/
  ElementNode *current = *list_handle;

  /* If data is 0, stop */
  if (data == 0)
  {
    return 1;
  }

  /* If list is empty, crate new list*/
  if (current == NULL)
  {
    /* Create new list */
    ElementNode *new_node = make_node(pos, data);
    /* Malloc fail check*/
    if (new_node == NULL)
    {
      return 0;
    }
    new_node->data = data;
    new_node->pos = pos;
    *list_handle = new_node;
    return 1;
  }
  /* If head pos == pos, replace and done*/
  if (current->pos == pos)
  {
    current->data = data;
    return 1;
  }
  else if (current->pos > pos)
  {
    /* Create new node between current and next */
    ElementNode *new_node = make_node(pos, data);
    if (new_node == NULL)
    {
      return 0;
    }
    new_node->next = current;
    *list_handle = new_node;
    return 1;
  }
  /*Walk the list, until next hits the end*/
  while (current->next != NULL)
  {
    if (current->next->pos == pos)
    { /* If next pos equals post, replace, done*/
      current->next->data = data;
      return 1;
    }
    else if (current->next->pos > pos)
    {
      /* Create new node between current and next */
      ElementNode *new_node = make_node(pos, data);
      if (new_node == NULL)
      {
        return 0;
      }
      ElementNode *next     = current->next;
      current->next         = new_node;
      new_node->next        = next;
      return 1;
    }
    /*walk the list*/
    current = current->next;
  }
  /* Append to the tail */
  ElementNode *new_node = make_node(pos, data);
  if (new_node == NULL)
  {
    return 0;
  }
  current->next = new_node;
  return 1;
}

=== EDIT ===
Refactored based on @user1118321

/**
 * @brief insert an element into a list
 * @details
 * list is ordered using pos if position
 * pos is already occupied, the value of
 * the node should be updated with data
 * @param p_list_handle Opaque pointer to list
 * @param pos Position to insert
 * @param data data at position
 * @return 0 = success, 1 = failed
 */
int insert_element(ElementNode_handle *p_list_handle, int pos, int data)
{
  /*Record the head*/
  ElementNode *current = *p_list_handle;
  /*Record previous*/
  ElementNode *previous = NULL;
  /*End result*/
  int result = -1;

  if (data == 0)
  {
    delete_element(p_list_handle, pos);
    return 0;
  }

  if (current == NULL)
  {
    return make_list(p_list_handle, pos, data);
  }

  while ((current != NULL) && (result == -1))
  {
    if (current->pos == pos)
    {
      current->data = data;
      result = 0;
    }
    else if (current->pos > pos)
    {
      ElementNode *new_node = make_node(pos, data);
      if (new_node == NULL)
      {
        result = 1;
      }
      if (previous == NULL)
      {
        *p_list_handle = new_node;
      }
      else
      {
        previous->next = new_node;
      }
      new_node->next = current;
      result = 0;
    }
    previous = current;
    current = current->next;
  }
  if (result == -1)
  {
    ElementNode *new_node = make_node(pos, data);
    if (new_node == NULL)
    {
      result = 0;
    }
    previous->next = new_node;
  }
  return result;
}

Do what you say

 * return 0 if operation is succesfull
 *        1 if malloc failed */

So if a malloc fails, you are supposed to return 1.  But when you do the actual check:

    ElementNode *new_node = make_node(pos, data);
    /* Malloc fail check*/
    if (new_node == NULL)
    {
      return 0;
    }

You return 0.

You return 1 on success -- the exact opposite of the comment.  You should either update the comment to match the code or change the code to match the comment.  Or remove the comment--if you have to read the code anyway, why bother?

Keep it simple

      ElementNode *next     = current->next;
      current->next         = new_node;
      new_node->next        = next;

Why do you need next?  Consider

      new_node->next = current->next;
      current->next = new_node;

You save an assignment and a variable declaration by just changing the order in which two lines happen.


Using fewer foreach loops to bind values to parameters in PDO I want to make a query to order by the IN clause items. I read from another thread that I can use ORDER BY FIELD(a.id, val1,val2......) When binding values to parameters for two occurrences of $in, I ended up using two foreach loops:

      foreach ($tid as $t) {
        $users->bindValue($c++, $t);
      }
      foreach ($tid as $t) {
        $users->bindValue($c++, $t);
      }

Would you prefer other solutions? Is there any way to reduce it to one foreach loop?

    try
    {
      $dbh = new PDO("mysql:host=$hostname;dbname=$databasename", $username, $password);
      $tid = $_POST["tid"];
      $comma = "";
      $in  = str_repeat('?,', count($tid) - 1) . '?';
      $sql = "
              SELECT b.name,a.users
              FROM `table1` a INNER JOIN
              `table2` b ON a.id = b.id
              WHERE a.id IN ($in)
              GROUP BY a.id
              ORDER BY FIELD(a.id, $in)
             ";
      $users = $dbh->prepare($sql);
      $users->bindValue(1, $_SESSION["user_id"]);
      $c = 2;
      foreach ($tid as $t) {
        $users->bindValue($c++, $t);
      }
      foreach ($tid as $t) {
        $users->bindValue($c++, $t);
      }
      $users->execute();
      $dbh = null;

    }
    catch(PDOException $e)
    {
     echo $e->getMessage();
    }

$_POST["tid"] Content Example:

Array
(
    [0] => 215
    [1] => 391
)

The PDOStatement class (which is what PDO::prepare returns, allows you to pass an array of values to the execute method. If you use that, you don't have to loop over any arrays, calling bindValue over and over:

$vals = array(
    $_SESSION['user_id']
);
$vals = array_merge(
    $vals,
    $_POST['tid']
);
$in = array_fill(
    0, count($vals),
    '?'
);//construct array containing ?
$stmt = $pdo->prepare(
    'SELECT b.name,a.users
     FROM `table1` a
       INNER JOIN `table2` b
         ON a.id = b.id
     WHERE a.id IN ('.implode(',',$in).')
     GROUP BY a.id
     ORDER BY FIELD(a.id, '.implode(',',$in).')'
);
$stmt->execute($vals);

The added benefit of this approach is that you're able to use named placeholders, with just a few minor adjustments:

$vals = array(
    ':sessionUid' => $_SESSION['user_id']
);
foreach ($_POST['tid'] as $idx => $val) {
    $vals[':uid'.$idx] => $val;
}
$in = implode(',', array_keys($vals));
$stmt = $pdo->prepare(
    'SELECT b.name,a.users
     FROM `table1` a
       INNER JOIN `table2` b
         ON a.id = b.id
     WHERE a.id IN ('.$in.')
     GROUP BY a.id
     ORDER BY FIELD(a.id, '.$in.')'
);
$stmt->execute($vals);

Of course, there are a few tradeoffs. Nothing too bad, but still:

PDOStatement::execute(array $bind) binds all of the values as PDO::PARAM_STR (ie string values). This implies you can't use array binding if you are using placeholders in, for example, LIMIT clauses. In those cases, a loop + $stmt->bindValue(':offset', $value, PDO::PARAM_INT); is required. Writing a wrapper that does this for you is not uncommon, and in some cases desirable.
re-use of placeholders is only supported if you're using PDO's emulated prepared statements:

Because you seem to be re-using the same placeholders here, you cannot disable emulated prepares. MySQL does not support the re-use of named placeholders. If you do disable emulated prepares, you will need to add a loop:

//create vals as before
$in = array_keys($vals);
$query = 'SELECT b.name,a.users
     FROM `table1` a
       INNER JOIN `table2` b
         ON a.id = b.id
     WHERE a.id IN ('.implode(','$in).')
     GROUP BY a.id
     ORDER BY FIELD(a.id, ';
foreach ($in as $idx => $key) {
    $vals[$key.$idx] = $vals[$key];//add value a second time, using a different key
    $in[$idx] .= $idx;//change values in $in
}
$query .= implode(',', $in).')';//add new placeholders to query
$stmt = $pdo->prepare($query);
$stmt->execute($vals);//should work now

Apart from that, I'd suggest you set PDO's error mode so that it actually throws exceptions. As it stands, PDO will throw an exception if the connection fails, but not if any of the other calls fail (prepare, execute,...).
To fix this, either call PDO::setAttribute, or pass an array to the constructor:

$pdo = new PDO(
    $dsn,
    $user,
    $pass,
    array(
        PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
    )
);
//or
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

All in all, loops like you have do very little harm, avoiding them can improve readability, but their impact on performance is so minute that getting rid of them, for the sake of optimization is not a great idea. Remember "Premature optimization is the root of all evil".


Function for creating unique strings for urls I wrote this function in order to create unique url's for my site. Using these urls, visitors could demo site functionality without having to create a login.  The function was intended to return a unique string of characters which could be added to a url like this:

http://www.mysite.com?temp_user=dd5vbrk4ax4v4o09l2bm

My function had the following criteria:

Every call must generate a unique string of alpha numeric characters.
Most important: Zero chance of string duplication.
It should be 20 characters long (secure, but not awkwardly long.)
Repeat calls should reflect a high degree of dissimilarity.
Character usage should appear as random as possible.

Here is my code

function url_code(){

  $letters =  "1234567890abcdefghijklmnopqrstuvwxyz";
  $microtime = microtime();
  $time_array = explode(' ', substr($microtime,2));
  $code = '';

  $time_array[0] = substr($time_array[0],0,6);
  $time_array[0] .= strrev($time_array[0]);
  $time_array[2] = strrev($time_array[1]);

  for($i = 0; $i <10; $i++){

    $num = ((int)$time_array[1][$i]);
    $num += ((int)$time_array[2][$i])*10;
    $num += ((int)$time_array[0][$i])*100;

    $mult   = (int)($num / 36);
    $mod    = $num - ($mult * 36);

    $code  .= $letters[$mult].$letters[$mod];

  }

  return $code;

}

Please let me know if there are any simple improvements I could make or if I missed any serious problems with string repetition.

One interesting note on character randomness: currently, characters generated by the $mult variable (the first, third, fifth, and other odd character in the code string) will never be t,u,v,w,x,y, or z. How can I fix this so that [t-z] are also used?

Possible fix (all characters will be used. However, [8-9a-s] are twice as likely to appear as the other characters, [0-7t-z]):

$mult   = (int)($num / 36);
$mod    = $num - ($mult * 36);
if($mult < 9 && mt_rand(0,1)){$mult += 27;}

$code  .= $letters[$mult].$letters[$mod];

Note, Scalability issue found:  If this function is run on more than one server, there is a tiny chance that two users could send a request at exactly the same time and get a duplicated code.

Algorithm

  Every call must generate a unique string of alpha numeric characters.
  Most important: Zero chance of string duplication.

"zero chance" is really hard to achieve, if it's even possible.
Your algorithm is essentially based on the current time in microseconds.
Since systems may adjust their clock, for example with NTP,
the same microsecond may happen again,
resulting in a non-unique string.

Striving for a unique random string is known challenge,
see for example the uniqid function.
The documentation offers some recommendations for cryptographic security,
and I think it might be a good idea to incorporate uniqid into your algorithm.

  Repeat calls should reflect a high degree of dissimilarity.

If I call the function twice in a row I get values with a similar prefix, for example:

qf1p1dojax7n7m08no1f
qf1p1drbj0nbnahjqg1f

gp70pxdfghnbnaetckpa
gp70pxg8j0q4q3hlfcpa

This is because in the microseconds parts are too similar in consecutive calls.
You seem to be using the microseconds part as a 6-digit random number.
You would get better results with a proper pseudo random generator,
for example one of the recommendations from uniqid.

Readability

The implemented algorithm is quite straightforward:

Prepare 3 strings of at least 10-digits:

a 12-digit string from the first 6 decimal digits of the microseconds elapsed since the current timestamp seconds, concatenated with its reverse
a 10-digit string from the current timestamp
a 10-digit string from the reverse of the current timestamp

Count from 0 until 9 to generate a 20-letter string:

Create a 3-digit number using i-th positions from the prepared strings
Encode the 3-digit number as a base-36 number, so that it becomes a 2-letter string
Concatenate the 2-letter encoded strings to get a 20-letter string

Unfortunately this flow of logic is not easy to see in the implementation,
because the variable names are not helping.

Instead of the array $time_array,
it would be better to use 3 variables with descriptive names.
If you extract the big logical steps to functions, for example one to generate a 3-digit number, and another to encode a 3-digit number in base-36, the code could read almost like a story.

Technique

Instead of $mod = $num - ($mult * 36) you could simply use the modulo operator: $mod = $num % 36.

Instead of hard-coding 36, which has to be the length of $letters,
it would be better to store it in a variable, and derive its value from the length of $letters.
So that if one day you change the alphabet (for example add capital letters too),
then you won't have to remember to replace all the 36 with the new value,
the program will "just work".

Instead of implementing base-36 encoding yourself,
you could use the existing base_convert function.
It's not 100% the same, because it uses slightly different alphabet,
and you would need to convert the number parameter to string.


Insertion Sort- inserting from the left I've implemented my own insertion sort algorithm, and here's the code.
def insertion(lst):
    for i in range(1, len(lst)):
        for j in range(i):
            if lst[i] < lst[j]:
                val = lst[i]
                del lst[i]
                lst.insert(j, val)
                break
    return lst

Typical insertion sort algorithms seem to compare values from the immediate left of the current index and make swaps as you continue left in the sorted values.
I implemented my algorithm such that it will compare values from left to right (the second for loop). Do you see any issues with my code, or do you have any feedback on this?

The typical insertion sort is O(n) for already sorted input, and "almost O(n)" for "almost sorted" input. Which is somewhat common in real world data. Your way you don't have that, you're \$\Theta(n^2)\$. Someone not familiar with how lists work internally might think your code is O(n) for reverse-sorted input, but since del lst[i] takes O(n - i) and lst.insert(j, val) takes O(n - j), that's not the case.
As Ted points out, better don't both modify and return. Choose one or the other. Just like Python's own sorting functions do: list.sort only modifies, and sorted only returns (well, it necessarily "modifies" a given iterator by consuming it).
"Insertion a list"... huh? insertion seems like an incomplete name for insertion sort. You can't really tell from the function signature what it does (and there's no doc string, either).
I'd go with enumerate, then you have val already, which is nicer and faster.

Improved version sticking with search-from-left but incorporating the other points:
def insertion_sort(lst):
    for i, val in enumerate(lst):
        for j in range(i):
            if val < lst[j]:
                del lst[i]
                lst.insert(j, val)
                break

A benchmark sorting 10000 random numbers:
2.17 seconds  insertion
1.69 seconds  insertion_sort

2.15 seconds  insertion
1.66 seconds  insertion_sort

2.17 seconds  insertion
1.69 seconds  insertion_sort

Benchmark code:
from timeit import timeit
from random import random

def insertion(lst):
    for i in range(1, len(lst)):
        for j in range(i):
            if lst[i] < lst[j]:
                val = lst[i]
                del lst[i]
                lst.insert(j, val)
                break
    # return lst

def insertion_sort(lst):
    for i, val in enumerate(lst):
        for j in range(i):
            if val < lst[j]:
                del lst[i]
                lst.insert(j, val)
                break

for _ in range(3):
    lst = [random() for _ in range(10000)]
    for func in insertion, insertion_sort:
        copy = lst * 1
        t = timeit(lambda: func(copy), number=1)
        assert copy == sorted(lst)
        print('%.2f seconds ' % t, func.__name__)
    print()

Try it online!


Replace ' ' Spaces in a URL with a '%20' - Coding Challenge This was a small coding challenge proposed at my school. The problem is:

  Given a string (or URL), replace the white-spaces inside of the string
  with %20, but any spaces on the outside should not be included.

  For example, input " Mr John Smith " would return "Mr%20John%20Smith".

I have completed this challenge successfully using the code below. My question is, is there any way to improve the efficiency? I believe the complexity is currently \$O(2n) = O(n)\$ given the 2 for loops. I do not want to use any libraries or functions like str.replace(). But I'm assuming there is a better way than trimming and then counting the whitespace.

public class URL {

    /**
     * @description URLify ~ A small method that makes a string with spaces URL Friendly!
     * @param str
     * @param length
     * @return String
     */

    public static String URLify(String str) {
        str = str.trim();
        int length = str.length();
        int trueL = length;
        if(str.contains(" ")) {
            for(int i = 0; i < length; i++) {
                if(str.charAt(i) == ' ') {
                    trueL = trueL + 2;
                }
            }
            char[] oldArr = str.toCharArray();
            char[] newArr = new char[trueL];
            int x = 0;
            for(int i = 0; i < length; i++) {
                if(oldArr[i] == ' ') {
                    newArr[x] = '%';
                    newArr[x+1] = '2';
                    newArr[x+2] = '0';
                    x += 3;
                } else {
                    newArr[x] = oldArr[i];
                    x++;
                }
            }
            str = new String(newArr, 0, trueL);
        }
        return str;
    }
    public static void main(String[] args) {
        String str = "     https://google.com/  testing  .pdf    ";
        str = URLify(str);
        System.out.print(str);
    }
}

Note: I am looking for any criticism. I would really like to improve my skills in general.

From a short review:

If you are going to write your own replace, then you should create a method for it, so that you can call myReplace( string, target, replacement);
Personally I would have checked if args got a String, and only if nothing is provided would I use the hard coded string, that would make it easier to show and test your design
No reason to do this in 2 lines:

str = URLify(str);
System.out.print(str);

you can just write

System.out.print(URLify(str));

Javadoc commenting looks good (but is not actually true?), otherwise there are zero comments
I am pretty you can merge those 2 loops into one if you used a stringbuilder. On the whole the replace algorithm could use work