Leading substrings exercise I have a couple of answers that I wrote up for this exercise. Both of my methods work. I'm hoping to get some feedback on each and how I could improve them, if needed.

The idea is to take a string, such as 'abc' and return an array like this: ['a', 'ab', 'abc']

For the second method, I used #map, but I'm not sure if this is the best way to achieve my desired result.

def substrings(string)
  subbed = []
  (0..string.length-1).each do |i|
    subbed.push(string.slice(0..i))
  end
  return subbed
end

def substrings2(string)
  string.split('').map.with_index do |c,i|
    string.slice(0..i)
  end
end

print substrings2('abc')

What you're looking for is known as a "scan" in functional programming.  Surprisingly, ruby's Enumerable, which is where this method belongs (since what you're doing is not, ultimately, specific to strings), does not provide a scan method or its equivalent.

But we can write one easily enough:

module Enumerable
  def scans
    reduce([]) {|m,x| m << (m.last || []) + [x]}
  end
end

Now we have a general purpose method which we can use like so:

[9] pry(main)> (1..5).to_a.scans
=> [[1], [1, 2], [1, 2, 3], [1, 2, 3, 4], [1, 2, 3, 4, 5]]

Your original problem now becomes a simple delegation to scans:

[14] pry(main)> 'abc'.split('').scans.map(&:join)
=> ["a", "ab", "abc"]


Inefficient PowerShell function for checking group membership I use the following function in a PowerShell script to check and - depending on the value of the $Action - either add or remove an Active Directory User Object from a Security Group.

Function Update-Group-Membership ($Action, $ADUser_Properties, $GroupName, $LogPathFileName) {

    $CurrentGroupMembers = Get-ADGroupMember -Identity:$GroupName | ? { $_.objectClass -ieq 'user' } | Select-Object -ExpandProperty "samAccountName"
    $ExistingMember = [bool]($ADUser_Properties | ? { $CurrentGroupMembers -contains $_.samAccountName })

    If ($Action -eq "ADD") {
        If ($ExistingMember) {
            "[" + (Get-Date -format "dd/MM/yyyy HH:mm:ss") + "]  ... Update NOT Required: Existing Member of $GroupName" | Out-File $LogPathFileName -Append
            Write-Host "  ... Update NOT Required: Existing Member of $GroupName" -ForegroundColor DarkGray
        } Else {
            # Add to the group
            $PSCommand = $ADUser_Properties | Add-ADPrincipalGroupMembership -MemberOf:$GroupName
            "[" + (Get-Date -format "dd/MM/yyyy HH:mm:ss") + "]  ... Updated: Added to $GroupName" | Out-File $LogPathFileName -Append
            Write-Host "  ... Updated: Added to $GroupName" -ForegroundColor Yellow
        }
    }

    If ($Action -eq "REMOVE") {
        If ($ExistingMember) {
            # Remove to the group
            $PSCommand = $ADUser_Properties | Remove-ADPrincipalGroupMembership -MemberOf:$GroupName -Confirm $False
            "[" + (Get-Date -format "dd/MM/yyyy HH:mm:ss") + "]  ... Updated: Removed from $GroupName" | Out-File $LogPathFileName -Append
            Write-Host "  ... Updated: Removed from $GroupName" -ForegroundColor Yellow
        }
}
}

The function is called as follows:

Update-Group-Membership "ADD" $ADUser_Properties "SECURITY_GROUP_NAME" $LogPathFileName

$ADUser_Properties contains the output of the Get-ADUser command.

Whilst this function performs as expected, it is not particularly efficient and takes over a second to check a single user which consequently means processing several hundred users takes several hours.

Does anyone have any suggestions on how to improve or fine-tune it?

This is completely 100% NOT TESTED because I don't have the means to test it at this time however, it should work the way you want and a bit faster than before because it will do alot less queries. It should in theory only query each group once throughout the entire script which if your running even a single group against hundreds of users will save hundreds of queries. Next step I would suggest that you move to a more different loggin function such as streamwriter, is literally over 100x faster at writing logs.

Function Update-GroupMembership {
    Param (
        [Parameter(Mandatory=$True)]
        [ValidateSet('Add','Remove')]
        [String]$Action,
        [Parameter(Mandatory=$True)]
        $ADUser_Properties,
        [Parameter(Mandatory=$True)]
        [String]$ADGroup,
        [Parameter(Mandatory=$True)]
        [String]$LogPathFileName
    )

    IF (-NOT (Test-Path Variable:/Function)) {
        New-Variable -Force -ErrorAction Stop -Scope Script -Name UGM -Value @{}
        $Script:UGM.Users = @{}
        $Script:UGM.Groups = @{}
        if (-not (Get-Module -Name "ActiveDirectory")) {
            Try {
                Import-Module ActiveDirectory
                $Script:UGM.ActiveDirectory = $True
            } Catch {
                $Script:UGM.ActiveDirectory = $False
            }
        }
    }

    #Skip script is active directory not loaded.
    If (!$Script:UGM.ActiveDirectory) {
        Throw "Active Directory module not loaded."
    }

    IF (-NOT ($Script:UGM.ContainsKey($ADGroup))) {
        $Script:UGM.$ADGroupName = New-Object -Type 'System.Collections.ArrayList'
        Get-ADGroupMember -Identity $ADGroup |Select -ExpandProperty SamAccountName |ForEach {$Script:UGM.$ADGroupName.Add($_)}
    }

    $UGM.InGroup = $Script:UGM.$ADGroup.Contains($ADUser_Properties.SamAccountName)
    $Date = ([datetime]::Now).ToString('dd/MM/yyyy HH:mm:ss')
    Switch ($Action) {
        'Add' {
            If ($UGM.InGroup) {
                Write-Host "$($ADUser_Properties.Name) Already in $ADGroup"
            } Else {
                Try {
                    Add-ADPrincipalGroupMembership -MemberOf:$ADGroup -Identity:$ADUser_Properties |Out-Null
                    $Script:UGM.$ADGroup.Add($ADUser_Properties.SamAccountName)
                    Write-Host "  ... Updated: Added to $ADGroup" -ForegroundColor Yellow
                    "[$Date]  ... Updated: Added to $ADGroup" | Out-File $LogPathFileName -Append
                } Catch {
                    Throw "[$Date]  ... Failed: Unable to add $($ADUser_Properties.Name) to $ADGroup"
                }
            }
        }
        'Remove' {
            If ($UGM.InGroup) {
                Write-Host "$($ADUser_Properties.Name) not in $ADGroup"
            } Else {
                Try {
                    Remove-ADPrincipalGroupMembership -MemberOf:$ADGroup -Identity:$ADUser_Properties |Out-Null
                    $Script:UGM.$ADGroup.Remove($ADUser_Properties.SamAccountName)
                    Write-Host "  ... Updated: Removed from $ADGroup" -ForegroundColor Yellow
                    "[$Date]  ... Updated: Removed from $ADGroup" | Out-File $LogPathFileName -Append
                } Catch {
                    Throw "[$Date]  ... Failed: Unable to remove $($ADUser_Properties.Name) from $ADGroup"
                }
            }
        }
        Default {}
    }
}


16-bit FizzBuzz in x86 NASM assembly Since this problem involves small numbers (particularly with a small loop count of 100), it's possible to ease the modulo operation setup by simply working with 16-bit and 8-bit registers:

$$\dfrac{\text{[AX] (16-bit register)}}{\text{[other 8-bit register]}} = \text{[AH] (remainder)}$$

My main concern is with the layout.  Every "basic" high-level implementation I've seen has a check and print together for each case.  I've found it easier to do the same thing here, but I'm not sure if that would also be too readable in assembly.

I'm also aware that it's good to minimize register-moving.  Unfortunately, I still do that with every case since I'm incrementing with one register (CX) and using another (AX) for the dividend.  I can stick to AX for both, but that may involve keeping a copy of the current counter value, which may just make the code a bit more complicated.  I suppose it's not much of a problem here anyway.

Macros used:

nwln -  prints a newline
PutStr - prints a defined string
PutInt - prints a 16-bit integer value

It's not necessary to address the macros; they do work properly.

%include "macros.s"

.DATA

fizz_lbl:       DB    "Fizz", 0
buzz_lbl:       DB    "Buzz", 0
fizzbuzz_lbl:   DB    "FizzBuzz", 0

.CODE
.STARTUP

xor   CX, CX ; counter

main_loop:
   inc   CX
   cmp   CX, 100

   jg    done

   fizzbuzz_check:
      mov   AX, CX ; dividend = counter
      mov   BH, 15 ; divisor
      div   BH     ; (counter / 15)

      cmp   AH, 0            ; counter divisible by 15?
      je    print_fizzbuzz   ; if so, proceed with printing

      jmp   fizz_check       ; if not, try checking for fizz

      print_fizzbuzz:
          PutStr   fizzbuzz_lbl
          nwln
          jmp      main_loop

   fizz_check:
      mov   AX, CX ; dividend = counter
      mov   BH, 3  ; divisor
      div   BH     ; (counter / 3)

      cmp   AH, 0        ; counter divisible by 3?
      je    print_fizz   ; if so, proceed with printing

      jmp   buzz_check   ; if not, try checking for buzz

      print_fizz:
          PutStr   fizz_lbl
          nwln
          jmp      main_loop

   buzz_check:
      mov   AX, CX ; dividend = counter
      mov   BH, 5  ; divisor
      div   BH     ; (counter / 5)

      cmp   AH, 0        ; counter divisible by 5?
      je    print_buzz   ; if so, proceed with printing

      jmp   print_other  ; if not, then can only display number

      print_buzz:
          PutStr   buzz_lbl
          nwln
          jmp      main_loop

   print_other:
      PutInt   CX
      nwln
      jmp   main_loop

done:
   .EXIT

Since we're doing this in assembly language, it makes sense to do it much more efficiently than is typically done in high level languages.  Otherwise, why bother with assembly language? So with that said, there are ways that this can be made much, much more efficient.
Avoid division
The div instruction in x86 is one of the slower instructions possible.  Since we already know that we're looking for numbers divisible by 3, 5 or both, what would make far more sense is to simple keep countdown counters for both.  Your initialization currently says:
    xor cx, cx

It could be easily expanded to say:
    xor cx, cx
    mov bx, 0503h  ; set bh = 5 counter, bl = 3 counter

Then instead of dividing, simply decrement:
    inc cx
    cmp cx, 100
    jg done
;  instead of this...
;    dec bh
;    dec bl
;    cmp bx, 0
; per suggestion from @Chris Jester-Young use this:
    sub bx, 0101h
    je print_fizzbuzz
    test bl, bl
    je print_fizz
    test bh, bh
    je print_buzz
print_other:

Naturally the various print_... routines would have to reset bh, bl or both as well as printing.
Improve formatting
Generally speaking, assembly language code is not indented in the way you have your code indented.  It's much more linear, with the only indentation for assembly language statements or directives.
Consider better I/O
Your output routines are not shown, but it's likely that it would be more efficient to keep the numeric output in string form, incrementing each ASCII digit and emitting the string, rather than repeatedly converting from binary register contents to a string value.


Factory method to create two kinds of cars based on an enum I am implementing the factory method approach. My intention is to know if my approach is correct for factory method. Any modifications to be done to align with the approach?

First  I am creating an interface IAuto.

 //IAuto.cs
    namespace TestDesignPatterns.Auto
    {
        public interface IAuto
        {

            void start();
            void stop();
        }
    }

Create a class that implements the IAuto.

    //clsBMW.cs
    namespace TestDesignPatterns.Auto
    {
        public class clsBMW : IAuto
        {
            public void start()
            {
                Console.WriteLine("BMW started");
            }

            public void stop()
            {
                Console.WriteLine("BMW stopped");
            }
        }
    }

Create another concrete class for Audi implementing IAuto.

namespace TestDesignPatterns.Auto
{
    public class clsAudi : IAuto
    {
        public void start()
        {
            Console.WriteLine("Audi started");
        }

        public void stop()
        {
            Console.WriteLine("Audi stopped");
        }
    }
}

Create an Interface of factory which creates the model.

//IautoFactory.cs
public interface IAutoFactory
    {
       IAuto createModel(ModelType m);
    }

concrete factory class.

//clsAutofactory.cs
 public class clsAutoFactory : IAutoFactory
    {
        public IAuto createModel(ModelType m)
        {
            IAuto instance = null;
            switch (Convert.ToInt32(m))
            {
                case 0:
                    instance = new clsBMW();
                    break;

                case 1:
                   instance =  new clsAudi();
                    break;
                default:
                    break;

            }

            return instance;
        }
    }

Main Program

 public enum ModelType
    {
        BMW=0,
        Audi=1
    }
//program.cs

            IAutoFactory factory = new clsAutoFactory();
            IAuto model = factory.createModel(ModelType.Audi);
            model.start();
            model.stop();
            Console.ReadKey();

Do not name things with data type prefixes

clsAutoFactory.  No.  Instead: AutoFactory

Names should be what they are in the problem domain - autos in this case. How a thing is implemented - is it an array? a class? an integer? etc. is not what's important, we can read code and the IDE is real smart. Writing code that is descriptive is the thing.

Unnecessary interfaces

There is no reason for any interfaces at this point. Just inherit from Auto class. Half the code goes away! Perhaps make it an abstract class and override methods as needed. I bet there is common code and/or default behavior.

In general interfaces are for giving unrelated classes similar behavior. A Parrot might implement the Idogable so it can bark like a dog even though a Parrot is not a Dog. But a BMW is an Auto.

  I am implementing the factory method

You wrote a factory class not a factory method.

A factory method would be a method in some other class that does the factory thing. The code simple enough at this point that you could create a factory method - say a method in the Auto class; but I am not saying you should.

There are 3 "levels" of factory design which facilitates increasing construction complexity. i.e. factory method, factory (a simple class), and abstract factory for very complex things. The abstract factory may be appropriate if you were creating multiple auto makes with multiple models with multiple individual options; and perhaps different kinds like SUV, sports car, truck.

Ditto to what @JAD said

switch default

this is a perfect place for catching errors. If the default case is hit then obviously that model has not been implemented in your code. So capture that fact and let the user know.

Be more specific with auto instance names. model is too generic.

Not this:

IAuto model = factory.createModel(ModelType.Audi);
model.start();
model.stop();

but this:

IAuto myAudi = factory.createModel(ModelType.Audi);
myAudi.start();
myAudi.stop();


TapeEquilibrium Java Implementation A non-empty zero-indexed array A consisting of N integers is given.
  Array A represents numbers on a tape.

  Any integer P, such that 0 < P < N, splits this tape into two
  non-empty parts: A[0], A[1], ..., A[P − 1] and A[P], A[P + 1], ...,
  A[N − 1].

  The difference between the two parts is the value of:

|(A[0] + A[1] + ... + A[P − 1]) − (A[P] + A[P + 1] + ... + A[N − 1])|

  Write a function:

int solution(int A[], int N);

  that, given a non-empty zero-indexed array A of N integers, returns
  the minimal difference that can be achieved.

  Complexity:

  expected worst-case time complexity is \$\mathcal{O}(N)\$;
  expected worst-case space complexity is \$\mathcal{O}(N)\$, beyond input storage (not counting the storage required for input arguments).

Here is my code..

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

public class TapeEquilibium {

    public static void main(String[] args) {
        int[] a = {3,1,2,4,3};
        System.out.println(solution(a));
    }

    public  static int solution(int[] A){

        long totalSum = 0;

        List<Holder> list = new ArrayList<TapeEquilibium.Holder>();
        for(int i=0;i<A.length;i++){
            totalSum = totalSum + A[i];
        }
        long start = A[0];
        for(int i=1;i<A.length;i++){
            Holder holder = new Holder(i,Math.abs((2*start)-totalSum));
            list.add(holder);
            start = start+A[i];
        }

        Collections.sort(list);

        return (int)(list.get(0).sum);

    }

    static class Holder implements Comparable<Holder>{

        int p;
        long sum;

        public Holder(int p, long sum){
            this.p = p;
            this.sum = sum;
        }

        public int compareTo(Holder o) {
            if(this == o) return 0;
            else{
                return new Long(this.sum).compareTo(new Long(o.sum));
            }
        }
    }
}

Time complexity

The solution doesn't meet the required \$O(N)\$ complexity.
Can you spot why?

       Collections.sort(list);

As it stands, the time complexity of your implementation is \$O(N\log N)\$. To get the minimum element of a collection, do you really need to sort all the elements?

Unused values, unnecessary complication

The p field of Holder is not used in the compareTo implementation.
In general, it's suspicious when a class implements Comparable but compareTo doesn't use all the fields of the class.
In fact this field is not used anywhere in the program.
So you can simplify by removing it.
And once this field is gone,
Holder will have only one field left.
A class with a single field in it is highly suspicious.
Is it really useful?

With Holder eliminated, the implementation can become much simpler, cleaner and faster.


Counting vowels and consonants in a string The output should display as follows...

PROGRAM: Vowel count

Please, enter a text string: fred
4 characters
a e i o u
0 1 0 0 0
b c d f g h j k l m n p q r s t v w x y z
0 0 1 1 0 0 0 0 0 0 0 0 0 1 0 0 0 0 0 0 0

It works; but, I  believe is way over long...; maybe, I should have used arrays/or, ASCII count/-etc.?

vowels="aeiou"
aCount=eCount=iCount=oCount=uCount=0

consonants="bcdfghjklmnpqrstvwxyz"
bCount=cCount=dCount=fCount=gCount=hCount=jCount=kCount=lCount=mCount=nCount=oCount=pCount=qCount=rCount=sCount=tCount=uCount=vCount=wCount=xCount=yCount=zCount=0

print("PROGRAM: Vowel count\n")

aString=input("Please, enter a text string: ")

for eachChar in aString:

    if eachChar in vowels:
        if eachChar == "a": aCount+=1
        if eachChar == "e": eCount+=1
        if eachChar == "i": iCount+=1
        if eachChar == "o": oCount+=1
        if eachChar == "u": uCount+=1

    if eachChar in consonants:
        if eachChar == "b": bCount+=1
        if eachChar == "c": cCount+=1
        if eachChar == "d": dCount+=1
        if eachChar == "f": fCount+=1
        if eachChar == "g": gCount+=1
        if eachChar == "h": hCount+=1
        if eachChar == "j": jCount+=1
        if eachChar == "k": kCount+=1
        if eachChar == "l": lCount+=1
        if eachChar == "m": mCount+=1
        if eachChar == "n": nCount+=1
        if eachChar == "p": pCount+=1
        if eachChar == "q": qCount+=1
        if eachChar == "r": rCount+=1
        if eachChar == "s": sCount+=1
        if eachChar == "t": tCount+=1
        if eachChar == "v": vCount+=1
        if eachChar == "w": wCount+=1
        if eachChar == "x": xCount+=1
        if eachChar == "y": yCount+=1
        if eachChar == "z": zCount+=1

print(len(aString),"characters")

print("a","e","i","o","u")
print(aCount,eCount,iCount,oCount,uCount)

print("b","c","d","f","g","h","j","k","l","m","n","p","q","r","s","t","v","w","x","y","z")
print(bCount,cCount,dCount,fCount,gCount,hCount,jCount,kCount,lCount,mCount,nCount,pCount,qCount,rCount,sCount,tCount,vCount,wCount,xCount,yCount,zCount)

To count the occurrences of things, use collections.Counter.

The lowercase letters are available as a predefined constant string.ascii_lowercase.  You can use a generator expression to filter out the vowels and obtain the consonants.  PEP 8, the official style guide, suggests using ALL_CAPS as names for constants.

I've used the * operator when calling print() to treat each element of a tuple or list as a separate argument.

Note that your formatting will break when any character has more than 9 occurrences.

from collections import Counter
from string import ascii_lowercase

VOWELS = tuple("aeiou")
CONSONANTS = tuple(c for c in ascii_lowercase if c not in VOWELS)

print("PROGRAM: Vowel count\n")
s = input("Please, enter a text string: ")
counts = Counter(s)

print('{0} characters'.format(len(s)))
print(*VOWELS)
print(*[counts[c] for c in VOWELS])
print(*CONSONANTS)
print(*[counts[c] for c in CONSONANTS])


Calculate the midpoint of a polyline I have polylines in a GIS database. The polylines are stored using a user-defined type called ST_GEOMETRY.

ST_GEOMETRY has lots of spatial functions. However, it does not have a polyline midpoint function.

Therefore, I have written a custom PL/SQL midpoint function to fill this gap.

As a novice programmer, I'm wondering, how can the code be improved to be as fast and robust as possible?

-- execute this as the SDE user
create or replace function ST_MidPoint
   (line_in IN sde.st_geometry)
   RETURN sde.st_geometry
IS midpoint sde.st_geometry;
  srid integer;
  line_length number(38);
  num_parts integer;
  num_points integer;
  partNum integer;
  distanceAlong number(38);
  segmentLength number(38);
  part sde.st_geometry;
  p1 sde.st_geometry;
  p2 sde.st_geometry;
  x1 double precision;
  y1 double precision;
  x2 double precision;
  y2 double precision;
BEGIN
  -- get the SRID of the line for later use in constructing the midpoint geometrey
  select sde.st_srid (line_in)
  into srid
  from dual;

  -- calculate the total length of the line
  select sde.st_length(line_in)
  into line_length
  from dual;

  -- get the number of parts that make up the line
  select sde.st_numgeometries(line_in)
  into num_parts
  from dual;

  distanceAlong := 0;
  for partNum in 1..num_parts loop
    --dbms_output.put_line(partNum);
    -- get the geometry for this part
    select sde.st_geometryn(line_in, partNum)
    into part
    from dual;

    -- get the number of points that make up this part
    select sde.st_numpoints(part)
    into num_points
    from dual;
    --dbms_output.put_line(num_points);

    -- get the first point (the "from" point) for the part
    select sde.st_pointn (part, 1)
    into p1
    from dual;

    -- iterate along the line until the section that contains the midpoint is found
    for pointNum in 2..num_points loop
      -- get the "to" point of the segment
      select sde.st_pointn(part, pointNum)
      into p2
      from dual;

      -- calculate the distance between the from point and the to point
      select sde.st_distance(p1, p2)
      into segmentLength
      from dual;

      -- add the distance along this segment to the running total
      distanceAlong := distanceAlong + segmentLength;
      --dbms_output.put_line(distanceAlong);
      -- check to see if the running total is past the midpoint
      if distanceAlong >= line_length/2.0 then
        -- the two current points encompass the midpoint of the line
        -- determine the midpoint geometry and return it
        select sde.st_x(p1)
        into x1
        from dual;
        select sde.st_y(p1)
        into y1
        from dual;
        select sde.st_x(p2)
        into x2
        from dual;
        select sde.st_y(p2)
        into y2
        from dual;
        select sde.st_point((x1+x2)/2.0, (y1+y2)/2.0, srid)
        into midpoint
        from dual;
        -- the midpoint has been found, not need to interogate the rest of the line
        RETURN midpoint;
      end if;
      -- save the endpoint as the first point and continue down the line looking for the midpoint
      p1:=p2;
    end loop;
  end loop;

  return null;

EXCEPTION
WHEN OTHERS THEN
   raise_application_error(-20001,'An error was encountered - '||SQLCODE||' -ERROR- '||SQLERRM);
END;
/
grant execute on ST_MidPoint to public;

--select objectid,sde.ST_MidPoint(shape) geom
--from gis.line_test
--order by objectid;

Update:

I have abandoned this function. It works, but it's horrendously inefficient because it's not possible to use the SDE functions & operators to directly assign values to variables. I needed to wrap them in queries instead.

More information here:
Use SDE.ST_GEOMETRY functions in a custom function

Originally, I had thought that the code could be improved. However, now I realize that this is not possible--due to the aforementioned problem.

It would be my preference that this question be deleted (I tried but couldn't because there's an answer).

Check this function which returns the mid point X,Y in WKT format. Please note this function uses the SDO_LRS of Oracle which is part of Oracle Spatial.

create or replace function get_line_midpoint
   (line_in IN sde.st_geometry)
  -- RETURN sde.st_geometry
  RETURN VARCHAR2
IS
wkt_geometry clob;
ora_geometry sdo_geometry;
mid_x number(10,6);
mid_y number(10,6);
mid_point_geom sde.st_geometry;
BEGIN

SELECT  sde.ST_AsText(line_in) INTO wkt_geometry FROM DUAL;
ora_geometry := SDO_UTIL.FROM_WKTGEOMETRY(wkt_geometry);

--mid_x:= sdo_cs.transform(SDO_LRS.CONVERT_TO_STD_GEOM(SDO_LRS.LOCATE_PT(SDO_LRS.CONVERT_TO_LRS_GEOM(ora_geometry, 3), SDO_GEOM.SDO_LENGTH(ora_geometry,3)/2)),8307).SDO_POINT.X;
mid_x:= SDO_LRS.CONVERT_TO_STD_GEOM(SDO_LRS.LOCATE_PT(SDO_LRS.CONVERT_TO_LRS_GEOM(ora_geometry, 3), SDO_GEOM.SDO_LENGTH(ora_geometry,3)/2)).SDO_POINT.X;

--mid_y:= sdo_cs.transform(SDO_LRS.CONVERT_TO_STD_GEOM(SDO_LRS.LOCATE_PT(SDO_LRS.CONVERT_TO_LRS_GEOM(ora_geometry, 3), SDO_GEOM.SDO_LENGTH(ora_geometry,3)/2)),8307).SDO_POINT.Y;
mid_y:= SDO_LRS.CONVERT_TO_STD_GEOM(SDO_LRS.LOCATE_PT(SDO_LRS.CONVERT_TO_LRS_GEOM(ora_geometry, 3), SDO_GEOM.SDO_LENGTH(ora_geometry,3)/2)).SDO_POINT.Y;

ora_geometry := SDO_UTIL.FROM_WKTGEOMETRY('point ('|| mid_x || ' ' || mid_y ||')');

return 'point ('|| mid_x || ' ' || mid_y ||')';

EXCEPTION
WHEN OTHERS THEN
   raise_application_error(-20001,'An error was encountered - '||SQLCODE||' -ERROR- '||SQLERRM);
END;


Increment all values in a Map by 1 What's the neatest way to increment all values in a HashMap by 1? The map is <String, Integer>, but the key doesn't matter as every value will be incremented.

Is it "cleaner" to use lambdas with forEach/compute/etc. or just loop through the entries?

HashMap<String, Integer> map = mobCounter.get(mob);
for (Entry<String, Integer> e : map.entrySet()) {
    map.put(e.getKey(), e.getValue() + 1);
}

This doesn't look too messy to me but I'm wondering if people like seeing lambdas more.

You must not modify the keys of a HashMap while iterating over it. You are just lucky it worked.

Instead of the put(...), write e.setValue(e.getValue() + 1).

If you have some kind of Multiset available (e.g. when you are using Guava), you could replace your HashMap with a Multiset<String>, which will make your intention clearer.

Using lambdas, your code would look like:

map.replaceAll((k, v) -> v + 1);

This looks very nice to me. It cannot get any shorter.


Find if a part of the integer from another integer is divisible by 7? The question states:

  Given an integer A, and two indices Li, Ri. Find if the number from A which is bounded by the two indices is divisible by 7?

  For example if A = 357753
  indices Li = 3, Ri = 4, then the number is 77, which is divisible.

My program give a time limit exceeded when executed for larger numbers. Are there any other performance improvement can be done in my code. Or the whole method is wrong for the solution. Any help would be appreciated. "q" is the number of indices that will be provided as input.

First code (not optimized):

inp = raw_input()

q = input()

temp = ""

for i in range(q):

    l, r = map(int, raw_input().split())

    temp =  int(inp[l - 1:r])

    if temp % 7 == 0:

        print "YES"
    else:

        print "NO"

My second program is:

inp = input()

q = input()

temp = 0
length = len(str(inp)) - 1
for i in range(q):

    left, right = map(int, raw_input().split())

    temp = inp
    end = length - right + 1
    temp = temp / (10 ** end)
    mul = 1
    final = 0

    while right >= left:

        cur = temp % 10
        temp = temp / 10
        right -= 1
        #print cur
        final += (cur * mul)
        mul = mul * 10

    if final % 7 == 0:

        print "YES"
    else:

        print "NO"

I'm not sure why you think the second one is "optimized"; the former at least looks faster to me.

Don't name variables temp - it tells you nothing about the actual job of the variable. q is also a bad name'; num_iters is clearer, and i is not used so just use an underscore instead.

Don't pre-initialize values, in this case temp. Assign to it only when you need to.

Don't use input - always use raw_input and explicit conversions.

Then you have

inp = raw_input()
num_iters = int(raw_input())

for _ in range(num_iters):
    l, r = map(int, raw_input().split())
    temp = int(inp[l - 1:r])

    if temp % 7 == 0:
        print "YES"
    else:
        print "NO"

which is simple enough to basically be ideal. A more advanced programmer might want to handle errors or put code into functions, but the advantages of those only really apply to larger programs.


Public property, backing up by a private field, with caching and null checking inside I have a private field called individualProfile (camelCase) and I've created a public property called IndividualProfile (PascalCase) on it:

private Profile individualProfile;
public Profile IndividualProfile
{
    get
    {
        if (individualProfile == null)
        {
            individualProfile = ProfileFacade.Instance.GetMainProfileByType
                                (
                                    PortalContext.CurrentUserId,
                                    ProfileType.IrnicIndividual
                                );
            if (individualProfile == null)
            {
                individualProfile = new Profile()
                {
                    FirstName = string.Empty,
                    Family = string.Empty,
                    Details = new IrnicIndividual()
                    {
                        NationalCode = string.Empty
                    }
                };
            }
        }
        return individualProfile;
    }
}

I have to use this public property in almost 10 places, so I first check the private member to see if it's null and load it only once, so that I load it only once and I cache it and use it 10 times. Also, since GetMainProfileByType can return null value, I check another time to see if the private member is null or not. If it's null, I simply set it to an empty object, so that I won't bother checking null values when I bind this object to my view.

Is this code efficient and maintainable? Is there anything I've overlooked?

Good thinking about the caching and efficiency. I have two minor suggestions

1 - Enforce SRP (single responsibility principal), which basically states (according to Robert C. Martin) that in the world of "Clean Code" classes must have one reason to change for better maintainability, which is one of the concerns you had in your question. So to apply this to your case, the class that contains the Profile property need not be changed if you decide to change the signature of the ProfileFacade.Instance.GetMainProfileByType method, or maybe choose another method to populate the Profile. So maybe refactoring that code out of the class would be a good idea. Here is what I mean by that:

private Profile _individualProfile;
public Profile IndividualProfile
{
    get
    {
        if (_individualProfile != null) return _individualProfile;

        return (_individualProfile = ProfileFacade.Instance.GetIrnicProfile() ?? Profile.NullProfile);
    }
}

As you can see in the code snippet I used the short hand null check using the ?? null-coalescing  operator.

2 - NULL object pattern. Basically define the neutral state (or NULL state) of your object inside the object itself, benefiting from not changing the calling code if ou decide to redefine what the NULL means for your object. In my code snippet I added Profile.NullProfile which is defined as such:

public class Profile
{
    public string FirstName { get; set; }
    // Other properties here
    public static Profile NullProfile
    {
        get
        {
            return new Profile
                        {
                            FirstName = String.Empty,
                            // etc...
                        };
        }
    }
}

Other than that... Good work!


Generating matrix/image kernel from List I have another question from same program where is full code, but this question focus only single process/function of that program.

My problem is performance when creating dynamic Image kernel from list of pixels.

List contains multiple single 4 byte Integer which is grayscale of pixel.

I have tried 3 different approach, still not happy with the time what it takes.

I think my basic idea about generating matrices is wrong.

Approach 1

def getKernels(self, pixels, width):
    kernels = []
    pixels = [x for sublist in pixels for x in sublist] #2D list to 1D
    for i in range(0,len(pixels)):
        x = (i % width) #Calc pixel x-cordinate
        y = math.floor(i / width) #Calc y-cordinate
        try:
            kernel = [pixels[(x-1)+((y-1)*width)],pixels[(x)+((y-1)*width)],pixels[(x+1)+((y-1)*width)],pixels[(x-1)+(y*width)],pixels[(x)+(y*width)],pixels[(x+1)+(y*width)],pixels[(x-1)+((y+1)*width)],pixels[(x)+((y+1)*width)],pixels[(x+1)+((y+1)*width)]]
        except Exception as e:
            kernel = [0] #Cannot create fullsize kernel so just make it black
        kernels.append(kernel) #Add kernel to kernels list
    return kernels

Function execute time 0.2249772548675537 seconds
With this project I can live with that time, but kernel generation is ugly and "hardcoded" so I cant change size of it.

Approach 2

    def getKernels(self, pixels, width):
        kernels = []
        pixels = [x for sublist in pixels for x in sublist]  #2D list to 1D
        for i in range(0,len(pixels)):
            tx = (i % width) #Calc pixel x-cordinate
            ty = math.floor(i / width) #Calc y-cordinate
            kernel = []
            for y, x in self.kernelGenerator(ty,tx,3): #Pass current pixel index cordinates and size of kernel
                try:
                    index = (x)+((y)*width) #Calc index
                    if index > -1 and index < len(pixels):  #Check that index is in list bounds
                        kernel.append(pixels[index])
                    else: #Out of list bounds make pixel black
                        kernel.append(0)
                except Exception as e:
                    raise e
            kernels.append(kernel)
        return kernels

    def kernelGenerator(self, x, y, size):
        if size % 2 == 0 or size < 3: #Because pixel has to be "center" of my kernel
            print("Kernel size should be odd and greater than 2")
            raise
        kernel = []
        for i in range(math.ceil(-size/2),math.ceil(size/2)):
            for j in range(math.ceil(-size/2),math.ceil(size/2)):
                kernel.append((x+i,y+j))
        return kernel

Function execute time 1.236647605895996 seconds
Ok, now I can change my kernel size but costs time.

Approach 3

    def getKernels(self, pixels):
        kernels = []
        for ty, row in enumerate(pixels): #get pixel Y-cordinate and row
            for tx, px in enumerate(row): #get pixel X-cordinate and px
                kernel = []
                for y, x in self.kernelGenerator(ty,tx,3):
                    try:
                        kernel.append(pixels[y][x])
                    except Exception as e:
                        kernel.append(0)
                kernels.append(kernel)
        return kernels

    def kernelGenerator(self, x, y, size):
        if size % 2 == 0 or size < 3: #Because pixel has to be "center" of my kernel
            print("Kernel size should be odd and greater than 2")
            raise
        kernel = []
        for i in range(math.ceil(-size/2),math.ceil(size/2)):
            for j in range(math.ceil(-size/2),math.ceil(size/2)):
                kernel.append((x+i,y+j))
        return kernel

Function execute time 0.8910479545593262 seconds
Approach 3 is now best. Comments got me thinking this again, still execute time is IMO too much.

Any suggestions to improve it?

Style

put spaces after comas to improve readability;
remove unnecessary parenthesis;
put spaces around operators;
put two spaces before and one space after the comment sign (#);
remove the 0 in range(0, <whatever>) as it is the default value;
except Exception as e is bad here as you are only actually expecting IndexError; plus the as e part is unnecessary as you don't use e (well, you do in the second snippet, but you could just raise with nothing more, even though the whole except: raise is useless as it adds nothing over the default behaviour);
range(len(<something>)) is often a red flag as it is better to iterate over elements than indices; but here, I don't think you have a choice anyway;
divmod can help you get rid of math.ceil.

In fact, only changing your first snippet to using divmod yield a 5% speedup on my machine:

def get_kernel(pixels, width):
    kernels = []
    pixels = [x for row in pixels for x in row]  # 2D list to 1D
    for i in range(len(pixels)):
        y, x = divmod(i, width)
        try:
            kernel = [
                    pixels[(y-1) * width + x - 1], pixels[(y-1) * width + x], pixels[(y-1) * width + x + 1],
                    pixels[y * width + x - 1], pixels[y * width + x], pixels[y * width + x + 1],
                    pixels[(y+1) * width + x - 1], pixels[(y+1) * width + x], pixels[(y+1) * width + x + 1],
            ]
        except IndexError:
            kernel = [0]
        kernels.append(kernel)  # Add kernel to kernels list
    return kernels

Algorithm

There is one thing that bothers you about this snippet: it is not generic enough to allow for kernels of various radius. There is an other thing that bothers me, it is how it warps around the edges of the image but the bottom. The fact that some_list[-x] will return the \$x^{th}\$ element of some_list starting from the end combined to the flattening of the list will make it so that when at the top row or when reaching a pixel at the right or the left of the image, the coordinates involved will always be a valid index in pixels. But for the last row, (y+1) * width + x will exceed the number of elements in pixels and raise IndexError. This inconsistent behaviour bothers me.

Looking at how you handle your kernels in your other question, we can see that kernels that are not full-sized are discarded. Great, let's generate kernels of lower size for the edges of the image. The key idea here is knowing that slicing a list will always return a list:

>>> a = list(range(20))
>>> a[4:8]
[4, 5, 6, 7]
>>> a[18:50]
[18, 19]
>>> a[-8:5]
[]

Also, slicing a list of lists will not copy inner list but only references to them, so it is rather fast:

def get_kernels(pixels, width, radius=1):
    kernels = []
    for y in range(len(pixels)):
        y_min = y - radius
        y_max = y + radius + 1
        for x in range(width):
            x_min = x - radius
            x_max = x + radius + 1
            kernels.append([
                    pixel for row in pixels[y_min:y_max]
                    for pixel in row[x_min:x_max]])
    return kernels

This version is 5 to 10% slower than your first one but has two advantages:

It is more readable;
It is more generic.

I chose to parametrize it with the kernel radius rather than its diameter to avoid the parity check and simplify the computations.

I also chose to keep the width parameter since you seemed to already have it handy. But if you want to remove it, you can still add:

def get_kernels(pixels, radius=1):
    if not pixels:
        return []
    width = len(pixels[0])

at the beginning of the function.

Going further

Your use case of such function seems only to be able to iterate over the returned value. In such case, you would gain more performances by turning this into a generator instead of building a list element by element.

The code is nearly identical, just remove the kernels list and use the yield keyword instead of the append function. And voilà. What it does is that it will compute the kernels on demand, during the iteration; removing the need for extra storage (and handling of that storage):

def get_kernels(pixels, radius=1):
    if not pixels:
        return

    width = len(pixels[0])
    for y in range(len(pixels)):
        y_min = y - radius
        y_max = y + radius + 1
        for x in range(width):
            x_min = x - radius
            x_max = x + radius + 1
            yield [
                pixel for row in pixels[y_min:y_max]
                for pixel in row[x_min:x_max]]

Oh yes, you might have noticed that I removed the self parameter as it is unused. This might interfere with the fact that you are using this as a method on your class. But I probably should explain what is wrong with that in that other question of yours.


Delete duplicates from unsorted linked list E.g: if a linked list contains 2->3->1->3->4 then the result should be 2->3->1->4. Notice that 3 has been eliminated.

This code is attributed to GeeksforGeeks.  I'm looking for code-review, optimizations and best practices.

public class EliminateDuplicateUnsortedLL<T> {

    private Node<T> first;
    private Node<T> last;

    public EliminateDuplicateUnsortedLL(List<T> items) {
        for (T item : items) {
            add(item);
        }
    }

    public void add(T item) {
        Node<T> node = new Node<>(item);
        if (first == null) {
            first = last = node;
        } else {
            last.next = node;
            last = node;
        }
    }

    private static class Node<T> {
        private Node<T> next;
        private T item;

        Node(T item) {
            this.item = item;
        }
    }

    public void eliminateDuplicates() {
        final Set<T> set = new HashSet<T>();
        Node<T> prev = null;
        for (Node<T> x = first; x != null; x = x.next) {
            if (set.contains(x.item)) {
                prev.next = x.next;
            } else {
                set.add(x.item);
                prev = x;
            }
        }
        last = prev;
    }

    public List<T> toList() {
        final List<T> list = new ArrayList<>();

        for (Node<T> x = first; x != null; x = x.next) {
            list.add(x.item);
        }

        return list;
    }

    @Override
    public int hashCode() {
        int hashCode = 1;
        for (Node<T> x = first; x != null; x = x.next)
            hashCode = 31*hashCode +  x.hashCode();
        return hashCode;
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj)
            return true;
        if (obj == null)
            return false;
        if (getClass() != obj.getClass())
            return false;
        EliminateDuplicateUnsortedLL<T> other = (EliminateDuplicateUnsortedLL<T>) obj;
        Node<T> currentListNode = first;
        Node<T> otherListNode =  other.first;

        while (currentListNode != null && otherListNode != null) {
            if (currentListNode.item != otherListNode.item) return false;
            currentListNode = currentListNode.next;
            otherListNode = otherListNode.next;
        }
        return currentListNode == null && otherListNode == null;
    }
}

Unit tests:

public class EliminateDuplicateUnsortedLLTest {

    @Test
    public void test1() {
        EliminateDuplicateUnsortedLL<Integer> edu1 = new EliminateDuplicateUnsortedLL<Integer>(Arrays.asList(1, 2, 3, 4, 5, 4, 3, 7));
        edu1.eliminateDuplicates();

        EliminateDuplicateUnsortedLL<Integer> eduExpected1 = new EliminateDuplicateUnsortedLL<Integer>(Arrays.asList(1, 2, 3, 4, 5, 7));
        assertEquals(eduExpected1, edu1);
    }

    @Test
    public void test2() {
        EliminateDuplicateUnsortedLL<Integer> edu2 = new EliminateDuplicateUnsortedLL<Integer>(Arrays.asList(7, 2, 5, 10, 4, 2, 9, 10));
        edu2.eliminateDuplicates();

        EliminateDuplicateUnsortedLL<Integer> eduExpected2 = new EliminateDuplicateUnsortedLL<Integer>(Arrays.asList(7, 2, 5, 10, 4, 9));
        assertEquals(eduExpected2, edu2);
    }

    @Test
    public void testAllDuplicates() {
        EliminateDuplicateUnsortedLL<Integer> edu3 = new EliminateDuplicateUnsortedLL<Integer>(Arrays.asList(2, 2, 2, 2, 2));
        edu3.eliminateDuplicates();

        EliminateDuplicateUnsortedLL<Integer> eduExpected3 = new EliminateDuplicateUnsortedLL<Integer>(Arrays.asList(2));
        assertEquals(eduExpected3, edu3);
    }

}

I also am not a fan of the code organization. I think a better way to approach this would be to implement the List interface around an ordered set like a LinkedHashSet as something like a SetList. Many of the methods could simply be delegated to the internal set, and only a few would really be left to implement.

Your code has a separate method, eliminateDuplicates, to filter out the duplicates, rather than filtering them as they are added. This means that, if that method is not called, the list is not consistent with its requirements. I would still get all the duplicates. A better strategy is to remove the duplicates as they are added to keep the list always consistent with its advertised behavior.

Lastly, the names of your test methods could use a little work. Names like test1 and test2 don't tell much of what the tests are or what they are testing.

Here's an example of such an implementation. It implements both List<T> and Set<T>, and so can be used in either place. I used two internal structures for simplicity.

public final class LinkedHashSetList<E> implements Set<E>, List<E>
{
    private Set<E> set;
    private List<E> list;

    public LinkedHashSetList() {
        set = new HashSet<>();
        list = new LinkedList<>();
    }

    public LinkedHashSetList(Collection<? extends E> c) {
        // LinkedHashSet so I don't have to track uniqueness and order
        set = new LinkedHashSet<>(c);
        list = new LinkedList<>(set);
    }

    @Override
    public boolean add(E el) {
        if(set.contains(el)) {
            return false;
        }

        list.add(el);
        set.add(el);
        return true;
    }

    // Optional method not implemented
    @Override
    public void add(int index, E element) {
        throw new UnsupportedOperationException("LinkedHashSetList#add(int, E) not implemented!");
    }

    @Override
    public boolean addAll(Collection<? extends E> c) {
        boolean isChanged = false;

        for(E el: c) {
            isChanged = isChanged || add(el);
        }

        return isChanged;
    }

    // Optional method not implemented
    @Override
    public boolean addAll(int index, Collection<? extends E> c) {
        throw new UnsupportedOperationException("LinkedHashSetList#addAll(int, Collection<? extends E> c) not implemented!");
    }

    // Optional method not implemented
    @Override
    public boolean retainAll(Collection<?> c) {
        throw new UnsupportedOperationException("LinkedHashSetList#retainAll(Collection<?> c) not implemented!");
    }

    @Override
    public E remove(int index) {
        E el = list.remove(index);
        set.remove(el);
        return el;
    }

    @Override
    public boolean remove(Object el) {
        list.remove(el);
        return set.remove(el);
    }

    @Override
    public boolean removeAll(Collection<?> c) {
        list.removeAll(c);
        return set.removeAll(c);
    }

    @Override
    public void clear() {
        list.clear();
        set.clear();
    }

    // Optional method not implemented
    @Override
    public E set(int index, E el) {
        throw new UnsupportedOperationException("LinkedHashSetList#set(int, E) not implemented!");
    }

    @Override
    public E get(int index) {
        return list.get(index);
    }

    @Override
    public boolean contains(Object el) {
        return set.contains(el);
    }

    @Override
    public boolean containsAll(Collection<?> c) {
        return set.containsAll(c);
    }

    @Override
    public int indexOf(Object el) {
        return list.indexOf(el);
    }

    @Override
    public int lastIndexOf(Object el) {
        return list.indexOf(el);
    }

    // WARNING: Changes to sublist can affect this list in undefined ways
    @Override
    public List<E> subList(int fromIndex, int toIndex) {
        return list.subList(fromIndex, toIndex);
    }

    @Override
    public boolean isEmpty() {
        return list.isEmpty();
    }

    @Override
    public int size() {
        return list.size();
    }

    // Not safe if remove is called on iterator
    @Override
    public Iterator<E> iterator() {
        return list.iterator();
    }

    // Not safe if remove is called on iterator
    @Override
    public ListIterator<E> listIterator() {
        return list.listIterator();
    }

    // Not safe if remove is called on iterator
    @Override
    public ListIterator<E> listIterator(int index) {
        return list.listIterator(index);
    }

    @Override
    public Object[] toArray() {
        return list.toArray();
    }

    @Override
    public <T> T[] toArray(T[] a) {
        return list.toArray(a);
    }
}

Unit Tests:

public class LinkedHashSetListTest
{
    @Test
    public void testRemovesDuplicates() {
        LinkedHashSetList<Integer> lhsl = new LinkedHashSetList<>(Arrays.asList(1, 1, 2, 2, 3, 3, 1, 7, 4, 6, 6));
        Integer[] expected = { 1, 2, 3, 7, 4, 6 };
        assertArrayEquals(expected, lhsl.toArray(new Integer[expected.length]));
    }

    @Test
    public void testRemovesNothingWhenNoDuplicates() {
        LinkedHashSetList<Integer> lhsl = new LinkedHashSetList<>(Arrays.asList(1, 2, 3, 7, 4, 6));
        Integer[] expected = { 1, 2, 3, 7, 4, 6 };
        assertArrayEquals(expected, lhsl.toArray(new Integer[expected.length]));
    }

    @Test
    public void testEmptyList() {
        LinkedHashSetList<Integer> lhsl = new LinkedHashSetList<>(new LinkedList<Integer>());
        Integer[] expected = {};
        assertArrayEquals(expected, lhsl.toArray(new Integer[expected.length]));
    }
}

Caveats regarding sublists and iterators are noted. The solution to the iterators would be to create a new Iterator specifically for this class. Sublisting is a bit more complicated to solve, since the sublist is backed by the original list.


Twin Prime Algorithm Optimization I have wrote the following algorithm to print out the Nth twin prime numbers:

def isprime(x):
    for i in range(2, (x + 2) / 2):
        if x % i == 0:
            return False
    return True

inputIndex = int(raw_input())
tempIndex = 1
n=2
while True:
    if isprime(n) & isprime(n + 2):
        if inputIndex == tempIndex:
            print "%d,%d" % (n,n+2)
            break
        else:
            tempIndex += 1
    n += 1

I am looking to optimize the speed and cannot import libraries like math and so or store a list of primes.

A better isprime check

At the moment, your function returns that 0 and 1 are prime, this is not what I have learnt.

A faster isprime check

You check primality by testing divisibility of various candidates between 2 and (x + 2)/2 (non included). You'd better check for candidates between 2 and sqrt(x) (included) as the smallest divisisor will be no bigger than this.

Here's the corresponding code from my toolbox :

def is_prime(n):
"""Checks if a number is prime."""
if n < 2:
    return False
return all(n % i for i in range(2, int(math.sqrt(n)) + 1))

Itertools is good

Instead of going through n by incrementing it, you can use itertools.count

for n in itertools.count(2):
    if isprime(n) & isprime(n + 2):
        if inputIndex == tempIndex:
            print "%d,%d" % (n,n+2)
            break
        else:
            tempIndex += 1

A bit of math

You know that twin primes will be odd (if you are not convinced of this, just think about it). Therefore, you can limit yourself to the search of odd numbers : for n in itertools.count(3, 2):.

Don't check things twice

At the moment, the primality of the same number is checked twice. A way not to do so would be to generate prime numbers and to check this to generate twin pairs.

def yield_odd_primes():
    for n in itertools.count(3, 2):
        if isprime(n):
            yield n

def yield_twin_pairs():
    p1, p2 = 0, 0
    for p in yield_odd_primes():
        p1, p2 = p2, p
        if p1 + 2 == p2:
            yield p1, p2

inputIndex = 1500
for i, (p1, p2) in enumerate(yield_twin_pairs()):
    if i + 1 == inputIndex:
        print "%d,%d" % (p1, p2)
        break

Better user experience

When I ran your code for the first time, nothing happened and I thought Python was just crunching numbers. After a while, I had a look at the code and realised that the script was waiting for some input.
By adding a prompt, you make this much clearer :

inputIndex = int(raw_input('Please enter an index: '))

Code organisation

At the moment, when one imports your code, he gets the prompt and everything. The usual way to do is to put the code "actually doing something" behind an if-main guard.

More code organisation

Your can split your code into smaller function. I found quite useful writing the following function to take the nth element of an iterable :

def get_nth_element(iterable, index):
    for i, e in enumerate(iterable):
        if i == index:
            return e

Tests

For such an algorithmic oriented problem, it is easy and a good habit to write test to ensure you do not break anything as you go through modifications.

The hard part is sometimes to rewrite your code in such a way that testing is made easy. It is now the case with my final version of the code.

Final code

import math
import itertools

def isprime(n):
    if n < 2:
        return False
    return all(n % i for i in range(2, int(math.sqrt(n)) + 1))

def yield_odd_primes():
    for n in itertools.count(3, 2):
        if isprime(n):
            yield n

def yield_twin_pairs():
    p1, p2 = 0, 0
    for p in yield_odd_primes():
        p1, p2 = p2, p
        if p1 + 2 == p2:
            yield p1, p2

def get_nth_element(iterable, index):
    assert index >= 0
    for i, e in enumerate(iterable):
        if i == index:
            return e

if __name__ == "__main__":
    inputIndex = int(raw_input('Please enter an index: ')) - 1
    print(get_nth_element(yield_twin_pairs(), inputIndex))

Some math to make the code faster

At the moment, we check the primality of each and every odd numbers. We can do better by considering division by 6 : all numbers can be written :

6k + 0 -> divisible by 2 & 3
6k + 1 -> potential prime
6k + 2 -> divisible by 2
6k + 3 -> divisible by 3
6k + 4 -> divisible by 2
6k + 5 -> potential prime

Thus, except for the obvious pair (3, 5), the only way two prime numbers can be separated by two are if they can be written (6*k + 5, 6*k + 7).

Once you have this, the code pretty much writes itself :

def yield_twin_pairs():
    yield (3, 5)
    for i in itertools.count(5, 6):
        if isprime(i) and isprime(i+2):
            yield (i, i+2)


Excel VBA macro loops through 20-28 sheets taken from filenames in cells and pastes them to the main workbook My apologies for a "repost", my original post was only partial and confusing. Here is a better snippet. This runs fairly slow (30 seconds or so) cycling through 20 workbooks. Any suggestions for improvement would be appreciated; I have left out my dimming of variables at the beginning, as well as my error handling portions, as those all work just fine.

For even MORE details, the user enters file paths into a column of up to 28 filepaths, and then the adjacent column derives the file name from the path. This macro, once initiated via a command button, begins cycling through all those filepaths, opening the document, copying and pasting it to the main workbook, until it reaches an empty cell in the column, to which then it stops.

There are also a few other small parts in there, like updating the last modified date when the button is clicked, as well as taking information from the first opened sheet to let the user know (on the main page) what kind of information is being pulled or has been pulled. Additionally, I often include frequent status bar updates for my impatient users who don't like staring at a spinning cursor without being told what's going on.

Private Sub CommandButton19_Click()

Application.DisplayStatusBar = True
Application.ScreenUpdating = False 'run faster
Application.DisplayAlerts = False 'prevents clipboard prompt
Application.Cursor = xlWait

On Error GoTo HandleIt

    Dim RangeDifferentiator As Integer
    Dim i As Long
    Dim SheettoCopyLoop
    Dim FileToCopyLoop
    Dim FileNameLoop

For i = 1 To 28 'start loop (28 because the last row being used is 34. If we need more rows this needs to change)

    RangeDifferentiator = i + 5
    Sheets("Admin").Select
    SheettoCopyLoop = Range("D" & RangeDifferentiator)
    FileToCopyLoop = Range("E" & RangeDifferentiator)
    FileNameLoop = Range("F" & RangeDifferentiator)

If FileToCopyLoop <> "" Then

    Application.StatusBar = "Pulling data for " & SheettoCopyLoop & "..." 'let's us know which one it's pulling
    Sheets(SheettoCopyLoop).Cells.ClearContents
    Workbooks.Open Filename:= _
        FileToCopyLoop
    'Needs optimizing
    Sheets("Summary").Cells.Copy

    'Establishes if this is a MTD or YTD file and up to what month it's pulling
    If i = 1 Then
        Workbooks("YTD File.xlsm").Sheets("SM Summaries").Range("C2").Value = ActiveWorkbook.Sheets("Summary").Range("C6").Value
        Workbooks("YTD File.xlsm").Sheets("SM Summaries").Range("C4").Value = ActiveWorkbook.Sheets("Summary").Range("C5").Value
    End If

    'Needs optimizing
    Windows("YTD File.xlsm").Activate
    Sheets(SheettoCopyLoop).Select
    Sheets(SheettoCopyLoop).Range("A1").Select
    ActiveSheet.Paste
    ActiveSheet.Range("A1").Select
    Windows(FileNameLoop).Close
    Sheets("Admin").Select
    Range("A1").Select

Else
Application.StatusBar = "READY"
Application.DisplayAlerts = True
Application.ScreenUpdating = True
Application.Cursor = xlDefault
'Show last modified date on admin page for the user
Workbooks("YTD File.xlsm").Sheets("Admin").Range("F1").Value = "Last modified: " & Date
MsgBox "Data pull complete!", vbInformation, "Success!"
Exit Sub
End If

Next i
Exit Sub

'The following is all error handling to help show what went wrong and where
HandleIt:
If SheettoCopyLoop <> "" Then
errorhandler = MsgBox("Oops! An error has occured on page " & SheettoCopyLoop & ". Check to make sure file exists/file path is correct.", vbExclamation, "Error!")
Application.StatusBar = "Error occured on page " & SheettoCopyLoop 'Show where error occured on status bar if applicable
Else
errorhandler = MsgBox("Oops! An unknown error has occured.", vbCritical, "Error!")
Application.StatusBar = "Unknown error"
End If
Worksheets("Admin").Activate
Range("H7").Select
Application.DisplayAlerts = True
Application.ScreenUpdating = True
Application.Cursor = xlDefault
Exit Sub
End Sub

I'm not sure why it takes 30 seconds to process the 20 files, because with my almost empty test files, your code only takes 2.202 seconds (21 files). I'm guessing those files are quite large, contain a lot of objects, charts, data (?)

If they have a lot of rows I'd suggest changing them to the xlsb format, which will compress them quite a bit (size could go down to 40%), and they will be a lot faster to open and close

  XLSB Format

  XLSB is the Excel binary format. It is structured as a compressed
  folder that contains a large number of binary files. It is much more
  compact than the XLS format, but the amount of compression very much
  depends on the contents of the workbook. For example, ten workbooks
  show a size reduction factor ranging from two to eight with an average
  reduction factor of four...

Another consideration is that interacting with files over the network is considerably slower than the local drive

Related to the code, there are a few basic issues:

ALWAYS Use "Option Explicit"
You almost never need the Select and Activate methods

Impact performance, and all you need to do can be accomplished without them

To properly replace the select statements you need to refer to all objects explicitly:

  Example - instead of

Windows("YTD File.xlsm").Activate
Sheets(SheettoCopyLoop).Select
Sheets(SheettoCopyLoop).Range("A1").Select
ActiveSheet.Paste

  use:

Windows("YTD File.xlsm").Sheets(SheettoCopyLoop).Range("A1").Paste

Indent your code to make it easier to read, and distinguish hierarchies of logic and nesting
It's better to prevent all errors than to rely on the "On Error GoTo" statements

There are few errors that cannot be avoided
The GoTo statement breaks the flow and structure of the code

Turn off Application.Calculation (to manual) if possible
Don't force the user to dismiss dialog boxes after every iteration

They only need to be aware if their task cannot be completed

I made some changes, such as copying the entire sheets to the main Workbook, instead of copying only the data, so here is how I would change your code:

Option Explicit

Private Sub CommandButton19_Click2()
    Dim wb As Workbook, ws As Worksheet, newWB As Workbook
    Dim wsSMS As Worksheet, wsS As Worksheet, wsNew As Worksheet
    Dim fileInfo As Variant, sheetToCopy As String, r As Long
    Dim fileToCopy As String, fileName As String, t As Double

    t = Timer

    OptimizeXl True, xlWait

    Set wb = ThisWorkbook
    Set ws = wb.Worksheets("Admin")
    Set wsSMS = wb.Worksheets("SM Summaries")

    fileInfo = ws.Range("D6:F34").Value2

    For r = 1 To UBound(fileInfo)               'max xl sheet name length = 31 chars
        sheetToCopy = Left(fileInfo(r, 1), 31)  'column 1 of Range("D6:F34") (Col D)
        fileToCopy = fileInfo(r, 2)             'column 2 of Range("D6:F34") (Col E)
        fileName = fileInfo(r, 3)               'column 3 of Range("D6:F34") (Col F)

        'Continue only if all necessary file details are not empty
        If Len(fileToCopy) > 0 And Len(sheetToCopy) > 0 And Len(fileName) > 0 Then
            If Len(Dir(fileToCopy)) > 0 Then    'continue only if file exists
                Application.StatusBar = "Pulling data for " & fileName & "..."

                Set newWB = Workbooks.Open(fileToCopy, False, True) 'NoLinks, ReadOnly

                If WsExists(newWB, "Summary") Then
                    'Delete existing sheet named "sheetToCopy"
                    If WsExists(wb, sheetToCopy) Then wb.Worksheets(sheetToCopy).Delete
                    Set wsS = newWB.Worksheets("Summary")
                    If r = 1 Then
                        wsSMS.Range("C2").Value = wsS.Range("C6").Value
                        wsSMS.Range("C4").Value = wsS.Range("C5").Value
                    End If
                    'Copy new sheet into this workbook, and rename it to "sheetToCopy"
                    wsS.Copy After:=wb.Worksheets(wb.Worksheets.Count)
                    Set wsNew = wb.Worksheets(wb.Worksheets.Count)
                    wsNew.Name = sheetToCopy
                Else
                    Debug.Print "Invalid sheet name for file on row " & r
                End If
                newWB.Close False
            Else
                Debug.Print "Invalid fileToCopy on row " & r & " (" & fileToCopy & ")"
            End If
            ws.Range("F1").Value2 = "Last modified: " & Date
        Else
            Debug.Print "Empty fileToCopy, sheetToCopy, or fileName on row " & r
        End If
    Next
    ws.Activate
    OptimizeXl False
    Application.StatusBar = "READY"
    Debug.Print "V2 Time: " & Format(Timer - t, "0.000") & " sec"
    MsgBox "Data pull complete!", vbInformation, "Success!"
End Sub

Private Sub OptimizeXl(ByVal stat As Boolean, Optional ByVal curs As Long = xlDefault)
    Application.Calculation = IIf(stat, xlCalculationManual, xlCalculationAutomatic)
    Application.ScreenUpdating = Not stat
    Application.DisplayAlerts = Not stat
    Application.Cursor = curs
End Sub

Private Function WsExists(ByRef wb As Workbook, ByVal wsName As String) As Boolean
    Dim ws As Worksheet
    If Not wb Is Nothing Then
        If Len(wsName) > 0 Then
            For Each ws In wb.Worksheets
                If ws.Name = wsName Then
                    WsExists = True
                    Exit Function
                End If
            Next
        End If
    End If
End Function

Test results:

V1 Time: 2.202 sec (your code)
V2 Time: 1.012 sec


Return Nth Weekday of Month My goal is to obtain the nth weekday of a given month. Parameters are a date from a given month and the nth weekday I'm trying to obtain. It returns the Nth Weekday of the month if it exist and returns DateTime.MinValue othewise. For my purposes weekdays are M-F.

Is there a more elegant or efficient way to implement this logic? My implementation feels crude, basically a while loop that counts weekdays until it finds the desired date or the end of the month.

    public static DateTime getNthWeekdayOfMonth(DateTime date, int nthWeekday)
    {
        //Valid inputs are greater than 0 but less than 24
        if (nthWeekday > 23 || nthWeekday < 1)
            return DateTime.MinValue;

        //start with 1st day of month from date param
        DateTime currentDay = new DateTime(date.Year, date.Month, 1);

        int i = 0;
        while(i < nthWeekday && currentDay.Month == date.Month)
        {
            if (currentDay.DayOfWeek != DayOfWeek.Saturday && currentDay.DayOfWeek != DayOfWeek.Sunday)
            {
                i++;
                if(i == nthWeekday)
                    return currentDay;
            }
            currentDay = currentDay.AddDays(1);
        }
        return DateTime.MinValue;
    }

Usage

var date1 = getNthWeekdayOfMonth(new DateTime(2015, 8, 11), 22);
//returns DateTime.MinValue
var date2 = getNthWeekdayOfMonth(new DateTime(2015, 8, 11), 21);
//returns 8/31/2015
var date3 = getNthWeekdayOfMonth(new DateTime(2015, 8, 11), 1);
//returns 8/3/2015

int i = 0;
    while(i < nthWeekday && currentDay.Month == date.Month)
    {
        if (currentDay.DayOfWeek != DayOfWeek.Saturday && currentDay.DayOfWeek != DayOfWeek.Sunday)
        {
            i++;
            if(i == nthWeekday)
                return currentDay;
        }
        currentDay = currentDay.AddDays(1);
    }

Your while loop is a little messy let's clean that up first, and you should also have an early continue statement in there.

Let me show you what I mean:

int i = 1;
while(i <= nthWeekday && currentDay.Month == date.Month)
{
    bool isWeekendDay = currentDay.DayOfWeek == DayOfWeek.Saturday || currentDay.DayOfWeek == DayOfWeek.Sunday;

    if (isWeekendDay)
    {
        currentDay = currentDay.AddDays(1);
        continue;
    }
    if (i == nthWeekday)
    {
        return currentDay;
    }
    currentDay = currentDay.AddDays(1);
    i++;
}

This is a lot cleaner, and shows exactly what our intent is.  It is much easier to read.

You shouldn't nest if statements inside of other if statements if you don't have to, it can get very confusing.


Updating my package version using only a Makefile So I have written a Makefile that has a release function that can be run like: make release bump=patch and I want it to be fully executable in the Makefile. The only issue with my current implementation is that there is a lot of redundancy and long lines of commands, which I think can be simplified a lot, although I haven't found any better solutions yet (mainly because of the incrementation). It then tags and pushes.

The required functionality is being able to pass in patch,minor,major and have it overwrite the current file with the new version commit that and push the new tag.

It just has that 'gross' feeling, which I strive to avoid.

My current makefile looks like:

release:
    $(eval v := $(shell git describe --tags --abbrev=0 | sed -E -e 's/^v//' -e 's/(.*)-.*/\1/'))
ifeq ($(bump),patch)
    @echo "$v" | grep -Eo '[0-9]+$$' | tail -n 1 | awk '{print $$1 + 1}' | xargs -I '{}' sed -i '' -E 's/([0-9]+"$$)/{}"/' ._version_.py
else ifeq ($(bump),minor)
    @echo "$v" | grep -Eo '(\.[0-9]+\.)' | awk '{print $$1 + 1}' | xargs -I{} sed -i '' -E 's/(\.[0-9]+\.)/.{}./' ._version_.py
else ifeq ($(bump),major)
 @echo "$v" | grep -Eo '([0-9]+)' | head -n 1 | awk '{print $$1 + 1}' | xargs -I{} sed -i '' -E 's/([0-9]+)/{}/' ._version_.py
endif
    $(eval a := $(shell grep -Eo '([0-9]+\.[0-9]+\.[0-9]+)' ._version_.py))
    @git tag "$a"
    @git commit -am "Bumped to version $a"
    @git push --tags

I think there's an easier way to get this done:

Extract the semver part from the tag (as you already did)
Use conditionals to determine the field to increment
Use a single Awk to increment the selected field

For example:

release:
    $(eval v := $(shell git describe --tags --abbrev=0 | sed -Ee 's/^v|-.*//'))
ifeq ($(bump), major)
    $(eval f := 1)
else ifeq ($(bump), minor)
    $(eval f := 2)
else
    $(eval f := 3)
endif
    @echo $(v) | awk -F. -v OFS=. -v f=$(f) '{ $$f++ } 1'

I would add some validation in case the version doesn't have major, minor, patch components.


Searching for categories and storing them This checks each row for a category. It stores ONE version of the category, ignore duplications. It then counts how many times a category appears.

There are a lot of for loops and a lot of table-checking. This takes time as the table is quite large. If it's possible, I would like to incorporate some of the loops to reduce time spent.

public partial class Stats : Form
{
    public Stats(ref TabControl languageTabs)
    {
        InitializeComponent();

        for (int i = 0; i < languageTabs.TabCount; i++)
        {
            Control[] ctrls = languageTabs.TabPages[i].Controls.Find(languageTabs.TabPages[i].Name + "Grid", true);
            DataGridView dgv = ctrls[0] as DataGridView;

            int charCount = 0;
            for (int j = 0; j < dgv.RowCount; j++)
            {
                charCount += (int)dgv.Rows[j].Cells[4].Value;
            }

            treeView1.Nodes.Add(languageTabs.TabPages[i].Text.ToUpper() + " Statistics");
            treeView1.Nodes[i].Nodes.Add("Total Number Of Strings: " + dgv.RowCount); // might need to remove '--' from count
            treeView1.Nodes[i].Nodes.Add("Total Number Of Chars: " + charCount);
            treeView1.Nodes[i].Nodes.Add("Total Number Of Strings and Chars per Category: ");

            //check for all different categories
            List<string> categories = new List<string>();

            int count = 0;
            for (int j = 0; j < dgv.RowCount; j++)
            {
                // look at every category in the dgv and see if we have a unique value in a container
                List<string> result = categories.FindAll(
                    delegate(string b)
                    {
                        return b == dgv.Rows[j].Cells[1].Value.ToString();
                    }
                );

                // if we don't have a unique ID then add it
                if (result.Count == 0)
                {
                    categories.Add(dgv.Rows[j].Cells[1].Value.ToString());
                }
            }

            int[] categoryCount = new int[categories.Count];

            foreach (string cat in categories)
            {
                for (int j = 0; j < dgv.RowCount; j++)
                {
                    if (dgv.Rows[j].Cells[1].Value.ToString() == cat)
                    {
                        categoryCount[count]++;
                    }
                }
                count++;
            }

            int countTwo = 0;
            foreach (string cat in categories)
            {
                treeView1.Nodes[i].Nodes[2].Nodes.Add(cat + ":   " + categoryCount[countTwo].ToString());
                countTwo++;
            }

        }
    }

There are two pairs of loops that are the same and can be combined like this:

int charCount = 0;
List<string> categories = new List<string>();
for (int j = 0; j < dgv.RowCount; j++)
{
    charCount += (int)dgv.Rows[j].Cells[4].Value;

    // look at every category in the dgv and see if we have a unique value in a container
    List<string> result = categories.FindAll(
        delegate(string b)
        {
            return b == dgv.Rows[j].Cells[1].Value.ToString();
        }
    );
    // if we don't have a unique ID then add it
    if (result.Count == 0)
    {
        categories.Add(dgv.Rows[j].Cells[1].Value.ToString());
    }
}

treeView1.Nodes.Add(languageTabs.TabPages[i].Text.ToUpper() + " Statistics");
treeView1.Nodes[i].Nodes.Add("Total Number Of Strings: " + dgv.RowCount); // might need to remove '--' from count
treeView1.Nodes[i].Nodes.Add("Total Number Of Chars: " + charCount);
treeView1.Nodes[i].Nodes.Add("Total Number Of Strings and Chars per Category: ");

int[] categoryCount = new int[categories.Count];
int count = 0;
foreach (string cat in categories)
{
    for (int j = 0; j < dgv.RowCount; j++)
    {
        if (dgv.Rows[j].Cells[1].Value.ToString() == cat)
        {
            categoryCount[count]++;
        }
    }

    treeView1.Nodes[i].Nodes[2].Nodes.Add(cat + ":   " + categoryCount[count].ToString());
    count++;
}

Note that the categoryCount array is no longer ncessary, and add Saeed's answer and you get:

int charCount = 0;
List<string> categories = new List<string>();
for (int j = 0; j < dgv.RowCount; j++)
{
    charCount += (int)dgv.Rows[j].Cells[4].Value;

    if (!categories.Any(x=>x == dgv.Rows[j].Cells[1].Value.ToString()))
        categories.Add(dgv.Rows[j].Cells[1].Value.ToString());
}

treeView1.Nodes.Add(languageTabs.TabPages[i].Text.ToUpper() + " Statistics");
treeView1.Nodes[i].Nodes.Add("Total Number Of Strings: " + dgv.RowCount); // might need to remove '--' from count
treeView1.Nodes[i].Nodes.Add("Total Number Of Chars: " + charCount);
treeView1.Nodes[i].Nodes.Add("Total Number Of Strings and Chars per Category: ");

foreach (string cat in categories)
{
    int count = 0;
    for (int j = 0; j < dgv.RowCount; j++)
    {
        if (dgv.Rows[j].Cells[1].Value.ToString() == cat)
        {
            count++;
        }
    }

    treeView1.Nodes[i].Nodes[2].Nodes.Add(cat + ":   " + count.ToString());
}

Then, assuming that dgv.Rows implements IEnumerable you can do this to the second loop:

foreach (string cat in categories)
{
    int count = dgv.Rows.Where(r => r.Cells[1].Value.ToString() == cat).Count();
    treeView1.Nodes[i].Nodes[2].Nodes.Add(cat + ":   " + count.ToString());
}


Algorithm to get an arbitrary perpendicular vector Is there a more efficient/direct way to find an arbitrary perpendicular vector of another vector?

def perpendicular_vector(v):
    r""" Finds an arbitrary perpendicular vector to *v*. """

    a, b = random.random(), random.random()
    if not iszero(v.z):
        x, y, z = v.x, v.y, v.z
    elif not iszero(v.y):
        x, y, z = v.x, v.z, v.y
    elif not iszero(v.x):
        x, y, z = v.y, v.z, v.x
    else:
        raise ValueError('zero-vector')

    c = (- x * a - y * b) / z

    if not iszero(v.z):
        return Vector(a, b, c)
    elif not iszero(v.y):
        return Vector(a, c, b)
    elif not iszero(v.x):
        return Vector(b, c, a)

The cross product of two vectors is perpendicular to both vectors, unless both vectors are parallel. So you could simply take the cross product of your first vector with (1, 0, 0), unless it is parallel to (1, 0, 0), in which case you could use (0, 1, 0). If you use lists rather than dedicated classes with attributes and are willing to use numpy, this gets ridiculously short:

import numpy as np

def perpendicular_vector(v):
    if v[1] == 0 and v[2] == 0:
        if v[0] == 0:
            raise ValueError('zero vector')
        else:
            return np.cross(v, [0, 1, 0]]
    return np.cross(v, [1, 0, 0])


Look for existing pivottables and change date/year colums based off the “Value” filter We have a macro that updates all dates/years in all our pivottables (around 100 of them pulling out data from our OLAP cube) based off of a "Master Date" at the top of our sheet (which is being updated monthly).
The problem is, that the way we're doing it, takes up a lot of space, and we've been forced to split the macro into multiple subs since it started exceeding the compile limit.

As you can see in the code example, its a very primitive way of updating the dates.

The next big problem is that the "YearMonth" filter that is being used to show the year and month is different, because some PT are looking at order quantity (QtyOrdered) and some are looking at sales quantity (QtyCustPackSlip), whihch are using a separate "YearMonth" filter.

Sub UpDateAllPivots()
' UpDateAllPivots Macro, nr.1

Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
'These ranges are the "Master Date
    D1 = Sheets("DATA").Range("B6").Value
    D2 = Sheets("DATA").Range("C6").Value
    D3 = Sheets("DATA").Range("D6").Value
    D4 = Sheets("DATA").Range("E6").Value
    D5 = Sheets("DATA").Range("F6").Value
    D6 = Sheets("DATA").Range("G6").Value
    D7 = Sheets("DATA").Range("H6").Value
    D8 = Sheets("DATA").Range("I6").Value
    D9 = Sheets("DATA").Range("J6").Value
    D10 = Sheets("DATA").Range("K6").Value
    D11 = Sheets("DATA").Range("L6").Value
    D12 = Sheets("DATA").Range("M6").Value

'This is a PT looking at QtyOrdered, using the Date.YearMonth
ActiveSheet.PivotTables("PivotTable1").PivotFields( _
        "[Date].[YearMonth].[YearMonth]").VisibleItemsList = Array("", _
        "[Date].[YearMonth].&[" & D1 & "]", "[Date].[YearMonth].&[" & D2 & "]", "[Date].[YearMonth].&[" & D3 & "]", _
        "[Date].[YearMonth].&[" & D4 & "]", "[Date].[YearMonth].&[" & D5 & "]", "[Date].[YearMonth].&[" & D6 & "]", _
        "[Date].[YearMonth].&[" & D7 & "]", "[Date].[YearMonth].&[" & D8 & "]", "[Date].[YearMonth].&[" & D9 & "]", _
        "[Date].[YearMonth].&[" & D10 & "]", "[Date].[YearMonth].&[" & D11 & "]", "[Date].[YearMonth].&[" & D12 & "]")

'This is a PT looking at QtyCustPackSlip, using the StatusPickedDatetimeKey
ActiveSheet.PivotTables("PivotTable3").PivotFields( _
        "[StatusPickedDatetimeKey].[YearMonth].[YearMonth]").VisibleItemsList = Array("", _
        "[StatusPickedDatetimeKey].[YearMonth].&[" & D1 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D2 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D3 & "]", _
        "[StatusPickedDatetimeKey].[YearMonth].&[" & D4 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D5 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D6 & "]", _
        "[StatusPickedDatetimeKey].[YearMonth].&[" & D7 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D8 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D9 & "]", _
        "[StatusPickedDatetimeKey].[YearMonth].&[" & D10 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D11 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D12 & "]")

Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True

End Sub

What we ultimately want is to compress it into something that isn't as big, since we're only getting more data. You can imagine how big this is with 100+ PT's and growing.

I was wondering if you could make a loop that looks through all used rows, looking for pivottable1-pivottable(n+1), seing if it exist. If it does exist, then it goes through a switch with two cases being that it either uses QtyOrdered ot QtyCustPackSlip, and updates the .YearMonth accordingly.

Comepletely forgot to post an answer to this, that i found a little after asking this question. Hopefully someone can use it

The code we used before was not optimal and messy, since you had to write those six lines of code for each PT, and every time you added a new PT.

I then realised you could just add some for each ... then loops that would go through not only every PT, but also each field in the PT, and then I could add the If / elsif that would check which field each PT is using, and have it correct the dates accordingly.

I had to set the pt anme as a string pts = pt for it to recognize it as a name (I don't know why exactly), but just having it as ActiveSheet.PivotTables(pt).PivotFields( _ .... Didn't work as it thought the name was "pt" and not the actualt name like "PivotTable1"..

It works as intended, the process takes about 1 min to complete. I suspect you can make it faster, but I havn't looked at it.

Sub UpdateAllPivots_new()
'
'Looks through all pivots in the sheet and updates the dates accordingly
'
'Update time is approx. 1 min (How can this be improved)
'

Dim ws As Worksheet
Dim pt As PivotTable
Dim pf As PivotField
Dim pi As PivotItem
Dim pts As String

    D1 = Sheets("DATA").Range("B6").Value
    D2 = Sheets("DATA").Range("C6").Value
    D3 = Sheets("DATA").Range("D6").Value
    D4 = Sheets("DATA").Range("E6").Value
    D5 = Sheets("DATA").Range("F6").Value
    D6 = Sheets("DATA").Range("G6").Value
    D7 = Sheets("DATA").Range("H6").Value
    D8 = Sheets("DATA").Range("I6").Value
    D9 = Sheets("DATA").Range("J6").Value
    D10 = Sheets("DATA").Range("K6").Value
    D11 = Sheets("DATA").Range("L6").Value
    D12 = Sheets("DATA").Range("M6").Value

Application.Calculation = xlCalculationManual
Application.ScreenUpdating = False
Application.DisplayStatusBar = False
Application.EnableEvents = False

Set ws = ActiveSheet

    For Each pt In ws.PivotTables
    pts = pt

        For Each pf In pt.PivotFields

                    If pf.Name = "[StatusPickedDatetimeKey].[YearMonth].[YearMonth]" Then
                         ActiveSheet.PivotTables(pts).PivotFields( _
                         "[StatusPickedDatetimeKey].[YearMonth].[YearMonth]").VisibleItemsList = Array("", _
                         "[StatusPickedDatetimeKey].[YearMonth].&[" & D1 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D2 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D3 & "]", _
                         "[StatusPickedDatetimeKey].[YearMonth].&[" & D4 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D5 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D6 & "]", _
                         "[StatusPickedDatetimeKey].[YearMonth].&[" & D7 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D8 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D9 & "]", _
                         "[StatusPickedDatetimeKey].[YearMonth].&[" & D10 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D11 & "]", "[StatusPickedDatetimeKey].[YearMonth].&[" & D12 & "]")

                    ElseIf pf.Name = "[Date].[YearMonth].[YearMonth]" Then
                        ActiveSheet.PivotTables(pts).PivotFields( _
                        "[Date].[YearMonth].[YearMonth]").VisibleItemsList = Array("", _
                        "[Date].[YearMonth].&[" & D1 & "]", "[Date].[YearMonth].&[" & D2 & "]", "[Date].[YearMonth].&[" & D3 & "]", _
                        "[Date].[YearMonth].&[" & D4 & "]", "[Date].[YearMonth].&[" & D5 & "]", "[Date].[YearMonth].&[" & D6 & "]", _
                        "[Date].[YearMonth].&[" & D7 & "]", "[Date].[YearMonth].&[" & D8 & "]", "[Date].[YearMonth].&[" & D9 & "]", _
                        "[Date].[YearMonth].&[" & D10 & "]", "[Date].[YearMonth].&[" & D11 & "]", "[Date].[YearMonth].&[" & D12 & "]")

                     End If

        Next pf

    Next pt

Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True
Application.DisplayStatusBar = True
Application.EnableEvents = True

End Sub
```


fftshift implementation for OpenCV I'm trying to implement fftshift from matlab for OpenCV. Can you please review the correctness of my algorithm? Have I missed something? Also, is there a better and faster way to do it?

cv::Mat ff;
cv::dft(distanceF, ff, cv::DFT_COMPLEX_OUTPUT);

//Make place for both the complex and the real values
cv::Mat planes[] = {cv::Mat::zeros(distanceF.size(),1, CV_32F), cv::Mat::zeros(distanceF.size(),1, CV_32F)};
cv::split(ff, planes);    // planes[0] = Re(DFT(I), planes[1] = Im(DFT(I))

cv::vector<float> im(planes[1].cols);
cv::vector<float> re(planes[0].cols);
int m = planes[0].cols;
int p = ceil(m/2);

for (int i = 0; i < p; i++)
{
    im[i] = planes[1].at<float>(p + i +1);
    im[p +i +1] = planes[1].at<float>(i +1);

    re[i] = planes[0].at<float>(p + i +1);
    re[p +i +1] = planes[0].at<float>(i +1);
}
im[p] = planes[1].at<float>(0);
re[p] = planes[0].at<float>(0);
cv::Mat input[] = {cv::Mat(im),cv::Mat(re)};
cv::merge(input,2,ff);

I think the best way to implement fftshift is like it is done in OpenCv itself:

//The body is inspired from OpenCV, the fct_fftshift is mine
void fct_fftshift(cv::Mat& src)
{
    int cx = src.cols/2;
    int cy = src.rows/2;

    cv::Mat q0(src, cv::Rect(0, 0, cx, cy));
    cv::Mat q1(src, cv::Rect(cx, 0, cx, cy));
    cv::Mat q2(src, cv::Rect(0, cy, cx, cy));
    cv::Mat q3(src, cv::Rect(cx, cy, cx, cy));

    cv::Mat tmp;
    q0.copyTo(tmp);
    q3.copyTo(q0);
    tmp.copyTo(q3);

    q1.copyTo(tmp);
    q2.copyTo(q1);
    tmp.copyTo(q2);
}

int main(int argc, char* argv[])
{
  cv::Mat src=cv::imread(argv[1], CV_8UC1);
  fct_fftshift(src); //it will change your original image
}


Set time zone from cookie Is the set_cookie variable the correct way to signal that the cookie should be set? It does not seems pythonic to me:

cookie = request.COOKIES.get(TIMEZONE_COOKIE_NAME)
set_cookie = False
if cookie and cookie in pytz.all_timezones_set:
    timezone.activate(pytz.timezone(cookie))
elif request.ip:
    geo = GeoIP2()
    try:
        time_zone = geo.city(request.ip).time_zone
        if time_zone:
            timezone.activate(pytz.timezone(time_zone))
            set_cookie = True
    except GeoIP2Error:
        self.logger.warning('Could not determine time zone of ip address: %s', request.ip)

response = self.get_response(request)
if set_cookie:
    response.set_cookie(TIMEZONE_COOKIE_NAME, time_zone)
return response

Edit: Well my previous suggestions don't work with the fact that
get_response has to be called with the correct timezone active.

Is the time_zone attribute from GeoIP2 always compatible with
pytz btw.?

Perhaps the following would work, verifying the cookie value only once
and discarding it if it doesn't fit:

cookie = request.COOKIES.get(TIMEZONE_COOKIE_NAME)
if cookie not in pytz.all_timezones_set:
    cookie = None

if not cookie and request.ip:
    try:
        cookie = GeoIP2().city(request.ip).time_zone
    except GeoIP2Error:
        self.logger.warning('Could not determine time zone of ip address: %s', request.ip)

if cookie:
    timezone.activate(pytz.timezone(cookie))

response = self.get_response(request)
if cookie:
    response.set_cookie(TIMEZONE_COOKIE_NAME, cookie)
return response

So, downside is a lot of mentions of cookie, upside is that it's not
more costly what you had before and the call to set the timezone is only
there once.

Last point, if anything like this happens to be needed more than once,
put it into separate functions.  This snippet is a bit short to make
that call.


UnitOfWork without repository and transaction I've created the unitofwork class
below:

 public class UnitOfWork : IUnitOfWork
  {
    private AppDbContext _context { get; set; }
    private IDbTransaction _transaction { get; set; }
    private ObjectContext _objectContext { get; set; }
    private bool _isDisposed;
    public UnitOfWork(AppDbContext context)
    {
      _context = context;
    }
    public DbSet<T> Set<T>() where T : class
    {
      return _context.Set<T>();
    }
    public DbEntityEntry<T> Entry<T>(T entity) where T : class
    {
      return _context.Entry(entity);
    }
    public void Begin()
    {
      _objectContext = ((IObjectContextAdapter)_context).ObjectContext;

      if (_objectContext.Connection.State != ConnectionState.Open)
      {
        _objectContext.Connection.Open();
        if(_transaction != null)
        {
          _transaction.Dispose();
        }
        _transaction = _objectContext.Connection.BeginTransaction();
      }
    }
    public void AutoDetectChangesEnabled(bool option)
    {
      _context.Configuration.AutoDetectChangesEnabled = option;
    }
    public void LazyLoadingEnabled(bool option)
    {
      _context.Configuration.LazyLoadingEnabled = option;
    }

    public void SaveChanges()
    {
      _context.SaveChanges();
    }
    public Task SaveChangesAsync()
    {
      return _context.SaveChangesAsync();
    }
    public void Commit()
    {
      _transaction.Commit();
    }
    public void Rollback()
    {
      _transaction.Rollback();
      foreach (var entry in _context.ChangeTracker.Entries())
      {
        switch (entry.State)
        {
          case System.Data.Entity.EntityState.Modified:
            entry.State = System.Data.Entity.EntityState.Unchanged;
            break;
          case System.Data.Entity.EntityState.Added:
            entry.State = System.Data.Entity.EntityState.Detached;
            break;
          case System.Data.Entity.EntityState.Deleted:
            entry.State = System.Data.Entity.EntityState.Unchanged;
            break;
        }
      }
    }
    public void Dispose()
    {
      if (!_isDisposed)
      {
        if (_context != null)
        {
          _context.Database.Connection.Close();
          _context.Dispose();
          _context = null;
        }
        if(_transaction != null)
        {
          _transaction.Dispose();
          _transaction = null;
        }
        _isDisposed = true;
      }
    }
  }
}

usage

without transaction

_unitOfWork.Set<User>().Add(MyModel);

with transaction

_unitOfWork.Begin();
try {
   _unitOfWork.Set<User>().Add(MyModel);
   _unitOfWork.SaveChanges();
   _unitOfWork.Commit();
}
catch {
   _unitOfWork.Rollback();
}

I must close the transaction in dispose?
How can I improve this code?

Observation: I'm use DI with IUnitOfWork, use AutoFac

Ef already implements unit of work in itself, as does it the repository pattern.

Since you open the connection yourself I take it you use EF6, which also automatically wraps SaveChanges in a transaction if there isn't one already, so you have little use of your UnitOfWork.

Read more at Microsoft.

If you DO need to open a transaction manually, and you don't care to use the TransactionScope class, the transaction must be opened before the connection opens and should be disposed before you close the connection again.


Two approaches to print all permutations - returning versus passing through the “result” list I have noticed many of the backtracking problems have two ways of solving.

One is to return "whatever's the required list", vs passing-through the "result" to every call and appending to it. What is the downside of returning (is it less memory/time efficient)? Example - To print all possible permutations, what makes this solution inefficient vs the second one.

public List<List<Integer>> perm(int[] nums){
    List<List<Integer>> result = new ArrayList<List<Integer>>();
    if(nums.length == 0){
        result.add(new ArrayList<Integer>());
        return result;
    }
    for(int i= 0;i<nums.length;i++){
        int first = nums[i];
        int[] remnums = new int[nums.length-1];
        int j = 0;
        for(int cur : nums){
            if(cur != first){
                remnums[j] = cur;j++;
            }
        }
        List<List<Integer>> tmp = perm(remnums);

        for(List<Integer> t : tmp){
            t.add(0,first);

        }
        result.addAll(tmp);
    }
    return result;
}

2nd approach ---

public List<List<Integer>> permute(int[] nums) {
   List<List<Integer>> list = new ArrayList<>();
   // Arrays.sort(nums); // not necessary
   backtrack(list, new ArrayList<>(), nums);
   return list;
}

private void backtrack(List<List<Integer>> list, List<Integer> tempList, int [] nums){
   if(tempList.size() == nums.length){
      list.add(new ArrayList<>(tempList));
   } else{
      for(int i = 0; i < nums.length; i++){
         if(tempList.contains(nums[i])) continue; // element already exists, skip
         tempList.add(nums[i]);
         backtrack(list, tempList, nums);
         tempList.remove(tempList.size() - 1);
      }
   }
}

I think the question you are asking is:  Given a recursive algorithm, is there an inherent difference between diving deep and capturing a result at the deepest level of the recursion, vs. passing that result back up the chain to be captured at the shallowest level of recursion.

Inherently, there should not be much difference.

If we look specifically at your implementation though, there are big performance issues that far outweigh any difference in the method of recursion.

In your first code example, this operation is essentially an O(N^2) operation.

    int[] remnums = new int[nums.length-1];
    int j = 0;
    for(int cur : nums){
        if(cur != first){
            remnums[j] = cur;j++;
        }
    }

In each recursive call, you are copying the N items (and removing one), resulting in a total of N * N/2 copies.  (N/2) is the average number of items copied, as we copy N-1 items in the first level, and 1 item in the last level.

Then notice:

    for(List<Integer> t : tmp){
        t.add(0,first);
    }

Adding to the front of an ArrayList must shift everything else over.  So this is another O(n^2) operation.  (or is this actually n^3, given all of the lists?)

In the second code example, this is an O(n^2) operation:

   if(tempList.contains(nums[i])) continue; // element already exists, skip

This is an O(n) linear search, for each N.

So, (at least in this example) it's not the recursion that's a problem.  It's the choice of algorithm and data structures.