Find LCM of two numbers The problem is to find LCM of two numbers. I have tried to solve the problem in two ways. First by using the LCM formula :

LCM (a,b)= a*b/GCD(a,b).

Second, by finding out the multiples of each number and then finding the first common multiple.
 Below are the codes that I have written:

Code 1:

#Using the LCM formula LCM = a*b / gcd(a,b)
def LCM(x , y):
    """ define a function LCM which takes two integer inputs and return their LCM using the formula LCM(a,b) = a*b / gcd(a,b) """
    if x==0 or y == 0:
        return "0"

    return (x * y)/GCD(x,y)

def GCD(a , b):
    """ define a function GCD which takes two integer inputs and return their common divisor"""
    com_div =[1]
    i =2
    while i<= min(a,b):
        if a % i == 0 and  b % i ==0:
            com_div.append(i)
        i = i+1
    return com_div[-1]

print LCM(350,1)
print LCM(920,350)

Code 2:

#Finding the multiples of each number and then finding out the least common multiple
def LCM(x , y):
    """ define a function LCM which take two  integerinputs and return their LCM"""
    if x==0 or y == 0:
        return "0"
    multiple_set_1  = []
    multiple_set_2  = []
    for i in range(1,y+1):
        multiple_set_1.append(x*i)
    for j in range(1,x+1):
        multiple_set_2.append(y*j)
    for z in range (1,x*y+1):
        if z in multiple_set_1:
            if z in multiple_set_2:
                return z
                break

print LCM(350,450)

I want to know which one of them is a better way of solving the problem and why that is the case. Also suggest what other border cases should be covered.

About Version 1:

GCD(a, b) has a time complexity of \$ O(min(a, b))\$ and requires an
array for intermediate storage. You could get rid of the array by
iterating over the possible divisors in reverse order, so that you
can early return if a common divisor is found.

About Version 2:

LCM(x , y) has a time complexity of \$ O(xy)\$ and requires two
arrays for intermediate storage, so this worse than version 1.
You could improve this by pre-computing only the multiples of one number,
and then test the multiples of the other number (in reverse order)
until you find a common multiple, and then early return.

Common issues:

LCM should always return a number, your code returns the string
"0" in some cases.
Both functions take integer arguments (according to the docstring
) but do not produce sensible results for negative input.
The usage of whitespace in your code is inconsistent. Examples:

if x==0 or y == 0:
i =2

More PEP8 coding style
violations (most of them related to spacing) can be detected by checking
your code at PEP8 online.

A better algorithm

The "Euclidean Algorithm" is a well-known method for computing the greatest common
divisor, and superior to both of your approaches.

It is already available in the Python standard library:

>>> from fractions import gcd   # Python 2
>>> from math import gcd        # Python 3
>>> gcd(123, 234)
3

This should be used as the basis for implementing an LCM
function.

Have a look at https://rosettacode.org/wiki/Greatest_common_divisor#Python,
if you want to implement the GCD yourself (for educational purposes),
for example

def gcd_iter(u, v):
    while v:
        u, v = v, u % v
    return abs(u)

This is short, simple, needs no additional space, and fast:
the time complexity is (roughly) \$ =O(\log(max(a, b))\$
(see for example What is the time complexity of Euclid's Algorithm).


Simple authorization module with Rails I created a simple Authorization module with Rails. I found that there are other authorization systems, such as CanCanCan, but they grant permissions at Model level and, for this particular website I am developing, it is more convenient to authorize at Controller level.

The idea is simple: the permissions are stored in a YAML file in the config directory and a function checks if the combination of admin_role, controller and action exist in the config file. There is also a wildcard :all.

# app/controllers/admins/base_controller.rb
class Admins::BaseController < ApplicationController

  include Admins::AuthorizationHelper

  before_action :authorize_admin

  ...

end

# app/helpers/admin/authorization_helper.rb
module Admins::AuthorizationHelper

  private

  def authorize_admin
    unless is_authorized? params[:controller], params[:action]
      refuse_access_to_admin_site
    end
  end

  def is_authorized?(controller, action)
    permissions = Rails.application.config_for :admin_auth
    authority = current_admin.authority

    if permissions[authority].nil?
      false
    elsif permissions[authority] == ['all']
      true
    elsif permissions[authority][controller].nil?
      false
    elsif permissions[authority][controller] == ['all']
      true
    elsif permissions[authority][controller].include? action
      true
    else
      false
    end
  end

  def refuse_access_to_admin_site
    flash[:error] = 'Permission denied'

    if request.referer.present?
      redirect_to :back
    else
      redirect_to admins_products_path
    end
  end

end

# config/admin_auth.yml
defaults: &defaults
    super_admins:
        - all
    admins:
        admins/shops:
            - index
            - show_used_history
        admins/products:
            - index
        admins/prescriptions:
            - all
        admins/products:
            - index
    operators:
        admins/products:
            - index
        admins/prescriptions:
            - all
        admins/shipments:
            - index
            - show
test:
    <<: *defaults
development:
    <<: *defaults
staging:
    <<: *defaults
production:
    <<: *defaults

There's a lot of repeated code in that long if..else.  Here's an idea (code not tested) for shortening it up:

def is_authorized?(controller, action)
  permissions = Rails.application.config_for :admin_auth
  authority = current_admin.authority

  global_auth = permissions.fetch(authority, [])
  return true if global_auth.include? 'all'

  # Note the implicit assumption in this code, and the original code,
  # that global_auth is a Hash if we have not already returned

  ctrl_auth = global_auth.fetch(controller, [])
  ctrl_auth.include? 'all' || ctrl_auth.include? action
end

Please see the inline comment for an assumption your making about the config data.

At a higher level, the deeper reason this code has to be overly complicated and do nil checks and checks for different kinds of data (array or hash) is because the config data structure has no regularity.

The real fix here is probably to wrap that returned data structure in an object that regularizes it, essentially using something similar to the NullObject pattern.  So that your code could then read like this:

permissions = Rails.application.config_for :admin_auth
permissions = ConfiguredPermissions.new permissions

permissions.global_access? || permissions.for?(controller, action)

You can also avoid the if..else in your redirect method:

  def refuse_access_to_admin_site
    flash[:error] = 'Permission denied'

    location = request.referer.present? ? :back : admins_products_path
    redirect_to location
  end


My take on the algorithmic crush problem from HackerRank This is my solution to the Algorithmic Crush problem from HackerRank. Can I get any comments regarding where I can improve the code to be more efficient?

  You are given a list of size \$N\$, initialized with zeroes. You have
  to perform \$M\$ operations on the list and output the maximum of
  final values of all the \$N\$ elements in the list. For every
  operation, you are given three integers \$a, b\$ and \$k\$ and you
  have to add value to all the elements ranging from index \$a\$ to
  \$b\$ (both inclusive).

  Input Format

  First line will contain two integers \$N\$ and \$M\$ separated by a
  single space. Next \$M\$ lines will contain three integers \$a, b\$
  and \$k\$ separated by a single space. Numbers in list are numbered
  from \$1\$ to \$N\$.

  Constraints

  \$3 \leq N \leq 10^7\$

  \$1\leq M \leq 2*10^5\$

  \$1 \leq a \leq b \leq N\$

  \$ 0 \leq k \leq 10^9\$

  Output Format

  A single line containing maximum value in the updated list.

  Sample Input

5 3
1 2 100
2 5 100
3 4 100

  Sample Output

200

def updateList(seq, listA):
    st, end, value = map(int, seq.split())
    for i in range(st-1, end):
        listA[i] = listA[i]+value
    return listA

n, m = map(int, raw_input().split())

lis = [0 for i in range(n)]

for ins in range(m):
    cmds = raw_input()
    resultSet = updateList(cmds, lis)
    lis = resultSet
print sorted(resultSet)[-1]

The code is generally clean, however, maintaining the same algorithm will probably result in a timeout for this problem. Your current complexity is \$ O(N\cdot M) \$ which could be up to \$ 10^7 \times 10^5 \$ according to the problem statement.

A similar problem to this is the maximum interval overlap problem, discussed here. The problem is given the guest arrival and departure times in a party, find the interval with the maximum guests. It should be a good exercise to apply the same idea to this problem.


Reading binary data from a file I have a binary file with alternating uint8 and uint64 data stamps.
I read those in by using the following line:

clicks = np.fromfile(filename, dtype=[('time','u8'),('channel','u2')])

This works well and fast enough. Now i want to go though the array and set the time values to the time difference with respect to the last 'click' seen on channel 7 (the so called gate clicks). The array is sorted by time. In C I would do this with a simple for loop going over the array (and this works extremly fast). When I implement this in python i get a data rate of only 2 mb/s. The best solution i came up with looks like this:

    ''' create an array with the indices of the channel-7 clicks '''
    gate_clicks = clicks['channel']==7
    gate_ind = np.array(range(len(gate_clicks)))
    gate_ind = gate_ind[gate_clicks]
    gate_ind_shift = np.delete(gate_ind,0,0)

    ''' slice out the clicks between to gate clicks and set the time stamps '''
    for start,end in zip(gate_ind,gate_ind_shift):
        start_time = data[start]['time']
        slice = data[start:end]
        slice['time'] = slice['time']-start_time
        data[start:end] = slice

This gives a data rate of about 4.


Alternative class to imitate AsyncTask class (deprecated) with Thread and Handler classes As the title says, I have created an alternative class to imitate AsyncTask class (deprecated) with Thread and Handler classes but i feel uncomfortable to use it.(the app works fine with my class) Can you guys tell me is it ok or not? I feel like there is nothing changed. The reason i want to stop using AsyncTask is memoryleak issue.
import android.os.Handler;

public class BackgroundTask
{
    protected Handler localHandler;
    protected Thread localThread;

    public void execute()
    {
        this.localHandler = new Handler();
        this.onPreExecute(); //onPreExecute Method

        this.localThread = new Thread(new Runnable()
        {
            @Override
            public void run()
            {
                BackgroundTask.this.doInBackground(); //doInBackground Method
                BackgroundTask.this.localHandler.post(new Runnable()
                {
                    @Override
                    public void run()
                    {
                        BackgroundTask.this.onPostExecute(); //onPostExecute Method
                    }
                });
            }
        });
        this.localThread.start();
    }

    public void cancel()
    {
        if(this.localThread.isAlive())
            this.localThread.interrupt();
    }

    protected void onPreExecute()
    {

    }

    protected void doInBackground()
    {

    }

    protected void onPostExecute()
    {

    }
}

Example for use (I can interrupt the Thread when i want so this must be solving the leak issue but i want this code to be seen by some other programmers too.)
public class HttpPostUserScore extends BackgroundTask
{
    private final Context context;
    private Exception taskException;
    private final String point;

    public HttpPostUserScore(Context context, String point)
    {
        this.context = context;
        this.point = point;
    }

    @Override
    protected void doInBackground()
    {
        try
        {
            OkHttpClient client = new OkHttpClient();

            FormBody.Builder formBuilder = new FormBody.Builder();
            formBuilder.add("token", User.loggedUser.getToken());
            formBuilder.add("score", this.point);
            RequestBody requestBody = formBuilder.build();

            Request request = new Request.Builder().url(this.context.getString(R.string.user_score_api_url_body)).post(requestBody).build();
            Response response = client.newCall(request).execute();

            Log.println(Log.WARN, "ScorePost Response", response.body().string());
        }
        catch (Exception e) { this.taskException = e; }
    }

    @Override
    protected void onPostExecute()
    {
        if(this.taskException != null)
            Log.println(Log.ERROR, "ScorePost Error", this.taskException.getMessage());
        else
            Toast.makeText(this.context, this.point + " Points Added!", Toast.LENGTH_LONG).show();
    }
}

I have examined some codes from Facebook's android login api. They are still using AsyncTask for asynchronous programming. Is it a big deal or not?

public void execute()
    {
        this.localHandler = new Handler();
        this.onPreExecute(); //onPreExecute Method

        this.localThread = new Thread(new Runnable()
        {
            @Override
            public void run()
            {
                BackgroundTask.this.doInBackground(); //doInBackground Method
                BackgroundTask.this.localHandler.post(new Runnable()
                {
                    @Override
                    public void run()
                    {
                        BackgroundTask.this.onPostExecute(); //onPostExecute Method
                    }
                });
            }
        });
        this.localThread.start();
    }

This part would be much easier to read using method references:
public void execute()
{
    localHandler = new Handler();
    onPreExecute();

    localThread = new Thread(this::backgroundExecutor)
    localThread.start();
}

protected void backgroundExecutor() {
    doInBackground();

    localHandler.post(this::onPostExecute)
}

        this.localHandler = new Handler();
        this.onPreExecute(); //onPreExecute Method

One should only use this if it is necessary to differentiate between a local and a member variable. And that should only be necessary in setters and the constructor. You can remove this and remove a lot of noise with it.

        this.localHandler = new Handler();
        this.onPreExecute(); //onPreExecute Method

Comment on why you're doing something, not what you're doing. This comment tells me nothing the code on the left doesn't tell me.

        if(this.localThread.isAlive())
            this.localThread.interrupt();

One should only use curly-braces to make the scopes easily visible and minimize possible scope errors.

Your code has a few problems when it comes to threading. Invoking execute multiple times will reset the local state and will yield inconsistent and potentially unknown results, as localHandler and localThread are getting overwritten with every call (imagine execute being called from multiple threads). That will also impact cancel, as it will only be called on the last run thread. Additionally, calling interrupt might or might not actually end the thread. There is no failsafe way to end a thread except killing the JVM.
Your implementation has similar issues, as it shares state using class members, which means that multiple invocations will lead to lost state and possible confusions (three invocations, the last two fail, but the first one is the last to finish).
What you're trying to do is not easy. I'd recommend looking at the implementation of the AsyncHandler class and getting ideas what they were doing. Depending in your use-case, for example if you have a lot of background actions, you might want instead to utilize a thread-pool, to minimize constantly spinning up threads.


Basic Caesar Cipher in Swift 3.1 I have been learning Swift for the past day and thought I'd try a basic problem where I can do String manipulation. I am only on pg.100 of the Swift Programming Language on iBooks. Compared to python it seems to be quite difficult to actually modify individual characters as Swift is strongly typed/type safe.

  A Caesar cipher is used to encrypt a String by shifting each letter by x, where x is an integer, and replaces each letter withe the shifted letter. So encrypt("AB", 2) becomes "BC"

If anyone could point where I can reduce the number of lines or an easier way of doing/or idioms I have missed that'd be just what I am looking for.

import Cocoa

func encrypt(message: String, shift: Int) -> String {

    let scalars = Array(message.unicodeScalars)
    let unicodePoints = scalars.map({x in Character(UnicodeScalar(Int(x.value) + shift)!)})

    return String(unicodePoints)
}

let message = "Attack at dawn"
print(encrypt(message: message, shift: 2))

Using the unicodeScalars view of the Swift String is a good start,
but the code can be simplified:

There is no need to create an array first, you can call map
directly on the unicodeScalars collection.
The conversion to Character is not needed, the array of shifted
unicode scalars can be converted directly back to a string
(attribution to this answer on Stack Overflow).

Also the variable name unicodePoints is misleading, because that
is an array of Character in your code.

Your method then becomes

func encrypt(message: String, shift: Int) -> String {
    let unicodeScalars = message.unicodeScalars.map { UnicodeScalar(Int($0.value) + shift)! }
    return String(String.UnicodeScalarView(unicodeScalars))
}

However, this is probably not yet what you want. Example:

let message = "Attack at dawn"
print(encrypt(message: message, shift: 2))    // Cvvcem"cv"fcyp
print(encrypt(message: message, shift: 10))   // K~~kmu*k~*nkx

All characters are shifted by the given amount so that a space
becomes a quotation mark, and some letters are transformed to non-letters. Also

print(encrypt(message: message, shift: -100))

crashes at runtime, because the unicode scalar value is shifted to
an invalid value.

The traditional Caesar Cipher transforms only letters in the range
A...Z, and rotates them, so that shifting "Z" by 2 becomes "B".
Lowercase letters can be transformed to uppercase, but other characters
are usually ignored (either removed or left unchanged).

Here is a possible implementation with an inner function
shiftLetter() which transforms a single letter.
This inner function is used as argument in

msg.unicodeScalars.map(shiftLetter)

to transform the entire String:

func encrypt(message: String, shift: Int) -> String {

    func shiftLetter(ucs: UnicodeScalar) -> UnicodeScalar {
        let firstLetter = Int(UnicodeScalar("A").value)
        let lastLetter = Int(UnicodeScalar("Z").value)
        let letterCount = lastLetter - firstLetter + 1

        let value = Int(ucs.value)
        switch value {
        case firstLetter...lastLetter:
            // Offset relative to first letter:
            var offset = value - firstLetter
            // Apply shift amount (can be positive or negative):
            offset += shift
            // Transform back to the range firstLetter...lastLetter:
            offset = (offset % letterCount + letterCount) % letterCount
            // Return corresponding character:
            return UnicodeScalar(firstLetter + offset)!
        default:
            // Not in the range A...Z, leave unchanged:
            return ucs
        }
    }

    let msg = message.uppercased()
    return String(String.UnicodeScalarView(msg.unicodeScalars.map(shiftLetter)))
}

Examples:

let message = "Attack at dawn"
print(encrypt(message: message, shift: 2))    // CVVCEM CV FCYP
print(encrypt(message: message, shift: 10))   // KDDKMU KD NKGX
print(encrypt(message: message, shift: -100)) // EXXEGO EX HEAR


Building a number pyramid I have been working a number pyramid program. This is a question from Y. Daniel Laing's Introduction to programming using Python (which is based on Python 3.2). The question is from chapter 5, and it is number 5.19 in programming exercises portion of the chapter.  The question is stated as follows:

  Write a program that prompts the user to enter an integer from 1 to 15
  and displays it as the following sample run:

I don't have the image, but it is something like this:

  Enter the number of lines: 3

                          1
                       2  1  2
                    3  2  1  2  3

The solution I have come up with is this:

p = eval(input("Enter the number of lines: "))

num = 0
y = 1

while num < p:
    y = num

    line = str(y+1) + "  "

while y >= 1:

    y -= 1
    line += str(y+1) + "  "

while  y < num:

    line += str(y + 2) + "  "

    y +=1

print(format(line, "^80s"))

num +=1

If anyone has a better solution that would be simpler, please let me know.

Like 200_success said, the key is in thinking at the higher level. But I think his approach can be simplified.

Lets first define a row function, which simply calculates a list of numbers in an n'th row of our output:

# When n=3, returns [3, 2, 1, 2, 3]
def row(n):
    return list(reversed(range(2, n+1))) + list(range(1, n+1))

And now we can use this to print out the lines:

for n in range(1, 5):
    print(row(n))

Giving us the following output:

[1]
[2, 1, 2]
[3, 2, 1, 2, 3]
[4, 3, 2, 1, 2, 3, 4]

The core of the problem is now solved. The only thing left is formatting. For this we can write another function, which formats each number in list into a three-character box (with "%3s"), and also appends a certain amount of padding to make sure lines are aligned like a pyramid:

def formatted_row(n, num_rows):
    padding = '   ' * (num_rows-n)
    return padding + ''.join(["%3s" % x for x in row(n)])

Now we're almost there. Just need to tie it all together:

# When n=3, returns [3, 2, 1, 2, 3]
def row(n):
    return list(reversed(range(2, n+1))) + list(range(1, n+1))

def formatted_row(n, num_rows):
    padding = '   ' * (num_rows-n)
    return padding + ''.join(["%3s" % x for x in row(n)])

num_lines = eval(input("Enter the number of lines: "))
for n in range(1, num_lines+1):
    print(formatted_row(n, num_lines))


Check if an inputted string is in the right format for a valid rgb color It is a practice from the site edabit.com and you have to check if the input is valid. That is the original task:

Given an RGB(A) CSS color, determine whether its format is valid or not. Create a function that takes a string (e.g. "rgb(0, 0, 0)") and return True if it's format is correct, otherwise return False.

The rules are shown in the testing code:
# True Tests
Test.assert_equals(valid_color('rgb(0,0,0)'), True, 'rgb lowest valid numbers')
Test.assert_equals(valid_color('rgb(255,255,255)'), True, 'rgb highest valid numbers')
Test.assert_equals(valid_color('rgba(0,0,0,0)'), True, 'rgba lowest valid numbers')
Test.assert_equals(valid_color('rgba(255,255,255,1)'), True, 'rgba highest valid numbers')
Test.assert_equals(valid_color('rgba(0,0,0,0.123456789)'), True, 'alpha can have many decimals')
Test.assert_equals(valid_color('rgba(0,0,0,.8)'), True, 'in alpha the number before the dot is optional')
Test.assert_equals(valid_color('rgba(   0 , 127 , 255 , 0.1 )'), True, 'whitespace is allowed around numbers (even tabs)')
Test.assert_equals(valid_color('rgb(0%,50%,100%)'), True, 'numbers can be percentages')

# False Tests
Test.assert_equals(valid_color('rgb(0,,0)'), False, 'INVALID: missing number')
Test.assert_equals(valid_color('rgb (0,0,0)'), False, 'INVALID: whitespace before parenthesis')
Test.assert_equals(valid_color('rgb(0,0,0,0)'), False, 'INVALID: rgb with 4 numbers')
Test.assert_equals(valid_color('rgba(0,0,0)'), False, 'INVALID: rgba with 3 numbers')
Test.assert_equals(valid_color('rgb(-1,0,0)'), False, 'INVALID: numbers below 0')
Test.assert_equals(valid_color('rgb(255,256,255)'), False, 'INVALID: numbers above 255')
Test.assert_equals(valid_color('rgb(100%,100%,101%)'), False, 'INVALID: numbers above 100%')
Test.assert_equals(valid_color('rgba(0,0,0,-1)'), False, 'INVALID: alpha below 0')
Test.assert_equals(valid_color('rgba(0,0,0,1.1)'), False, 'INVALID: alpha above 1')

I fear that my solution is not very elegant, efficent or short:
def create_rgbA_tuple(color):
    indexBracket1 = color.rfind("(")
    save = color[0] + color[1] + color[2] + color[3]
    if save != 'rgb(' and save != 'rgba':
        return False
    indexBracket2 = color.rfind(")")
    bracketArea = ""
    for i in range(indexBracket1 + 1, indexBracket2):
        bracketArea = bracketArea + color[i]

    bracketArea = bracketArea.split(',')  # bracketArea is now an Array (r,g,b/A)
    if len(bracketArea) == 4 and save != 'rgba' or len(bracketArea) == 3 and save != 'rgb(':
        return False
    try:
        for i in range(len(bracketArea)):
            bracketArea[i] = float(bracketArea[i])
    except:
        try:
            for i in range(len(bracketArea)):
                if bracketArea[i][-1] == "%":
                    bracketArea[i] = int(bracketArea[i].strip('%')) / 100 * 255
                else:
                    return False
        except:
            return False
    return bracketArea

def valid_color(color):
    rgb = create_rgbA_tuple(color)
    if not rgb:
        return False
    for i in range(3):
        if int(rgb[i]) < 0 or int(rgb[i]) > 255:
            return False

    if color[3] == "a":  # if rgbA
        if rgb[3] < 0 or rgb[3] > 1:
            return False
    return True
```

One thing the tests don't indicate is whether space is allowed before/after the color string. I'm inclined to be generous, so I'd suggest just discarding any space before you start:
color = color.strip()

Next, you are doing a lot of work to replace some python built-in functions. Before you do anything else, you should probably read the String Methods documentation to see what tools are available to you.
Try something like:
is_rgb = color.startswith('rgb(')
is_rgba = color.startswith('rgba(')

if not is_rgb and not is_rgba:
    return False

if not color.endswith(')'):
    return False

# strip off prefix "...(" and suffix ")"
color = color[color.index('(') + 1:-1]

You can use .endswith to check for percent signs, also.


Pivot some rows to new columns in DataFrame I'm after a pythonic and pandemic (from pandas, pun not intended =) way to pivot some rows in a dataframe into new columns.

My data has this format:

           dof  foo  bar  qux
idxA idxB
100  101     1   10   30   50
     101     2   11   31   51
     101     3   12   32   52
     102     1   13   33   53
     102     2   14   34   54
     102     3   15   35   55
200  101     1   16   36   56
     101     2   17   37   57
     101     3   18   38   58
     102     1   19   39   59
     102     2   20   40   60
     102     3   21   41   61

The variables foo, bar and qux actually have 3 dimensional coordinates, which I would like to call foo1, foo2, foo3, bar1, ..., qux3. These are identified by the column dof. Each row represents one axis in 3D, dof == 1 is the x axis, dof == 2 the y axis and dof == 3 is the z axis.

So, here is the final dataframe I want:

           foo1  bar1  qux1  foo2  bar2  qux2  foo3  bar3  qux3
idxA idxB
100  101     10    30    50    11    31    51    12    32    52
     102     13    33    53    14    34    54    15    35    55
200  101     16    36    56    17    37    57    18    38    58
     102     19    39    59    20    40    60    21    41    61

Here is what I have done.

import pandas as pd

data = [[100, 101, 1, 10, 30, 50],
        [100, 101, 2, 11, 31, 51],
        [100, 101, 3, 12, 32, 52],
        [100, 102, 1, 13, 33, 53],
        [100, 102, 2, 14, 34, 54],
        [100, 102, 3, 15, 35, 55],
        [200, 101, 1, 16, 36, 56],
        [200, 101, 2, 17, 37, 57],
        [200, 101, 3, 18, 38, 58],
        [200, 102, 1, 19, 39, 59],
        [200, 102, 2, 20, 40, 60],
        [200, 102, 3, 21, 41, 61],
        ]

df = pd.DataFrame(data=data, columns=['idxA', 'idxB', 'dof', 'foo', 'bar', 'qux'])
df.set_index(['idxA', 'idxB'], inplace=True)

#
# Here is where the magic happens - and I'm not too happy about this implementation
#

# Create an ampty dataframe with the same indexes
df2 = df[df.dof == 1].reset_index()[['idxA', 'idxB']]
df2.set_index(['idxA', 'idxB'], inplace=True)

# Loop through each DOF and add columns for `bar`, `foo` and `qux` manually.
for pivot in [1, 2, 3]:
    df2.loc[:, 'foo%d' % pivot] = df[df.dof == pivot]['foo']
    df2.loc[:, 'bar%d' % pivot] = df[df.dof == pivot]['bar']
    df2.loc[:, 'qux%d' % pivot] = df[df.dof == pivot]['qux']

However I'm not too happy with these .loc calls and incremental column additions inside a loop. I thought that pandas being awesome as it is would have a neater way of doing that.

groupby

When iterating over the values in a column, it is bad practice to hardcode the values (for pivot in [1, 2, 3]). A better way would have been for pivot in df["dof"].unique(), but the best way is with DataFrame.groupby

To see what happens in the groupby, I try it first with an iteration, and printing the groups

for pivot, data in df.groupby("dof"):
    print(pivot)
    print(data)

Then I get to work with one of the data to mold it the way I want. In this case, we don't need the column dof any more, since we have it in the pivot variable, and we rename the columns using rename

for pivot, data in df.groupby("dof"):
    print(pivot)
    print(
        data.drop(columns="dof").rename(
            mapper={
                column_name: f"{column_name}{pivot}"
                for column_name in data.columns
            },
            axis=1,
        )
    )

Then we can use pd.concat to stitch it together

pd.concat(
    [
        data.drop(columns="dof").rename(
            mapper={
                column_name: f"{column_name}{pivot}"
                for column_name in data.columns
            },
            axis=1,
        )
        for pivot, data in df.groupby("dof")
    ],
    axis=1,
)

unstack

An alternative is with unstack:

From you description, dof is part of the index, so add it there. Then you can use DataFrame.unstack to bring it to the columns.

df2 = df.set_index("dof", append=True).unstack("dof")

        foo foo foo bar bar bar qux qux qux
dof     1   2   3   1   2   3   1   2   3
idxA idxB
100 101 10  11  12  30  31  32  50  51  52
100 102 13  14  15  33  34  35  53  54  55
200 101 16  17  18  36  37  38  56  57  58
200 102 19  20  21  39  40  41  59  60  61

If you are okay with a MultiIndex, which will be handier then the concatenated strings in most cases, you can leave it at that. If you want it in the form you have it, you can do df2.columns = df2.columns.map(lambda x: f"{x[0]}{x[1]}").


PHP Pagination for 1,2,3... vs 1,2,3,4,5,etc I am having an issue where I am doing a query and it is coming back with about 100 pages, I was hoping to have it come back with 1,2,3...30,31...99,100 instead of 1,2,3,4,5,6,7,8,etc. please see below for working code that returns back results as unwanted format, I am new here so if this question is written wrong please let me know and I will make necessary changes , thanks in advance:

function getPage($stmt, $pageNum, $rowsPerPage)
{
$offset = ($pageNum - 1) * $rowsPerPage;
$rows = array();
$i = 0;
while(($row = sqlsrv_fetch_array($stmt, SQLSRV_FETCH_NUMERIC,
 SQLSRV_SCROLL_ABSOLUTE,         $offset + $i)) && $i < $rowsPerPage)
{
array_push($rows, $row);
$i++;
}
return $rows;
}

// Set the number of rows to be returned on a page.
$rowsPerPage = 30;

// Connect to the server.
$serverName = 'test';
$connOptions = array("Database"=>"test");
$conn = sqlsrv_connect($serverName, $connOptions);
if (!$conn)
die( print_r( sqlsrv_errors(), true));

//  Define and execute the query.
// Note that the query is executed with a "scrollable" cursor.
$sql = "select * from info";

$stmt = sqlsrv_query($conn, $sql, array(), array( "Scrollable" => 'static' ));
if ( !$stmt )
die( print_r( sqlsrv_errors(), true));

// Display the selected page of data.
echo "
";
$pageNum = isset($_GET['pageNum']) ? $_GET['pageNum'] : 1;
$page = getPage($stmt, $pageNum, $rowsPerPage);

foreach($page as $row)
echo "
$row[0]	$row[1]	$row[2]	$row[3]
";

echo "
";
?>

<?php
// Get the total number of rows returned by the query.
// Display links to "pages" of rows.
$rowsReturned = sqlsrv_num_rows($stmt);
if($rowsReturned === false)
die( print_r( sqlsrv_errors(), true));
elseif($rowsReturned == 0)
{
echo "No rows returned.";
exit();
}
else
{
// Display page links.
$numOfPages = ceil($rowsReturned/$rowsPerPage);
for($i = 1; $i<=$numOfPages; $i++)
{
 $pageLink = "?pageNum=$i";
 print("$i&nbsp;&nbsp;");
}
echo "

";
}

 sqlsrv_close( $conn );

Do you just mean the links at the bottom of your page? If so you have this code:

// Display page links.
$numOfPages = ceil($rowsReturned/$rowsPerPage);
for($i = 1; $i<=$numOfPages; $i++)
{
    $pageLink = "?pageNum=$i";
    print("$i&nbsp;&nbsp;");
}

Modify this to render just  the first three links, then your ellipses (...) separator and then the final page number.

EDIT to add pseudo code
This is messy but hopefully gives you the idea, create your function, make it work and then post a new question asking how to make it prettier (for example tidy up the ifs, add logic to only render a single set of ellipses, don't repeat any rendering code).

$numOfPages = ceil($rowsReturned/$rowsPerPage);
$renderedEllipses = false;
for($i = 1; $i<=$numOfPages; $i++)
{
    //If you always want to render pages 1 - 3
    if($i < 4) {
        //render link
    }

    //If you always want to render current page number
    else if($i == $pageNum) {
        //render link
        //reset ellipses
        $renderedEllipses = false;
    }

    //if you always want the last page number
    else if ($i == $numOfPages - 1) {
        //render link
    }

    //make sure you only do this once per ellipses group
    else {
      if (!$renderedEllipses){
          print("...");
          $renderedEllipses = true;
      }
    }
}

EDIT added ellipses test
The $renderedEllipses flag will stop hundreds of ellipses being rendered.


C++ - create text file of given size This code creates a text file and writes a given number of characters to it. Is this a good method, or is there a faster method?

#include <fstream>
#include <iostream>

std::ofstream file;

int main() {
    int size = 10000000; //~10MB
    file.open("text_file.txt");
    for (int i = 0; i < size; i++) {
        file << "a";
    }
    return 0;
}

A great deal here depends on what you want to accomplish.

If you want to ensure that the file occupies N bytes of storage space, then your current method may easily fail. Some file systems (e.g., NTFS) support file compression. Writing the same character many times gives highly compressible data, so as you've written it, the file is likely to use only a small amount of storage ("small" is relatively, but certainly a lot less than the ~10 megabytes you intended to write).

If you're all right with that sort of result, then you can probably do quite a bit better with something like this:

int main() {
    std::ofstream file("text_file.txt");
    file.seekp(10000000);
    file << 'a';
}

This will often be much faster than what you've written. It'll also typically occupy very little space on disk. A directory listing will still normally show it as being the same size though.

If you want to ensure that your file really uses ~10 megabytes of disk space, you'll probably need to fill it with semi-random data instead of just repeating a single value. One way to do this would be to use a random number generator:

std::mt19937 gen{ std::random_device()() };

std::uniform_int_distribution<> dis(0, 255);

std::ofstream file("text_file.txt");
std::generate_n(std::ostream_iterator<char>(file, ""), size, [&]{ return dis(gen); });

Code Review

Globals vs. locals

There seems to be no real reason for making file a global variable as you did. Even if you're going to write to all the bytes of the file, it's almost certainly better to make it local to main (and initialize it upon creation):

int main() {
    int size = 10000000; //~10MB
    std::ofstream file("text_file.txt");
    [...]

Standard algorithms

Given that you're trying fill a container with a specified value, you might want to also consider using a standard algorithm designed for precisely that purpose:

    std::ofstream file("text_file.txt");
    std::fill_n(std::ostream_iterator<char>(file), size, 'n');

Use of headers

As it stands right now, you've included fstream.h, even though you don't seem to be using anything that's declared in that header. It's generally preferred to include only the headers you actually use.


Combine Mysql limit query Is it possible to combine these queries into one query? I am trying to see and average all scores but also count and average just the last three based on the same grouping.

This is the main query:

$query4 ="SELECT
studentsisid,coursename,learningoutcomeid,learningoutcomename,count(RecordID) as assessmentcount,
count(if(outcomescore >= 3,outcomescore, NULL)) as over3,
avg(outcomescore) as scoreavg
from studentscores
WHERE studentsisid LIKE '$studentid' AND coursename='$coursename'
Group by studentsisid,coursename, learningoutcomeid
ORDER BY studentname, learningoutcomename";

This is an example of the current sample data based on the query above is below (loid is learningoutcomeid and loname is learningoutcomename)

studentsisid;coursename;loid;loname;assessmentcount;over3;scoreavg
1234;"Course1";"7982";"LearningOutcome1";"1";"1";"3.2"
1234;"Course1";"7995";"LearningOutcome2";"4";"4";"2.5"
1234;"Course1";"7991";"LearningOutcome3";"6";"6";"3.8"
1234;"Course1";"7889";"LearningOutcome4";"1";"1";"3.4"
1234;"Course1";"7839";"LearningOutcome5";"2";"2";"2,6"

The goal is to get something like this where recent over 3 only looks at the last three records for each learningoutcomeid for each student for each course.

studentsisid;coursename;loid;loname;assessmentcount;over3;scoreavg;recentover3;recenscoretavg
1234";"Course1";"7982";"LearningOutcome1";"1";"1";"3.2";"1";"3.2"
1234;"Course1";"7995";"LearningOutcome2";"4";"4";"2.5";"2";"2.5"
1234;"Course1";"7991";"LearningOutcome3";"6";"6";"3.8";"3";"3.2"
1234;"Course1";"7889";"LearningOutcome4";"1";"1";"3.4";"1";"3.2"
1234;"Course1";"7839";"LearningOutcome5";"2";"2";"2,6";"1";"3.2"

This current solution is close but it is only getting the first learningoutcomeid and not the others that match for a particular student for a particular course.

studentsisid;coursename;loid;loname;assessmentcount;over3;scoreavg;recentover3;recentscoreavg
1234";"Course1";"7982";"LearningOutcome1";"1";"1";"3.2";"1";"3.2"

You need to convert your second query to a subquery and join it with the main query. To pass the learning outcome into the inner query you need to use the user-defined variable:

SELECT
    sc.studentsisid,
    sc.coursename,
    sc.learningoutcomeid,
    sc.learningoutcomename,
    count(sc.RecordID) as assessmentcount,
    count(if(sc.outcomescore >= 3,sc.outcomescore, NULL)) as over3,
    avg(sc.outcomescore) as scoreavg,
    l.recentover3,
    l.recentscoreavg
FROM studentscores sc
INNER JOIN (
    SELECT studentsisid,
        learningoutcomeid,
        count(if(outcomescore >= 3,outcomescore, NULL)) as recentover3,
        avg(outcomescore) as recentscoreavg
    FROM (
          SELECT
              (@studentsisid := studentsisid) as studentsisid,
              (@coursename := coursename) as courcename,
              (@learningoutcomeid := learningoutcomeid) as learningoutcomeid,
               @num := if(@studentsisid = studentsisid and
                          @coursename = coursename and
                          @learningoutcomeid = learningoutcomeid,
                       @num + 1, 1) as row_number,
               outcomescore
          FROM studentscores
          ORDER BY studentsisid, coursename, learningoutcomeid, recordid DESC) as r
        where row_number <= 3) AS l
ON sc.studentsisid = l.studentsisid AND sc.learningoutcomeid = l.learningoutcomeid
WHERE sc.studentsisid LIKE '$studentid' AND sc.coursename='$coursename'
GROUP BY sc.studentsisid, sc.coursename, sc.learningoutcomeid
ORDER BY sc.studentname, sc.learningoutcomename


jQuery templating with ES6/ES2015 template strings I have been using a particular pattern over and over again when I want to keep my jQuery apps more safe from XSS. I do something like this. (It uses ES6 template strings)

function doSomething ($el, user) {
  var $html = $(`

`);
  $html.appendTo($el);
  $html.find('.username-text').append(document.createTextNode(user.name));
  $html.find('.description').append(document.createTextNode(user.description));

  return $html;
}

var user = {
  name: "John",
  description: "",
  img: 'http://placehold.it/100x100'
};

doSomething($('body'), user);

JS Bin

User defined fields become text nodes which means I need to create a jQuery element or some sort of DOM structure to hold them. I also use find which means at least in jQuery I have to append it to the actual DOM itself. So I was thinking it would be nice to have a more general way of nesting jQuery elements (I guess you could say "components") inside the template itself kind of like React's JSX but for jQuery.

This is what I came up with. (It uses a tagged template literal which returns a jQuery object).

var jQTemplate = (function ($) {

  var tempFiller = function (key) {
    return `<div id='jQT-jQTemplate-tempFiller-${key}'></div>`;
  };

  var jQTemplate = function (strings, ...values) {
      var $docFrag = $(document.createDocumentFragment());
      var nodeIndexes = [];
      var html = "";
    for(let i = 0; i < strings.length; ++i) {
      html += strings[i];

      var value = values[i];
      var allNodes = Array.isArray(value) && value.every(el=>el instanceof Node || el instanceof jQuery)
      if(value) {
        if(value instanceof Node || value instanceof jQuery || allNodes) {
          nodeIndexes.push(i);
          html += tempFiller(i);
        } else if(value) {
          html += value;// || '';
        }
      }
    }
    $docFrag.append(html);
    nodeIndexes.forEach(function (i) {
      var value = values[i];
      $($docFrag[0].getElementById(`jQT-jQTemplate-tempFiller-${i}`))
    .replaceWith(value);
    });

      return $docFrag;
    };

  return {
    template: jQTemplate,
    textNode: (text) => document.createTextNode(text)
  };

}($));

Here is some example usage: (not the best "app" but shows the different ways I intended it to be used)

/*
* Example usage
*
*/

var {template, textNode} =  jQTemplate;

console.log("started");

function bootstrapButton (type, text) {
  return $(`<a class="btn btn-${type || "default"} btn-lg" href="#" role="button">${text || ""}</a>`)
}

function jumbotron () {
  var $learnMore = bootstrapButton("primary", "Add Content");

  $learnMore.on('click', function (el) {
    $("body").append('
Lorem Ipsum.
');
  })

  return template`

Hello, world!

...

${$learnMore}

`;
}

$("body").on("click", '.js-deleteParent', function () {
  $(this).parent().remove();
});

var userInput = ['', "more user input"];

var trustedHtmlContent = [`<h1>Header</h1>`, `<h3>Sub Header</h3>`];

var extraClass ="container";

template`

      ${trustedHtmlContent.map(el=>$(el))}
      ${userInput.map(el=>textNode(el))}

     ${jumbotron()}

`.appendTo("body");

JS Bin

Any thoughts, suggestions, or comments? Is there any reason why this shouldn't be used in production?

I was reading through your code and I got to

  if(value) {
    if(value instanceof Node || value instanceof jQuery || allNodes) {
      nodeIndexes.push(i);
      html += tempFiller(i);
    } else if(value) {
      html += value;// || '';
    }
  }

and I had to read over it a few times, but I think that you can do away with the outside if statement or at least not check value a second time.

either one of these look like they are going to do about the same thing

if (value instanceof Node || value instanceof jQuery || allNodes) {
    nodeIndexes.push(i);
    html += tempFiller(i);
} else if (value) {
    html += value;
}

this would go through both checks, much the same way that the original code did, except you don't check the existence of value first, this might cause some errors, so I would probably do this instead

if (value) {
    if (value instanceof Node || value instanceof jQuery || allNodes) {
      nodeIndexes.push(i);
      html += tempFiller(i);
    } else {
      html += value;
    }
}

turned the else if into an else, because we already know that value is going to pass a truthy test, no need to check it a second time.


Find the biggest rectangle in a bar graph Find the largest rectangle area on a bar graph. Example: if you had a graph of 1,4,5,3,3,5 the answer would be 15 (3x5) which is formed by the rectangle that is 3 high and spans from position 2 (1 based) to 6.

Please review my answer and comment about runtime complexity, memory usage, and basically anything else you think I need to improve if this was a job interview for your company.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Interviews
{
    class Program
    {
        static void Main(string[] args)
        {
            List<int> barGraph = new List<int>() { 1, 4, 5, 3, 3, 5 };
            int max = GetBiggestRectSize(barGraph);
            //should output 15
            barGraph = new List<int>() { 1, 4, 2, 3, 3, 5 };
            max = GetBiggestRectSize(barGraph);
            //should output 10
        }

        private static int GetBiggestRectSize(List<int> barGraph)
        {
            int maxRectSize = 0;
            int max = barGraph.Max();
            for (int i = 1; i <= max; i++)
            {
                int tempSize = 0;
                for (int j = 0; j < barGraph.Count; j++)
                {
                    if (barGraph[j] >= i)
                    {
                        tempSize += i;
                    }
                    else
                    {
                        if (tempSize > maxRectSize)
                        {
                            maxRectSize = tempSize;
                        }
                        tempSize = 0;
                    }
                    if (tempSize > maxRectSize)
                    {
                        maxRectSize = tempSize;
                    }
                }
            }
            return maxRectSize;
        }
    }
}

As others have pointed out, your algorithm is very inefficient. Especially if the size of the columns vary widely:

int[] columns = { 1, 5, 40000, 3 } // Just to make my point clear :-)

This will cause your algorithm to check 39996 numbers (= column heights) for no reason.

So a minor change in your code could make it a lot more efficient, although it isn't still a very efficient approach:

private int GetBiggestRectSize(List<int> barGraph)
{
  int maxRectSize = 0;
  //int max = barGraph.Max();

  // CHANGE: Instead of checking all values from max to 1 - only check values in the input graph.
  foreach (int i in barGraph)
  {
    int tempSize = 0;
    for (int j = 0; j < barGraph.Count; j++)
    {
      if (barGraph[j] >= i)
      {
        tempSize += i;
      }
      else
      {
        if (tempSize > maxRectSize)
        {
          maxRectSize = tempSize;
        }
        tempSize = 0;
      }
      if (tempSize > maxRectSize)
      {
        maxRectSize = tempSize;
      }
    }
  }

  return maxRectSize;
}


Is this a correct implementation/use of move semantics? I'm trying to account for the fact that my program will contain calculations such as:

v(t) = w ∗ v(t−1)+ c1∗r1∗( pBest − x(t −1)) + c2∗r2∗(gBest − x(t−1))

where v, x, pBest, gBest are vectors (std::arrays). I'm a little worried if I make correct use of the move semantics.

Here's the relevant snippet:

template<class T, class F, size_t l> std::array<T, l> operator *(std::array<T, l>&& lhs, const F rhs ) {

    static_assert(std::is_arithmetic<T>::value, "The type parameter of the first argument is not of arithmetic type");
    static_assert(std::is_arithmetic<F>::value, "The passed scalar is not of arithmetic type");

    std::array<T, l> tmp = std::move(lhs);

    for(size_t i = 0; i < l; ++i)
        tmp[i] *= rhs;

    return tmp;
}

//calling the above operator
template<class T, class F, size_t l> std::array<T, l> operator *(const F rhs, std::array<T, l>&& lhs ) {

    return std::move(lhs) * rhs;
}

POST-ANSWER EDIT: As Loki Astari pointed out this SHOULD NOT be the sole implementation for operator* as this can bind to lvalues and will destroy them.

Yes BUT.

The std::array itself is not moveable. But it is still optized for move semantics when its content type T is moveable. So it will move each of its internal elements rather than copy them. But your function limits the type of T to std::is_arithmetic which is only defined for built-in integers and floats. Moving integers and floats is the same as copying them. So you get no real benefit.

But lets take this a step further.
If you remove the static_assert so it can be used by any type that supports the operator* etc.. then you have a usability issue.

When you do a multiplication like this you destroy the original array (Its content has been moved so there is no way to know what the elements are or if they are usable. About the only thing you can do is reset them to a specific state).

#include <array>

int main()
{
    std::array<MyMathType, 5>  a = initArray();
    std::array<MyMathType, 5>  c;

    // Your code moves the content of a here.
    c = a * 5;

    // So at this point the content of a is in an undefined state
    // So you can not rely on what is in `a` or use it in ANY way that would
    // need the content to be in a defined state (all you can really do is reset).
    //
    // This is not what I would expect of `operator*`
    // So it is quite easy to make mistakes with this code.
}


Bare bones SinglyLinkedList in C++11 I have done a bare bones implementation of a singly linked list in C++, which works:

Node class

template <typename T>
class ListNode{
    template <typename U > friend class SinglyLinkedList;
private:
    T data_;
    ListNode<T>* next_;
public:
    ListNode(void):
            data_{0},
            next_{nullptr}{}
    ListNode(T data_):
            data_{data_},
            next_{nullptr} {}

};

List class:

template <typename T>
class SinglyLinkedList{
private:
    ListNode<T>* head_;
    std::size_t size_;
public:
    SinglyLinkedList():
            head_{nullptr},
            size_{0}{}

    void insert(T item){
        ListNode<T> * p{new ListNode<T>(item)};
        if (size_ == 0){
            head_ = p;
        }else{
            p->next_ = head_;
            head_ = p;
        }
        size_++;
    }
    std::size_t getSize(){
        return size_;
    }

};

Here's what I'm looking to learn and understand:

Where is the newed up Node in the insert deleted (released)?
Is there a way to implement this without new at all?
Is there a difference between the following lines?

ListNode(void): data_{0}, next_{nullptr}{}

and

ListNode(void): data_(0), next_(nullptr){}

It seems to me that the ListNode type is part of the SinglyLinkedList. So there's no reason for that to be visible outside the SLL. I suggest making it a nested/private/hidden/secret class. (There are many ways to hide it, pick the one you like.)
Your method names are confusing. The std namespace has any number of collections defined. Pick one of them, and model your naming on that. For example: std::vector has a size method that returns the size. It also defines push_back and pop_back. Alternatively, std::list also defines push_front and pop_front. You need to pick a usage model for your SLL, and then choose method names compatible with it.
What, no iterators?


Print the largest odd number Ask user to input 10 integers and then print the largest odd number that was entered. If no odd number was entered, print a message to that effect.

I've written this code as my solution to the above exercise from chapter 2 of Computation and Programming Using Python by John Guttag. The book is meant to be used with Python 2.x. I need to know if the code can be "straightened up" in any way. So far, the book has covered variable assignment, print function, conditional branching, and loops.

largestOddNumber = None

for _ in range(10):  # '_' is a throw-away var
    number = int(raw_input('Enter an integer: '))
    if number % 2 != 0 and number > largestOddNumber:
        largestOddNumber = number

print largestOddNumber or 'No odd number was entered.'

Code looks good! I only have minor/pedantic comments:

for _ in range(10):  # '_' is a throw-away var

That comment is useless, the naming of the variable as _ is sufficient to indicate that we don't intend on using it. Also, in python2.7, range() returns a full list of all the values, which can get pretty inefficient as you go up, so you should prefer to use its lazier cousin, xrange():

for _ in xrange(10):

Then this:

print largestOddNumber or 'No odd number was entered.'

While I do like that or in python, rather than returning a bool returns the first argument that isn't False/None, it's a little difficult to read. Now, the code is perfectly correct for odd numbers but would actually be wrong for even numbers - if 0 ended up being the largest even number, we'd print the string. So I find it simpler to just be explicit:

if largestOddNumber is not None:
    print largestOddNumber
else:
    print 'No odd number was entered'

But really, the code is fine.


More testable record insertion code I don't have much experience with testing but I'm trying to make it part of my routine to develop better software.

The class below is a service class in Laravel. It needs to save a record on the database, saving the customer's customer_id, a route (location_id_from and location_id_to) and which location to bill (location_id_bill). The billing location can be null, in which case it will try to determine which one to use.

The code works, but I don't see how to completely unit test the newJob function. From what I've learned so far, I could replace the static methods whereId with an injected repository in the constructor.

Regarding the validation, I'm currently doing the validation, and if it doesn't passes, it will throw an exception with all errors messages.

So:

how to test the actual job creation without touching the database
is there a better way to do the validation?

class Job
{

    public function newJob(int $customer_id, int $location_id_from, int $location_id_to, int $location_id_bill): \App\Models\Customer\Job
    {
        $location_from = Location::whereId($location_id_from);
        $location_to = Location::whereId($location_id_to);

        if ($location_id_bill === null) {
            $location_bill = Location::whereId($location_id_bill);
        } else {
            $location_bill = new Location();
        }

        $errors = $this->newJobValidate($customer_id, $location_from, $location_to, $location_bill);
        if (count($errors)) {
            throw ValidationException::withMessages($errors);
        }

        $job = new \App\Models\Customer\Job();
        $job->customer_id = $customer_id;
        $job->location_id_from = $location_from->id;
        $job->location_id_to = $location_to->id;
        $job->location_id_bill = $location_bill->id;
        $job->save();

        return $job;
    }

    private function newJobValidate(int $customer_id, Location $location_from, Location $location_to, Location $location_bill)
    {
        $errors = [];

        if ($location_to->type === Location::TYPE_COMMON && $location_from->type === Location::TYPE_COMMON) {
            // same location, but not customer's
            $errors[] = __('job.errors.same_location_common');
        }

        if ($location_to->type === Location::TYPE_CUSTOMER && $location_to->customer->customer_id !== $customer_id ||
            $location_from->type === Location::TYPE_CUSTOMER && $location_from->customer->customer_id !== $customer_id) {
            $errors[] = __('job.errors.location_doesnt_belong_to_customer');
        }

        if ($location_bill->id === null) {
            if ($location_to->type === Location::TYPE_CUSTOMER && $location_from->type === Location::TYPE_CUSTOMER) {
               $errors[] = __('job.errors.must_select_bill_location');
            } elseif ($location_from->type === Location::TYPE_CUSTOMER) {
                $location_bill->id = $location_from->id;
            } else {
                $location_bill->id = $location_to->id;
            }
        }

        if ($location_bill->id !== $location_from->id && $location_bill->id !== $location_to->id) {
            $errors[] = __('job.errors.bill_location_different');
        }

        return $errors;
    }
}
```

An important question: why don’t you want to touch the database in the test? You have a few different options for getting phpunit to play nicely with a database. Doing so lets you test things more thoroughly as you can do assertions that the data has landed in the database as you expected.

As a starting point, based on what I see in this method, I would recommend adding something along these lines in phpunit.xml within the <php> tag

 <env name="DB_DATABASE" value="testing"/>

And create a database lined up with the “testing” value set in DB_DATABASE. These values equate to .env values, so you can add additional DB_USERNAME, etc values as appropriate.

You can use the RefreshDatabase trait to reset migrations within your newly created testing database on each test. That is described at https://laravel.com/docs/6.x/database-testing#resetting-the-database-after-each-test

It looks like you need some Location and Customer data in your database for this job to work. If you don’t already have factories for these models, make some as described at https://laravel.com/docs/6.x/database-testing#writing-factories

As a starting point you can manually call factories in your test – factory(App\Customer::class)->create(); will put a factory generated customer into your database. If you need three of them, – factory(App\Customer::class, 3)->create(); The factory method returns the created entities, so you can do:

    $customer = factory(App\Customer::class)->create();
    $locations = factory(App\Location::class, 3)->create();
    newJob(
     $customer->id,
     Arr::get($locations, 0)->id,
     Arr::get($locations, 1)->id,
     Arr::get($locations, 2)->id
);

Now the job should be in your testing.jobs table, and you can make an assertion of that:

$this->assertDatabaseHas(‘jobs’,[
‘customer_id’ => $customer->id,
‘location_id_from’ => Arr::get($locations, 0)->id,
‘location_id_to’ => Arr::get($locations, 1)->id,
‘location_id_ bill’ => Arr::get($locations, 2)->id,
]);

Yes there’s a some overhead in getting the database incorporated into your test setup and in writing the factory, but the end result is a more thorough test. Worth it, in my opinion.

I hope something here helps!


Which one of the two enum implementation is better? First Implementation

public enum ReviewFlowExample {

    Draft {

        @Override
        public ReviewFlowExample getNext() {
            return Review;
        }

        @Override
        public ReviewFlowExample getPrevious() {
            return null;
        }

    },
    Review {
        @Override
        public ReviewFlowExample getNext() {
            return Final;
        }

        @Override
        public ReviewFlowExample getPrevious() {
            return Draft;
        }

    },
    Final {
        @Override
        public ReviewFlowExample getNext() {
            return null;
        }

        @Override
        public ReviewFlowExample getPrevious() {
            return Review;
        }

    };

    public abstract ReviewFlowExample getNext();

    public abstract ReviewFlowExample getPrevious();

    public boolean isDraft() {
        return this.equals(Draft);
    }
}

Second Implemenation

public enum ReviewFlowExample {
    Draft,
    Review,
    Final;
    private ReviewFlowExample next;
    private ReviewFlowExample previous;

    static{
        Draft.setNext(Review);
        Review.setNext(Final);
        Review.setPrevious(Draft);
        Final.setPrevious(Review);
    }
    private ReviewFlowExample(){
    }

    public ReviewFlowExample getNext(){
        return next;
    }

    public ReviewFlowExample getPrevious(){
        return previous;
    }

    private void setNext(ReviewFlowExample next){
        this.next = next;
    }

    private void setPrevious(ReviewFlowExample previous){
        this.previous = previous;
    }

    public boolean isDraft(){
        return this == Draft;
    }

}

Why not use the order you are setting up in the enum declaration? I played with this a while ago and came up with something like this (modified to fit your implementation above):

private enum Planet { MERCURY, VENUS, EARTH, MARS, JUPITER, SATURN, URANUS, NEPTUNE;
   public Planet getNext() {
       return this.ordinal() < Planet.values().length - 1
           ? Planet.values()[this.ordinal() + 1]
           : this;
   }
   public Planet getPrevious() {
       return this.ordinal() > 0
           ? Planet.values()[this.ordinal() - 1]
           : this;
 }


Reading and writing an unknown character into an appropriately named file I would like to refactor this code a bit further and make it better and more generic.

As of right now, it is doing what I want it to do (reading a list of URL's, splitting the query and the ampersand and writing the split into appropriately named files). However, I would like to take the hash list out and make it as generic as possible. I guess what I mean is, if the loop finds a character, read what the character is (i.e: a=12(character would be 'a')), have Python create a text file for it ('a_file.txt') and append into the newly created text file. So, there was not file there before, but there is now. And Python would do this for every character past the query (http://www.asdasd.com/asds/asd?a=6&b=12&c=123 would create 'a_file.txt', 'b_file.txt', 'c_file.txt').

I feel as if I am asking the right question, but correct me if I am wrong.

import urlparse
def parse_file(input_file):
  tags = {tag: tag + '_file.txt' for tag in {'blog',
        'p','attachment_id','lang',
        'portfolio','page_id','comments_popup',
        'iframe','width','height'}}
with open(input_file) as input:
for line in input:
    parsed_url = urlparse.parse_qsl(urlparse.urlparse(line).query)
    if parsed_url > 0:
        for key, value in parsed_url:
            if key in tags:
                with open(tags[key], 'a') as output_file:
                    output_file.write(value)
            else:
                print key + " not in tags."
    else:
        print(line + " does not yield query.")
parse_file()

Once again, correct me if I am wrong or if my question is confusing.

This is only partly an answer, intended more as a thought exercise about what you might do. I provide a couple examples that you might want to pick and choose parts of, but you probably will not want to just grab the whole. So let's dive in.

What would benefit from this being more general? This seems like a fairly small and specific task. I could imagine parameterizing the input, the list of tags, the mapping from tag name to file name, or the handling once a matched tag is found. Let's examine doing all at once:

import urlparse
def parse_queries(input, get_filename, handle, tags={'blog', 'p', ...}):
    for line in input:
        qsl = urlparse.parse_qsl(urlparse.urlparse(line).query)
        if not qsl:
            print repr(line), "does not yield query"
            continue
        for key, value in qsl:
            if key in tags:
                handle(get_filename(tag), value)
            else:
                print key, "not in tags."

def dot_text(tag):
    return tag + '_file.txt'

def append_to_file(filename, value):
    with open(filename, 'a') as output_file:
        output_file.write(value)

with open("filename") as input:
    parse_queries(input, get_filename=dot_text, handle=append_to_file)

Do any of these help more than they harm readability? Does extracting the small methods like dot_text and append_to_file offer useful chunks of reusable code? I'm not sure myself. It certainly makes the call harder, but providing defaults is easy enough. Thinking it through should raise other questions, such as whether get_filename should be part of parse_queries, or part of handler.

Perhaps it's the line processing which could be parameterized:

def parse_file(input_file, parse):
    ...
    for line in input:
        for key, value in parse(line):
            if key in tags:
                ...

def extract_query_params(line):
    qsl = urlparse.parse_qsl(urlparse.urlparse(line).query)
    if not qsl:
        print repr(line), "does not yield query"
    return parsed_url

parse_file("filename", extract_query_params)

But again ask whether this helps generality more than it harms readability. I'd address other small issues first:

comparing a list to 0 (parsed_url > 0) doesn't have much meaning.
output_file.write(value) seems unlikely to insert newlines, so your values will blur together.
the name tags has a lot of implied meaning in an HTML context, so your use here in an HTTP context could be confusing.
indentation, at least here, your with statement is at the same level as its contained for loop.
unit tests; how better to verify whether this does what you want? Of course if you do want to test this, you may find that parameterizing things makes this measurably easier. Or maybe you really need to look at the resulting files.


A simple prompt-and-print program I have accomplished what I wanted to do but I think this can be done better. I want my code to look professional.
What my code does is ask for your name and gives output in below form.

Input: Jack
Output: j = joyful a = awesome c = curious k = kindhearted

Please show me how to code in a professional manner by reviewing the below code.
#!/usr/bin/env python
import sys
import string

names = {
'a': 'awesome',
'b': 'bold',
'c': 'curious',
'd':'delightful',
'e':'emotional',
'f':'fearless',
'g':'gifted',
'h':'helpful',
'i':'imaginary',
'j':'joyful',
'k':'kindhearted',
'l':'lovable',
'm':'memorable',
'n':'naughty',
'o':'open',
'p':'playful',
'q':'quarrelsome',
'r':'reliable',
's':'serious',
't':'thoughtful',
'u':'unique',
'v':'victorious',
'w':'wise',
'x':'xerox copy',
'y':'yummy',
'z':'zealous'
}

def main():
 i=j=0
 user = raw_input("What is your name?: ")
 print ""
 while i<len(user):
  while j<len(names):
   if user[i]==names.keys()[j]:
    print " " + user[i] + ' = ' + names.values()[j]
   j=j+1
  while j!=0:
   j=0
  i=i+1

if __name__ == '__main__':
  main()

Style

A few things can be easily detected/improved in your code regarding the style. If you want, you'll find various tools to perform checks automatically : pep8, pyflakes, pychecker. They'll give you a great deal of interesting information, from imported module xxx is not used, to bad indentation via missing documentation, wrong spacing and invalid constant name. Also, it might be good for you to know that in Python, there is a style guide called PEP8 and it's definitely worth a read.

After fixing everything to make the tools happy, your code looks like:

"""Module docstring"""

NAMES = {
    'a': 'awesome',
    'b': 'bold',
    'c': 'curious',
    'd': 'delightful',
    'e': 'emotional',
    'f': 'fearless',
    'g': 'gifted',
    'h': 'helpful',
    'i': 'imaginary',
    'j': 'joyful',
    'k': 'kindhearted',
    'l': 'lovable',
    'm': 'memorable',
    'n': 'naughty',
    'o': 'open',
    'p': 'playful',
    'q': 'quarrelsome',
    'r': 'reliable',
    's': 'serious',
    't': 'thoughtful',
    'u': 'unique',
    'v': 'victorious',
    'w': 'wise',
    'x': 'xerox copy',
    'y': 'yummy',
    'z': 'zealous'
}

def main():
    """Main function"""
    i = j = 0
    user = raw_input("What is your name?: ")
    print ""
    while i < len(user):
        while j < len(NAMES):
            if user[i] == NAMES.keys()[j]:
                print " " + user[i] + ' = ' + NAMES.values()[j]
            j = j + 1
        while j != 0:
            j = 0
        i = i + 1

if __name__ == '__main__':
    main()

Loops

Your are going through your containers (user and names) with a while loop. This is a bit impractical but if you really want to do so, it is clearer to initialise the variable you are going to loop with before the loop so that you not have to try to reset it after the loop.

i = 0
while i < len(user):
    j = 0
    while j < len(NAMES):
        if user[i] == NAMES.keys()[j]:
            print " " + user[i] + ' = ' + NAMES.values()[j]
        j = j + 1
    i = i + 1

Even better, the range function can generate the values your are looking for so that you do not need to do the incrementation yourself :

for i in range(len(user)):
    for j in range(len(NAMES)):
        if user[i] == NAMES.keys()[j]:
            print " " + user[i] + ' = ' + NAMES.values()[j]

but Python provides you an even better way to iterate over containers :

for c in user:
    for j in range(len(NAMES)):
        if c == NAMES.keys()[j]:
            print " " + c + ' = ' + NAMES.values()[j]

Also, because you are using a dictionary, there is no need to loop over NAMES.  You could lookup keys in the dict naturally:

for c in user:
    if c in NAMES:
        print " " + c + ' = ' + NAMES[c]


Flatten a nested dict structure in Python For some post-processing, I need to flatten a structure like this

{'foo': {
          'cat': {'name': 'Hodor',  'age': 7},
          'dog': {'name': 'Mordor', 'age': 5}},
 'bar': { 'rat': {'name': 'Izidor', 'age': 3}}
}

Each bottom entries will appear as a row on the output. The heading keys will appear each row, flattened. Perhaps an example is better than my mediocre explanation:

[{'age': 5, 'animal': 'dog', 'foobar': 'foo', 'name': 'Mordor'},
 {'age': 7, 'animal': 'cat', 'foobar': 'foo', 'name': 'Hodor'},
 {'age': 3, 'animal': 'rat', 'foobar': 'bar', 'name': 'Izidor'}]

I first wrote this function:

def flatten(data, primary_keys):
    out = []
    keys = copy.copy(primary_keys)
    keys.reverse()
    def visit(node, primary_values, prim):
        if len(prim):
            p = prim.pop()
            for key, child in node.iteritems():
                primary_values[p] = key
                visit(child, primary_values, copy.copy(prim))
        else:
            new = copy.copy(node)
            new.update(primary_values)
            out.append(new)
    visit(data, { }, keys)
    return out

out = flatten(a, ['foobar', 'animal'])

I was not really satisfied because I have to use copy.copy to protect my input arguments. Obviously, when using flatten one does not want its input data to be altered.

So I thought about one alternative that uses more global variables (at least global to flatten) and uses an index instead of directly passing primary_keys to visit. However, this does not really help me to get rid of the ugly initial copy:

keys = copy.copy(primary_keys)
keys.reverse()

So here is my final version:

def flatten(data, keys):
    data = copy.copy(data)
    keys = copy.copy(keys)
    keys.reverse()
    out = []
    values = {}
    def visit(node, id):
        if id:
            id -= 1
            for key, child in node.iteritems():
               values[keys[id]] = key
               visit(child, id)
        else:
            node.update(values)
            out.append(node)
    visit(data, len(keys))
    return out

I am sure some Python magic will help in this case.

Both algorithms recurse using the length of keys to stop, so I am going to assume that the nested dictionaries always have the same level of nesting too. If your input can be of the form:

{'foo': {
          'cat': {'name': 'Hodor',  'age': 7},
          'dog': {'name': 'Mordor', 'age': 5}},
 'bar': { 'rat': {'name': 'Izidor', 'age': 3}},
 'baz': 'woops',
}

then your approach can't handle it and neither will mine.

I quickly stopped trying to understand how your algorithm work and started to think about how I would implement it myself. This indicates that:

your algorithm is not that trivial;
it is poorly documented.

You should at least have comments indicating why you use some approaches: reversing the keys and iterating over them in decreasing order, storing your group names/values (values[keys[id]] = key) as you go into nesting levels and updating the last dictionary when you reach it…

Speaking about updating the last dictionary, note that data = copy.copy(data) does not protect your node.update(values) to modify the original data in place. You either need to use copy.deepcopy or to change the updated dictionary (create a new one and update it with both node and values).

Now let me show you an other approach. Rather than wrapping a function that access global variables (this is what visit look like) into flatten, you can make flatten the recursive function by splitting keys into its head and tail part. When there is no element left, you won't be able to do it and you can stop the recursion by returning the data you're on: this is one of the most nested dictionaries.

Otherwise, you can iterate over the key/values pairs, flatten the values using the tail as a new set of keys and then, build a list out of the flattened values and the {head: key} dictionary.

To make things a bit more efficient, I'll use generators instead of building lists, so you’ll want to change your calls from out = flatten(a, ['foobar', 'animal']) to out = list(flatten(a, ['foobar', 'animal'])) (calls of the form for flattened in flatten(a, ['foobar', 'animal']): don't need to be changed though):

def flatten(data, group_names):
    try:
        group, group_names = group_names[0], group_names[1:]
    except IndexError:
        # No more key to extract, we just reached the most nested dictionnary
        yield data.copy()  # Build a new dict so we don't modify data in place
        return  # Nothing more to do, it is already considered flattened

    for key, value in data.iteritems():
        # value can contain nested dictionaries
        # so flatten it and iterate over the result
        for flattened in flatten(value, group_names):
            flattened.update({group: key})
            yield flattened

I also changed keys to group_names to be able to use the generic names key and value when iterating over data.

In case the input data can ever contain less nested levels than the amount of items in group_names, you'll reach a point where data.iteritems() will raise and AttributeError. You can catch that if you so choose:

def flatten(data, group_names):
    try:
        group, group_names = group_names[0], group_names[1:]
    except IndexError:
        # No more key to extract, we just reached the most nested dictionnary
        yield data.copy()  # Build a new dict so we don't modify data in place
        return  # Nothing more to do, it is already considered flattened

    try:
        for key, value in data.iteritems():
            # value can contain nested dictionaries
            # so flatten it and iterate over the result
            for flattened in flatten(value, group_names):
                flattened.update({group: key})
                yield flattened
    except AttributeError:
        yield {group: data}

So

a = {
    'foo': {
        'cat': {'name': 'Hodor',  'age': 7},
        'dog': {'name': 'Mordor', 'age': 5},
    },
    'bar': {
        'rat': {'name': 'Izidor', 'age': 3},
    },
    'baz': 'woops',
}

list(flatten(a, ['foobar', 'animal']))

will return

[{'animal': 'woops', 'foobar': 'baz'},
 {'age': 5, 'animal': 'dog', 'foobar': 'foo', 'name': 'Mordor'},
 {'age': 7, 'animal': 'cat', 'foobar': 'foo', 'name': 'Hodor'},
 {'age': 3, 'animal': 'rat', 'foobar': 'bar', 'name': 'Izidor'}]


A method which, given a string, returns a string in which each character is separated by an asterisk Use recursion in the solution.

I would really appreciate if you can help me to find a better way to do it.

public class AppendCharacterAfterEachCharacterInString
{
    int index =0;
    StringBuilder outputString = new StringBuilder();

    public string AppendInputCharacterAfterEachCharacterInString(string sInputString, string inputRepeatCharacter)
    {
        if (string.IsNullOrEmpty(sInputString))
        {
            Console.WriteLine("Empty string !");
        }
        else
        {
            BuildNewString(sInputString, inputRepeatCharacter);
        }

        return outputString.ToString();
    }

    private void BuildNewString(string inputString, string repeatCharacter)
    {
        if (index < inputString.Length)
        {
            outputString.Append(inputString[index]);

            if(!(inputString[index].ToString().Trim() == string.Empty) && !(index + 1 == inputString.Length))
              outputString.Append(repeatCharacter);

            index++;
            BuildNewString(inputString, repeatCharacter);
        }

    }

Well, you've used recursion, but I wouldn't say that's a recursive solution because you have variables outside your functions that control what your functions do.

A recursive solution will have a function that is repeatedly called with smaller and smaller sub-problems, terminating in a base case, that together will accomplish your goal. A key aspect of recursive solutions is that the recursive function returns a value - your method is void and so returns no value.

As a first step, discard your index variable entirely. At each recursive step, call BuildNewString with a smaller version of the same problem. Combine the results in such a way as to produce the desired result.


How to reduce load when fading in multiple elements in jQuery I have a dynamic order page which has to update multiple elements when a value is changed.

Here is the main function that does that. Note it only updates the values if they are different from what is already there. So for example it doesn't update with a checkmark, if there already is a check mark there.

  function reput(element, content, animate) {
        target = $(element);
        target_check = target.html().indexOf("check_mark");
         //checks if target already has a check mark
        content_check = content.indexOf("check_mark");
         //checks if new value has check mark
        th = target.html();
        if(th == content || (target_check == content_check && target_check != -1)) {
            return true;
            //if any of the values match, then don't do anything
        }
        else {
            //values didn't match so update the element
            if (animate || animate == undefined)
                target.fadeOut(100).empty().fadeIn(200).append(content);
            else
                target.empty().append(content);
        }
    }

This function is called multiple times in several places like so:

reput(("#email"),   (cp.email));
reput(("#chat"),    (cp.chat));
reput(("#voice"),   (cp.voice));
reput(("#member"),  (cp.member));
reput(("#price"),   ('$' + cp.price), false);

What would be the best way to improve the load time of this function? Having it accept an array, iterating through it and appending the values one by one? Would it save resources to just use fadeIn instead of fadeOut/fadeIn?

Bonus question: What are some ways to test how much processing power your script/function is using up?

for optimizing this function - will be better if you'll give an example of working code. Just upload some sample to http://jsfiddle.net/.

Anyway, to reduce the load on any animations, use default CSS transitions.

-moz-transition: all 0.5s ease-out;
-o-transition: all 0.5s ease-out;
-webkit-transition: all 0.5s ease-out;
transition: all 0.5s ease-out;

And then change the opacity from 1 to 0 or change class. Here is an working example http://jsfiddle.net/x6fxx/ , works much faster then jquery's script, this one is css based solution which is using hardware acceleration ( modern browsers ).


Switch an image based on click coordinate I'm still learning how to use jQuery. I created a function where when I click on some coordinate, it will then show a different image. I would like to make the code cleaner and more reusable, but don't know how.

https://jsfiddle.net/ewtt0kzg/

$('.main-image').click(function(e) {
  var main_image = $(this);
  var topleft = $(this).siblings('.single-page-image').eq(0).attr('src');
  var botLeft = $(this).siblings('.single-page-image').eq(1).attr('src');
  var botRight = $(this).siblings('.single-page-image').eq(2).attr('src');
  var topRight = $(this).siblings('.single-page-image').eq(3).attr('src');
  var width = $(this).width();
  var height = $(this).height();
  var offset = $(this).offset();
  var relativeX = (e.pageX - offset.left);
  var relativeY = (e.pageY - offset.top);
  if (relativeY < (height / 2) && relativeX < (width / 2)) {
    main_image.attr('src', topleft);
  }
  if (relativeY < (height / 2) && relativeX > (width / 2)) {
    main_image.attr('src', topRight);
  }
  if (relativeY > (height / 2) && relativeX < (width / 2)) {
    main_image.attr('src', botLeft);
  }
  if (relativeY > (height / 2) && relativeX > (width / 2)) {
    main_image.attr('src', botRight);
  }
});
.second {
  display: none;
}

Overly complex

The code you have is overyly complex

Some issues

Don't use classes to locate single items.

If you wish to identify single elements use the id property to create a unique name and use that to locate and element.

Don't define what is not needed.

At the beginning of your function you define each image's src but the function will only ever use one of them. Rather than define them all set the src in the individual condition blocks. The same with var main_image = $(this); there is no reason you need to do this, just use the reference $(this) or better yet this. You don't have to wrap it in jQuery's code if not needed.

Don't mix and match naming styles.

You name a variable using snake_case in one spot then in others you use camelCase. Not that it affects your code, but later when you need to recall a variable, you remember it by its name, if you mix and match, can you be sure you remember if it is snake_case or camelCase.

Use the one style (camelCase) as that is what JavaScript uses and you will never have to guess, or look up what style a variable used.

Use else

If you have a set of mutually exclusive conditional statement you should use else. Only one of the conditions will pass so there is no need to test other conditions if one has passed. Also as at least one condition has to pass the last else need not have the if statement it is the last and only option left so it must pass.

Use constants if a variable will not change.

var foo = "data" the variable foo can be assigned a new value which is all good if that is the intention. But many time you assign a variable a value only once. If you at some stage accidentally assign that variable a value due to a typo in the source code, you may spend hours, days even looking for the problem, as where the problems occur may not be where the problem starts. And worse, you may not spot the problem due to lack of testing until your code is out there.

const foo = "data" This tells Javascript that the variable foo is a constant and will never change. If you try to change it (via typo in source code) Javascript will throw an error, letting you detect the problem and direct you to the location of the problem. You can fix it there and then saving a lot of time and effort.

Though it is not a major point in this example, using the correct variable type in every situation helps build good habits.

Don't repeat

Modern languages came from a need to keep lazy programmers happy. There is nothing worse than having to repeat the same code over and over. For you typing it, it means you are more likely to make a mistake, it means it takes longer to get a result, and it means (mostly but not always) your code will take longer to run. The worst bit about repeating code is when you have to change it.

You have the variables width and height and for each test you do the same thing offsetX < width / 2, repeating the same code.

Do it once const halfWidth = $(this).width() / 2; event better do the width test only once const leftClicked = (e.pageX - offset.left) < ($(this).width() / 2);

Rewrite of your code.

// Note for lines assigning leftClicked, rightClicked
// I use (...) to wrap expressions though not needed,
// it is good habit to do so for readability
$(".main-image").click(function(e) {
    const images = $(this).siblings(".single-page-image");
    const offset = $(this).offset();
    const leftClicked = (e.pageX - offset.left) < ($(this).width() / 2);
    const topClicked = (e.pageY - offset.top) < ($(this).height() / 2);
    var imageIndex = 2; // default image index for bottom right
    if (topClicked){
        if (leftClicked) {
            imageIndex = 0;
        } else {
            imageIndex = 3;
        }
    } else if (leftClicked) {
        imageIndex = 1;
    }
    $(this).attr("src", images.eq(imageIndex).attr("src"));
});

Is there a better way?

It is often better to embed as much context / semantic specific information in the page.

In this example you could have used a map element to define the click areas and used Javascript just to change the image src.

Example

The example shows it done with minimum required code. I know that the sandboxed page is completely under my control and can thus reliably use directly referenced elements to add functionality. It is done without jQuery to keep it simple.

imageMap.addEventListener("click", (e) => {
  mainImage.src = window[e.target.dataset.imageId].src;
});
.hide { display: none; }
<map id="imageMap" name="imageAreas">
  <area shape="rectangle" coords="0,0,200,200" title="One" data-image-id = "image1"/>
  <area shape="rectangle" coords="200,0,400,200" title="Two"  data-image-id = "image2"/>
  <area shape="rectangle" coords="0,200,200,400" title="Three"  data-image-id = "image3"/>
  <area shape="rectangle" coords="200,200,400,400" title="Four"  data-image-id = "image4"/>
</map>

A word about jQuery

A word of warning. JQuery is a popular library, and in its day was a good solution to the problem of cross browser compatibility, and has some relevance today (dam IE11). Unfortunately it has produced a generation of front end coders that are unable to use the DOM directly. As a beginner you are training your self for the future, jQuery is struggling to stay relevant now. Don`t invest precious time where you don't need to. The step from vanilla to JQuery is a lot easier than the other way around, so be careful where you invest your time.


Python DictionaryTagger class takes too long to initialize With big inspiration from code from different webpages, I have created this DictionaryTagger. My problem is that it takes around 2 sec. to initialize the taggers / class.

When the tagger has been initialized, it only takes a few milliseconds to use the different taggers. I see two different ways to optimize the tagger:

Do the initialize of the tagger in some preload function. This have
to be used through a homepage. If it could be initialize and the stay
in memory all the time.
Rewrite the whole class with eye for performance

class DictionaryTagger(object):

def __init__(self, dictionary_paths):
    files = [open(path, 'r') for path in dictionary_paths]
    dictionaries = [yaml.load(dict_file) for dict_file in files]
    map(lambda x: x.close(), files)
    self.dictionary = {}
    self.max_key_size = 0
    for curr_dict in dictionaries:
        for key in curr_dict:
            #print(key)
            if key in self.dictionary:
                self.dictionary[key].extend(curr_dict[key])
            else:
                self.dictionary[key] = curr_dict[key]
                self.max_key_size = max(self.max_key_size, len(key))

def tag(self, postagged_sentences):
    return [self.tag_sentence(sentence) for sentence in postagged_sentences]

def tag_sentence(self, sentence, tag_with_lemmas=False):
    """
    the result is only one tagging of all the possible ones.
    :type sentence: object
    The resulting tagging is determined by these two priority rules:
        - longest matches have higher priority
        - search is made from left to right
    """
    tag_sentence = []
    n = len(sentence)
    if self.max_key_size == 0:
        self.max_key_size = n
    i = 0
    while i < n:
        j = min(i + self.max_key_size, n)  #avoid overflow
        tagged = False
        while j > i:
            expression_form = ' '.join([word[0] for word in sentence[i:j]]).lower()
            expression_lemma = ' '.join([word[1] for word in sentence[i:j]]).lower()
            if tag_with_lemmas:
                literal = expression_lemma
            else:
                literal = expression_form
            if literal in self.dictionary:
                #print(self.dictionary)
                is_single_token = j - i == 1
                original_position = i
                i = j
                taggings = [tag for tag in self.dictionary[literal]]
                tagged_expression = (expression_form, expression_lemma, taggings)
                if is_single_token:  #if the tagged literal is a single token, conserve its previous taggings:
                    original_token_tagging = sentence[original_position][2]
                    tagged_expression[2].extend(original_token_tagging)
                tag_sentence.append(tagged_expression)
                tagged = True
            else:
                j -= 1
        if not tagged:
            #print(literal)
            tag_sentence.append(sentence[i])
            i += 1

    return tag_sentence

So far all I can see being called is the __init__ method, so I thought I would focus on that. It feels weird, so I wanted to first explain why it felt that way, and then address your questions a little.

Readability first

I think the core reason is that this code feels weird is that it appears to use list comprehensions "just because". It creates a temporary list called files; it creates a temporary list called dictionaries; it closes all the already-opened files, and then it iterates over all the dictionaries. This feels weird because these temporaries serve no obvious purpose. Instead the code could easily set its invariants (initializing self.dictionary and self.max_key_size) and then do a simple for loop over thedictionary_paths:

def __init__(self, dictionary_paths)
    self.dictionary = {}
    self.max_key_size = 0
    for path in dictionary_paths:
        dict_file = open(path, 'r')
        curr_dict = yaml.load(dict_file)
        dict_file.close()
        for key in curr_dict:
            : : :

This may have some performance benefits, but they are likely to be too small to measure. Namely it puts less file-based pressure on the system as it only opens a single file at a time. Similarly there's somewhat less memory pressure, as only one curr_dict is maintained at a time. One downside is if initialization should fail if it can't open one of the dictionary_paths, that now happens after it does more work. I'm fine with that trade-off.

This change also gives you the opportunity to use modern approaches for closing files. For example, the open/load/close could instead use with to automatically close at the end of the block:

    with open(path, 'r') as dict_file:
        curr_dict = yaml.load(dict_file)

All in all, this advice so far makes things slightly easier to read. There are fewer moving pieces to keep in your head at the same time, and the ones that are there, are ones that we are used to tracking.

Performance concerns

Once the code is easy to read, it becomes easier to consider performance. If you're used to optimizing a compiled language, you'll find that optimizing python can be counter-intuitive. Where in a compiled language it's sometimes helpful to do a larger number of smaller operations (building manual caches, doing shifts and additions to avoid a multiplication), in python it's almost always the reverse: the fewer operations the better, even if they each take longer. What's in common? Always profile. I don't have your data, so I'm going to shoot from the hip here and offer some general ideas. It's up to you to profile them and see if any of the ideas are useful.

Loops are a good first guess, and the loop of __init__ is almost just a conditional. At first it might seem like there's nowhere to go with this, but there are actually three main things I would test to see if they are worth changing. First, the code traverses the internals of curr_dict multiple times; once is in the for loop, and once is in the lookup curr_dict[key]. Do these together by using dict.items (or dict.iteritems in python 2.x if curr_dict might be large):

for key, value in curr_dict.items():         # or .iteritems() in 2.x
    if key in self.dictionary:
        self.dictionary[key].extend(value)
    else:
        self.dictionary[key] = value
        self.max_key_size = max(self.max_key_size, len(key))

Second, to make the if and else case more similar, pull the max_key_size calculation out of the conditional:

for key, value in curr_dict.items():         # or .iteritems() in 2.x
    if key in self.dictionary:
        self.dictionary[key].extend(value)
    else:
        self.dictionary[key] = value
    self.max_key_size = max(self.max_key_size, len(key))

Or better, per what I mentioned above, let's call it once outside of the loop, having it do more work the single time we call it:

for key, value in curr_dict.items():         # or .iteritems() in 2.x
    if key in self.dictionary:
        self.dictionary[key].extend(value)
    else:
        self.dictionary[key] = value
self.max_key_size = max(len(key) for key in self.dictionary)

Third, back to the conditional. Let's examine two ways to collapse this all into a single line of code. One way is to use dict.setdefault. It reads a little funny, but it does exactly the above in one line:

self.dictionary.setdefault(key, []).extend(value)

The upside is that this only traverses the dictionary internals once. What's the downside? Even if you don't need it, this creates an empty list. That's unlikely to be a big performance concern, but if it is, the second way avoids this. A specialized dictionary type, collections.defaultdict lets you choose a "factory" for missing items. So selecting list as that factory will make self.dictionary do something similar to that setdefault line on its own:

self.dictionary = collections.defaultdict(list)
: : :
    self.dictionary[key].extend(value) # creates and extend empty list if key not present

Of course there's a potential downside to this approach too: this dictionary will create empty lists rather than throwing exceptions if someone tries to query a missing key. If that's a problem, you can always convert it back to a regular dictionary later. For convenience, here's all of that together:

def __init__(self, dictionary_paths)
    self.dictionary = collections.defaultdict(list)

    for path in dictionary_paths:
        with open(path, 'r') as dict_file:
            curr_dict = yaml.load(dict_file)
        for key, value in curr_dict.items():
            self.dictionary[key].extend(value)

    # optionally: self.dictionary = dict(self.dictionary)
    self.max_key_size = max(len(key) for key in self.dictionary)

Note also at this point it may be simpler to combine the with and for blocks, as the name curr_dict exists solely to let you close a single dict_file a little earlier. That's much less important now that it's only a single file, and perhaps having one less name and one less line of code is worth keeping the file open a little longer.

What about your other ideas?

First I'd make the loading as fast as it can be without having to resort to ugly tricks. The next steps depend a lot on initialization patterns you don't show here.

Do you instantiate a lot of DictionaryTagger instances with the same set of dictionary_paths? If so, it may be worth caching the dictionary that is created for a set of paths, and just copying (or referencing) that dictionary in later instances.
Do you instantiate a lot of DictionaryTagger instances with different combinations of a few underlying dictionary files? If so, it may be worth caching the dictionary created for each file, and merging them instead of loading the yaml file from scratch.
Do you instantiate just a single DictionaryTagger or not share any dictionary files? If so, there's nothing I can see worth trying to share.
Can you reuse a DictionaryTagger multiple times? If so, and if it's expensive enough to create it, by all means cache and share the instance. If doing so adds more than a tiny bit of complexity to your code, try profiling to be certain that the benefit will be worth it. At the very least, do a simple test with timeit to verify that class initialization is the problem.

Finally, before you even consider a performance rewrite, write some unit tests. That's even a good idea before refactoring like I showed above. I nearly missed the difference between append and extend, and thought your code had a bug in the if/else condition. Had I refactored that while misunderstanding it, I would have introduced a bug. Unit tests can help you avoid such errors, or help you find others.

Other comments

I didn't read your other two functions very closely, but I did notice a comment in tag_sentence that I wanted to call out:

   j = min(i + self.max_key_size, n)  #avoid overflow

Since your code uses j in a slicing operation, this comment suggests a mindset I would expect in a different language. Slicing operations automatically limit to the bounds of the container. For example, the following three slices give the same result:

>>> hello = "world"
>>> hello[3:5]
'ld'
>>> hello[3:10]
'ld'
>>> hello[3:]
'ld'

Then again, your code does math on the value of j, so letting it start too large could result in extra iterations. Thus the min may be necessary. Or there may be a way to start with the expressions built from the whole sentence, then remove the last word each time through the loop until it's empty. When you can find algorithms in python that don't involve index arithmetic, you'll often be happier with the results.

Correspondingly it's usually a good idea to prefer constructs that look like

for i in range(1, n):
    : : :

over ones that look like

while i < n:
    : : :
    i += 1

however your actual case has some extra conditionals in there that might not allow this conversion, so this advice may not apply here.