Concatenating rows in CSV files and removing old CSV files I have written my first Python script that will go into live use at my workplace. I am relatively new to Python and I think this script is pretty good but I would like your input.

It runs very fast, as far as I can tell (~0.01seconds). It is also my first time writing a script that takes arguments from the way it is called, and these arguments are set in the .bat file it is ran from.

import os
import time
from itertools import izip_longest
import fnmatch
import datetime
from datetime import timedelta, datetime
import shutil
import time
from sys import argv

date, raw_dir, today_dir, archive_dir, error_file = argv[1:]

def merge(raw_dir, today_dir, archive_dir, date):
    """Read through all .CSVs in RAW directory and merge rows into Today's "full" CSV """
    archive_file = archive_dir + date + '.csv'
    with open(today_dir + 'TODAY.CSV', 'w') as f_obj:
        rows = []
        files = os.listdir(raw_dir)
        for fi in files:
            if fnmatch.fnmatch(fi, '*.csv') and not fnmatch.fnmatch(fi, 'TODAY.CSV'):
                 rows.append(open(os.path.join(raw_dir, fi)).readlines())
        iter = izip_longest(*rows)
        for row in iter:
            f_obj.write('   '.join([field.strip() for field in row if field is not None]) + '\n')
        shutil.copyfile(today_dir + 'TODAY.CSV', archive_dir + date + '.csv')

def clear(raw_dir):
    """ remove any raw csv files that are older than 14 business days. Data itself
    is not lost because it has been archived in the full csv"""
    files = os.listdir(raw_dir)
    file_paths = []
    file_ints = []
    for f in files:
        file_paths.append(os.path.join(raw_dir, f))
    if len(file_paths) > 14:
        for f in files:
            file_ints.append(int(f[:8])) #converts file names to ints from datenumber
        del_file = raw_dir + str(min(file_ints)) + '.csv' #makes a valid file path from     of the lowest value datenumber
        os.remove(del_file) # deletes lowest value
        print del_file + 'was deleted in clean up'
    else:
        return

I've left out the part with the functions actually being called as it's just simple if __name__ == "__main__" and some try/except

Clear

List Comprehension

It's an unneeded habit in Python to pre-declare your variables.  Instead of

myList = []
for x in otherList:
    myList.append(foo(x))

you can use list comprehension,

myList = [foo(x) for x in otherList]
# identical: myList = map(foo, otherList)

Clarity

for f in files:
    file_ints.append(int(f[:8])) #converts file names to ints from datenumber

The comment explains that f[:8] returns the date number of the file but how that happens is a mystery to me.  Why don't you just use os.path.getctime?

file_ints = [os.path.getctime(f) for f in files]

This next bit is confusing.

# makes a valid file path from of the lowest value datenumber
del_file = raw_dir + str(min(file_ints)) + '.csv'

If only makes sense if your files' naming convention is based on their ctime.  You aren't defining that naming convention here.  Don't rely on external rules. Your file_ints line up with your file_paths so just use

del_file = file_paths(min(file_ints))

You might consider maintaining the alignment by zipping the two together like this

path_mtime = zip(file_paths, file_ints)

Assumptions

Do you get the files daily?  Is the script guaranteed to be run daily?  If both of these are ever false then your clearing will be out of sync.  You are not deleting every file older than 14 days, you are deleting the oldest file if there are more than 14.  It could be 15 days old, it could be 3 day's old.

Instead, iterate over all files and delete anything older than 14 days.

for path, ctime in zip(file_paths, file_ints):
    if is_too_old(ctime): # implement externally.  use time module to get today.
       os.remove(path)

You are also assuming all files in this directory are .csv.  I don't know the nature of the directory but it could be possible that some jerk comes and drops a .txt or .xls in your directory.  os.listDir will also return any subfolders that you might have.  You might not have them but if you did you surely don't want to remove them. Using glob might be the safer bet.  It will also return the full path so you don't need the intermediate files variable.

file_paths = glob.glob(os.path.join(raw_dir, "*.csv"))

This is optional though. It might be more than fair to assume what is or isn't in your folder.

Results

import glob

def clear(raw_dir):
    """ remove any raw csv files that are older than 14 business days. Data itself
    is not lost because it has been archived in the full csv"""
    file_paths = glob.glob(os.path.join(raw_dir, "*.csv"))
    # Optional early exit assuming you get no less than 14 files a day.
    # if len(file_paths ) <= 14:
    #     return
    file_ints = [os.path.getctime(f) for f in file_paths]
    for path, ctime in zip(file_paths, file_ints):
        if is_too_old(ctime): # implement externally.  Use time module to get today.
           os.remove(path)
           print path + ' was deleted in clean up'

Merge

You declare this variable, and then never use it,

archive_file = archive_dir + date + '.csv'

the same value is used at the end,

shutil.copyfile(today_dir + 'TODAY.CSV', archive_dir + date + '.csv')

but I say drop the declaration and leave the end as is, except please use os.path.join

Order

The main issue with this procedure is the order you perform events.  Currently, it is

open output file
read input files
write output file.

It should be,

read input files
open output file
write output file.

reading before you create the output file will simplify the read to,

rows = []
files = os.listdir(raw_dir)
for fi in files:
    if fnmatch.fnmatch(fi, '*.csv'):
         rows.append(open(os.path.join(raw_dir, fi)).readlines())

using glob will make it simpler,

rows = \
    [open(fi).readlines() for fi in glob.glob(os.path.join(raw_dir, "*.csv")]

Clarity

I have no idea why you are transposing your rows.

iter = izip_longest(*rows)

Each row is the list of lines of a file so

rows = [[line1_file1, line2_file1, ...], [line1_file2, line2_file2, ...], ...]

then you transpose that, so you get

iter = [[line1_file1, line1_file2, ...], [line2_file1, line2_file2, ...]]

So then each line of your output is a join of the non-null lines of your files.
Are you sure that's what you want?  I could help you more but I don't understand the process you are going through.  Your output file looks like it could be

line1_file1 line1_file2 line1_file3
line2_file1 line2_file2 line2_file3
line3_file1 line3_file3
line4_file3

Point is it's confusing and there should be some explanation.

Results

def merge(raw_dir, today_dir, archive_dir, date):
    """Read through all .CSVs in RAW directory and merge rows into Today's "full" CSV
    """
    rows = \
        [open(fi).readlines() for fi in glob.glob(os.path.join(raw_dir, "*.csv")]
    merged_file = os.path.join(today_dir, 'TODAY.CSV')
    archive_file = os.path.join(archive_dir, date + '.csv')
    with open(merged_file, 'w') as f_obj:
        # whatever is going on here...
        for row in izip_longest(*rows):
            f_obj.write('   '.join([field.strip() for field in row if field is not None]) + '\n')
        shutil.copyfile(merged_file, archive_file)


Spree Billing/Shipping Address Checkbox I used JS to re-populate the shipping address into the billing address when a checkbox is clicked. I'm wondering whether there is a more concise/better way to write the code below:

var checkBox = document.getElementById('order_use_billing');

var shipFirstName = document.getElementById('order_ship_address_attributes_firstname');
var billFirstName = document.getElementById('order_bill_address_attributes_firstname');
var shipLastName = document.getElementById('order_ship_address_attributes_lastname');
var billLastName = document.getElementById('order_bill_address_attributes_lastname');
var shipStreet = document.getElementById('order_ship_address_attributes_address1');
var billStreet = document.getElementById('order_bill_address_attributes_address1');
var shipStreetTwo = document.getElementById('order_ship_address_attributes_address2');
var billStreetTwo = document.getElementById('order_bill_address_attributes_address2');
var shipState = document.getElementById('order_ship_address_attributes_state_id');
var billState = document.getElementById('order_bill_address_attributes_state_id');
var shipCity = document.getElementById('order_ship_address_attributes_city');
var billCity = document.getElementById('order_bill_address_attributes_city');
var shipCountry = document.getElementById('order_ship_address_attributes_country_id');
var billCountry = document.getElementById('order_bill_address_attributes_country_id');
var shipZip = document.getElementById('order_ship_address_attributes_zipcode');
var billZip = document.getElementById('order_bill_address_attributes_zipcode');
var shipPhone = document.getElementById('order_ship_address_attributes_phone');
var billPhone = document.getElementById('order_bill_address_attributes_phone');

$(function(){
   $('input[type=checkbox]').prop("checked", false);
});

function filling(){

  if(checkBox.checked == true) {
    shipFirstName.value       = billFirstName.value;
    shipLastName.value        = billLastName.value
    shipStreet.value          = billStreet.value
    shipStreetTwo.value       = billStreetTwo.value
    shipCity.value            = billCity.value
    shipState.value           = billState.value
    shipCountry.value         = billCountry.value
    shipZip.value             = billZip.value
    shipPhone.value           = billPhone.value

  }
   if(checkBox.checked == false) {
    shipFirstName.value       = "";
    shipLastName.value        = "";
    shipStreet.value          = "";
    shipStreetTwo.value       = "";
    shipCity.value            = "";
    shipState.value           = "";
    shipCountry.value         = "";
    shipZip.value             = "";
    shipPhone.value           = "";

  }
};

$(function() {
    $('input[type="checkbox"]').on('change', function() {
         filling();
    });
});

My approach for something like this would be to have the two (sub-)forms wrapped in a grouping element such as a fieldset and just loop over all fields in one and match them with the other. Quick incomplete, untested example:

<form>
  <fieldset id="shipping-address">
    <input name="shipping-firstname">
    <input name="shipping-lastname">
  </fieldset>
  <fieldset id="billing-address">
    <input name="billing-firstname">
    <input name="billing-lastname">
  </fieldset>
</form>

<script>
  $("shipping-address input").each(function() {
    var shippingInput = $(this);
    var billingInputName = "billing" + shippingInput.attr("name").slice(8);
    $("#billing-address [name=" + billingInputName + "]").val(shippingInput.val());
  });
</script>


Python: Combing two programs that analyze strings I am trying to combine two programs that analyze strings into one program. The result should look at a parent string and determine if a sub-string has an exact match or a "close" match within the parent string; if it does it returns the location of this match

i.e. parent: abcdefefefef
sub cde is exact and returns 2
sub efe is exact and returns 4, 6, 8
sub deg is close and returns 3
sub z has no match and returns None

The first program I wrote locates exact matches of a sub-string within a parent string:

import string

def subStringMatchExact():
    print "This program will index the locations a given sequence"
    print "occurs within a larger sequence"
    seq = raw_input("Please input a sequence to search within: ")
    sub = raw_input("Please input a sequence to search for: ")
    n = 0
        for i in range(0,len(seq)):
        x = string.find(seq, sub, n, len(seq))
        if x == -1:
            break
        print x
        n = x + 1

The second analyzes whether or not there is an close match within a parent string and returns True if there is, and false if there isn't. Again a close match is an exact match or one that differs by only one character.

import string

def similarstrings():
    print "This program will determine whether two strings differ"
    print "by more than one character. It will return True when they"
    print "are the same or differ by one character; otherwise it will"
    print "return False"
    str1 = raw_input("Enter first string:")
    str2 = raw_input("Enter second string:")
    x = 0
    for i in range(len(str1)):
        if str1[i] == str2[i]:
            x = x + 1
        else:
            x = x
    if x >= len(str1) - 1:
        print True
    else:
        print False

Any suggestions on how to combine the two would be much appreciated.

Firstly, a quick review of what you've done:

import string is unnecessary really. You utilize string.find, but you can simply call the find method on your first argument, that is, seq.find(sub, n, len(seq)).

In your second method, you have:

if str1[i] == str2[i]:
    x = x + 1
else:
    x = x

The else statement is completely redundant here, you don't need it.

I think we can simplify the logic quite a bit here. I'm going to ignore raw_input and take the strings as arguments instead, but it doesn't change the logic (and is arguably better style anyway). Note that this is not fantastically optimized, it can definitely be made faster:

def similar_strings(s1, s2):
    '''Fuzzy matches s1 with s2, where fuzzy match is defined as
    either all of s2 being contained somewhere within s1, or
    a difference of only one character. Note if len(s2) == 1, then
    the character represented by s2 must be found within s1 for a match
    to occur.

    Returns a list with the starting indices of all matches.

    '''
    block_size = len(s2)
    match = []
    for j in range(0, len(s1) - block_size):
        diff = [ord(i) - ord(j) for (i, j) in zip(s1[j:+block_size],s2)]
        if diff.count(0) and diff.count(0) >= (block_size - 1):
            match.append(j)
    if not match:
        return None
    return match

How does this work? Well, it basically creates a "window" of size len(s2) which shifts along the original string (s1). Within this window, we take a slice of our original string (s1[j:j+block_size]) which is of length len(s2). zip creates a list of tuples, so, for example, zip(['a','b','c'], ['x','y','z']) will give us back [(a, x), (b, y), (c, z)].

Then, we make a difference of all the characters from our s2 sized block of s1 and our actual s2 - ord gives us the (integer) value of a character. So the rather tricky line diff = [ord(i) - ord(j) for (i, j) in zip(s1[j:+block_size],s2)] will give us a list of size len(s2) which has all the differences between characters. As an example, with s1 being abcdefefefef and s2 being cde, our first run through will give us the slice abc to compare with cde - which gives us [-2, -2, -2] since all characters are a distance of 2 apart.

So, given diff, we want to see how many 0's it has, as two characters will be the same when their ord difference is 0. However, we need to make sure there is at least 1 character the same in our block (if you simply pass in z, or any string length 1, diff will always have length 1, and block_size - 1 will be 0. Hence diff.count(0) >= 0 will always be True, which is not what we want). Thus diff.count(0) will only be true if it is greater than 0, which protects us from this. When that's true, look at the block size, make sure it is at least len(s2) - 1, and if so, add that index to match.

Hopefully this gives you an idea of how you'd do this, and I hope the explanation is detailed enough to make sense of some python constructs you may not have seen before.


Finding the next palindrome code I have been trying recently to develop a code finding the next palindrome from a certain number. For example, if the input is 800, the output should be 808, as 808 being the next palindrome from 800.

Here's my code. It's fully functional, but on my online judge I get a problem with the time of the execution. My method is not optimal. Any advice on this?

def reverseNumber(a):
   degreMax = findDegree(a)
   result = 0
   while a != 0:
      r = a % 10
      a = a // 10
      result = result + r*10**degreMax
      degreMax = degreMax - 1
   return result

def findDegree(a):
   degre = 0
   while a >= 10:
     a = a/10
     degre = degre + 1
   return degre

def isPalindrome(a):
   test = False
   if reverseNumber(a) == a:
      test = True
   return test

def nextPalindrome(a):
   while isPalindrome(a) == False:
      a = a + 1
   return a

nbTest = int(raw_input())
inputs = []

for i in range(0,nbTest):
   a = int(raw_input())
   inputs.append(a)

for i in range(0,len(inputs)):
   print nextPalindrome(inputs[i])

A bit of style

Before anything, let's rewrite/reorganise the code a bit to make it more pythonic but also easier to test.

Python has a style guide called PEP8. Among other things, you do not follow the naming and spacing conventions. You might find interesting to know that there are various tools to check your code (against PEP8 but also for various other things : pep8, pylint, pychecker, pyflakes and even tools to fix style automatically like autopep8.

Also, it is usual (and very useful) to use an if __name__ == '__main__': guard so that you have your definitions (classes, functions, constants, etc) on one hand and your actual program doing something on the other hand.

So far, your "improved" code looks like :

def reverse_number(a):
    degreMax = find_degree(a)
    result = 0
    while a != 0:
        r = a % 10
        a = a // 10
        result = result + r*10**degreMax
        degreMax = degreMax - 1
    return result

def find_degree(a):
    degre = 0
    while a >= 10:
        a = a/10
        degre = degre + 1
    return degre

def is_palindrome(a):
    test = False
    if reverse_number(a) == a:
        test = True
    return test

def next_palindrome(a):
    while not is_palindrome(a):
        a = a + 1
    return a

if __name__ == '__main__':
    nb_test = int(raw_input())
    inputs = []

    for i in range(0, nb_test):
        a = int(raw_input())
        inputs.append(a)
    # inputs = [5, 10, 303, 800]

    for i in range(0, len(inputs)):
        print next_palindrome(inputs[i])

So far, I haven't done anything relevant as far as performances are concerned and you might think I'm wasting your time here. I'm almost done.

More style

So far, I've done cosmetic changes. A few other changes can be performed to make your code more beautiful but also potentially faster (even if it is very subtle).

Python has a divmod function which does exactly what you are trying to do in reverse_number.
You don't need a temporary variable in is_palindrome.
You don't need to provide first argument to range if you start at 0 and step is 1.
You don't need temporary variable a in your main.
In Python 2 (which is the version I am guessing you are using), range creates a list which might be more than what you want. If you just need something to iterate on, xrange is what you need (in most cases this is what you need and it's why it is the behavior for range in Python 3).
It is the convention to name _ a variable whose variable is not used such as i in your code initialising the input list.
Whenever you are using range and len to iterate over a container, you are most probably doing something wrong. Python provides you a way to iterate over something that can hardly be easier :

for i in inputs:
    print next_palindrome(i)

It's easy/concise/hard to get wrong/fast.

to create a list and fill it, instead of having a boring loop and append, Python has a very expressive way to do it : list comprehension. If you have never read about them, make yourself a favor and just google it. In your case, defining your inputs gets down to this :

    inputs = [int(raw_input()) for _ in xrange(nb_test)]

you can use division assignment /= to write a /= 10 instead of a = a/10. Also (and this is more common), you can use += and -= in various places of your code.
you don't even need to store inputs into an array, you can just compute and print answers as you do over inputs.

At this stage, your code looks like :

def reverse_number(a):
    degreMax = find_degree(a)
    result = 0
    while a != 0:
        a, r = divmod(a, 10)
        result = result + r*10**degreMax
        degreMax -= 1
    return result

def find_degree(a):
    degre = 0
    while a >= 10:
        a /= 10
        degre += 1
    return degre

def is_palindrome(a):
    return reverse_number(a) == a

def next_palindrome(a):
    while not is_palindrome(a):
        a += 1
    return a

if __name__ == '__main__':
    nb_test = int(raw_input())
    inputs = [int(raw_input()) for _ in xrange(nb_test)]
    # inputs = [5, 10, 303, 800]

    for i in inputs:
        print next_palindrome(i)

User interface

A last note before diving into performances. It might be convenient to tell the user what you are asking him. Otherwise, one might think your program is just computing stuff when it is just waiting for input.

nb_test = int(raw_input("Nb tests? "))
inputs = [int(raw_input("Test ? ")) for _ in xrange(nb_test)]

Mesuring performances

In order to improve performance, it is best to be able to measure it while ensuring correctness. This can be achieved through unit tests. Without being bothered with a full unit testing framework, you can just write things like :

def unit_tests():
    assert next_palindrome(5) == 5
    assert next_palindrome(10) == 11
    assert next_palindrome(80) == 88
    assert next_palindrome(12345678) == 12355321
    assert next_palindrome(1234567890) == 1234664321
    assert next_palindrome(12345678901) == 12345754321
    # assert next_palindrome(123456789012) == ?? too slow..

As your code probably gets too slow for bigger inputs, it's a good option to add a few tests with huge inputs.

Improving performance

Finally!!

You'll probably be disappointed to see that most of the logic you've written can be rewritten using standard functions (this has been said by others):

def is_palindrome(a):
    s = str(a)
    return s == ''.join(reversed(s))

But for a more dramatic improvement, we'll have to look for a totally different algorithm, one that will be very efficient for very huge values. This can be done if you spot a pattern in the different solutions for the different tests we have written.

Let's consider a number with n digits : $$d1,d2,...,dn$$ and let's try to split it in the middle :

if n is odd, we have $$n = 2*k+1$$ and we have 3 parts : $$d1,d2,..,dk$$ in the piece of the left, $$dk+1$$ in the middle, $$dk+2..dn$$ in the piece on the right. Based on the comparison of the left part and of the right part, the next palindrom the left part and its reversed with either dk or dk+1 in the middle.
if n is even, the same logic applies without a middle component.

A few edge cases are to be considered but the following code works on basic cases :

def next_palindrome(a):
    s = str(a)
    n = len(s)
    k, r = divmod(n, 2)
    if r:
        left, mid, right = s[:k], s[k], s[k+1:]
        assert len(left) == len(right) == k
        assert s == left + mid + right
        print(s, left, mid, right)
        mid_n = int(mid)
        if left:
            left_n, right_n, right_n_rev = int(left), int(right), int(right[::-1])
            if left_n > right_n and left_n > right_n_rev:
                return int(left + mid + left[::-1])
            else:
                return int(left + str(mid_n+1) + left[::-1])
        else:
            return mid_n # 1-digit number
    else:
        left, right = s[:k], s[k:]
        assert len(left) == len(right) == k
        assert s == left + right
        print(s, left, right)
        left_n, right_n, right_n_rev = int(left), int(right), int(right[::-1])
        if left_n > right_n and left_n > right_n_rev:
            return int(left + left[::-1])
        else:
            new_left = str(left_n + 1)
            return int(new_left + new_left[::-1])

I've tried (unsucessfully) to give meaningful names to variables.


Inch  cm conversion program I just wrote this small inch <-> cm conversion program. It works just fine, although I know that I've done some things in a rather stupid manner and that it could be improved.

def conversion():
    try: amount = int(raw_input("Please enter the value: "))
    except ValueError:
        print "Please specify a valid amount."
        return conversion()
    answer = raw_input("Please choose between converting FROM kilograms/pounds: ")
    if answer == "kilograms":
        return amount * 2.2
    elif answer == "pounds":
        return amount / 1.45
    else:
        print "Please choose between kilograms and pounds."
        restart = raw_input("Try again? ")
        if restart == "yes":
            return conversion()
        elif restart == "y":
            return conversion()
        else:
            print "Okay, bye."
            return

print conversion()

The first point would be the separation of concerns: every entity (function, class …) should only be responsible for one task (except, ironically, printing the result).

In your code, one function (conversion) is responsible for everything. Try separating the different issues:

user input
conversion
output

Granted, for such a very small program the result will almost certainly be larger. But it will also be much clearer and easier to extend.

Next up, your use of recursion. I love recursion as much (more!) as the next programmer but in your case a few loops wouldn’t hurt readability.

A third point, magic constants should be avoided. Your use of the conversion factors 2.2 and 1.45 is innocuous enough but it soon becomes a problem in larger programs. Use properly-named constants instead.


Reducing a list of points into a list of pairs of points I have some data…

let points = [(718, 620), (4596, 1280), (410, 333), (4597, 993),
(410, 337), (4597, 996), (428, 337), (4597, 1000), (431, 335), (4599, 1044)]

… that follows the pattern

[(pointStartOne.x, pointStartOne.y), (pointEndOne.x, pointEndOne.y),
(pointStartTwo.x, pointStartTwo.x) // ...

I need to structure it to

typealias PointPair = [(start: Point, end: Point)]
struct Point {
    let x: Int
    let y: Int
}

Here is my code:

var result: PointPair = []
while i < points.count{
    result.append((Point(x: points[i].0, y: points[i].1), Point(x: points[i+1].0, y: points[i+1].1)))
    i+=2
}

I think maybe it is nice situation to use functional programming reduce

Here is code ituitive:

let result = zip((points.enumerated().filter({ (offset: Int, element: (Int, Int)) -> Bool in
        return offset%2==0
    })), (points.enumerated().filter({ (offset: Int, element: (Int, Int)) -> Bool in
        return offset%2==1
    }))).reduce(PointPair(), { (result: PointPair, arg) -> PointPair in
        let (one, other) = arg
        let (offset, point) = one
        let (offsetTwo, otherPoint) = other
        return result + [(Point(x: point.0,y: point.1), Point(x: otherPoint.0,y: otherPoint.1))]
    })

It is messy. Trying to use functional programming conveniently and elegantly.

Here is the improved code:

result = points.enumerated().reduce(into: PointPair(), {(temp, arg) in
        let (offset, point) = arg
        if offset % 2 == 0{
            temp += [(Point(x: point.0, y: point.1), Point(x: 0, y: 0) )]
        }
        else if let last = temp.last {
           temp[temp.count - 1] = (last.0, Point(x: point.0, y: point.1))
        }
    })

Let's start with the first version: A loop

var i = 0
while i < points.count {
    // ...
    i += 2
}

can be written as a for-loop over a stride:

for i in stride(from: 0, to: points.count, by: 2) {
    // ...
}

which restricts the scope of i to the loop body, and makes it a constant. Now each loop iteration appends exactly one element to var result: PointPair, which means that we can write this as a map operation:

let result: PointPair = stride(from: 0, to: points.count, by: 2)
    .map { i in
        (start: Point(x: points[i].0, y: points[i].1),
         end: Point(x: points[i+1].0, y: points[i+1].1))
    }

This may already be the “functional” version that you are looking for, but let's also have a look at your other two variants.

The “intuitive code” creates two additional arrays by filtering the original one for even/odd indices. After filtering the arrays the offsets are not needed, giving the warnings

Immutable value 'offset' was never used; consider replacing with '_' or removing it
Immutable value 'offsetTwo' was never used; consider replacing with '_' or removing it

This can be fixed with

let (_, point) = one
let (_, otherPoint) = other

A better way is to use compactMap() (which can be thought of as a combination of filter() and map()) to create arrays of the points with even/odd indices, without the element offsets.

Using reduce() for creating the result array is inefficient, as it creates intermediate arrays for each iteration. reduce(into:_:) would be an improvement, but actually this is just a map():

let result: PointPair = zip(
    points.enumerated().compactMap { (offset, element) -> Point? in
        offset % 2 == 0 ? Point(x: element.0, y: element.1) : nil },
    points.enumerated().compactMap { (offset, element) -> Point? in
        offset % 2 == 1 ? Point(x: element.0, y: element.1) : nil }
).map { ($0, $1) }

This looks a lot “less messy” than your original version, but still has the disadvantage of creating intermediate arrays.

I have not much to say about your last version. It uses reduce(into:) which is good to avoid the creation of intermediate arrays. The only disadvantage is that temporary pairs are created and modified in every second step.

So the  “stride + map” variant looks like the best to me: It is efficient and easy to read.

A final remark: The type alias

typealias PointPair = [(start: Point, end: Point)]

is misleading, because it does not define a pair of points but an array of pairs of points. I would probably define a dedicated type for a pair of points instead

struct Line {
    let from: Point
    let to: Point
}

and then use [Line]  as the result type.


Storing disassembled data in a structured way I want to store the information returned by the dis function of the dis module in a structured way, using a dict, associating the mnemonics of each code of a line to the correspondent line number.

Note: Instruction is a subclass of _Instruction, defined in the dis module as a namedtuple.

Output:

disassembled_dict =
{
 147: [ Instruction(opname='LOAD_FAST', opcode=124, arg=0, arg1=0, arg2=0, argval='x', argrepr='x', offset=0, starts_line=147, is_jump_target=False),
        Instruction(opname='LOAD_CONST', opcode=100, arg=1, arg1=1, arg2=0, argval=1, argrepr='1', offset=3, starts_line=None, is_jump_target=False),
        Instruction(opname='INPLACE_ADD', opcode=55, arg=None, arg1=None, arg2=None, argval=None, argrepr='', offset=6, starts_line=None, is_jump_target=False),
        Instruction(opname='STORE_FAST', opcode=125, arg=0, arg1=0, arg2=0, argval='x', argrepr='x', offset=7, starts_line=None, is_jump_target=False)
      ]

 148: [ Instruction(opname='LOAD_FAST', opcode=124, arg=0, arg1=0, arg2=0, argval='x', argrepr='x', offset=10, starts_line=148, is_jump_target=False),
        Instruction(opname='LOAD_CONST', opcode=100, arg=2, arg1=2, arg2=0, argval=2, argrepr='2', offset=13, starts_line=None, is_jump_target=False),
        Instruction(opname='BINARY_POWER', opcode=19, arg=None, arg1=None, arg2=None, argval=None, argrepr='', offset=16, starts_line=None, is_jump_target=False),
        Instruction(opname='RETURN_VALUE', opcode=83, arg=None, arg1=None, arg2=None, argval=None, argrepr='', offset=17, starts_line=None, is_jump_target=False)
      ]
}

Code:

import dis

def preatty_dis(function):
    fcode = function.__code__
    ## call dis._get_instructions_bytes just the original dis function do
    disassembled_raw = [instruction for instruction in
                        dis._get_instructions_bytes(fcode.co_code, fcode.co_varnames,
                                                    fcode.co_names, fcode.co_consts,
                                                    fcode.co_cellvars + fcode.co_freevars,
                                                    dict(dis.findlinestarts(fcode)))]
    iter_instructions = iter(disassembled_raw)
    disassembled_dict = {}
    line_pack = []
    while True:
        try:
            if not(line_pack):
                instruction = next(iter_instructions)
                line_pack.append(instruction)
            else:
                instruction = line_pack[0]

            if(instruction.starts_line):
                instruction = next(iter_instructions)
                while(instruction.starts_line is None):
                    line_pack.append(instruction)
                    instruction = next(iter_instructions)
                else:
                    ## line_pack[0] is the first mnemonic of the code line
                    ## line_pack[0].starts_line is the number of the code line
                    disassembled_dict.update({line_pack[0].starts_line : (line_pack)})
                    line_pack = [instruction]
            else:
                disassembled_dict.update({line_pack[0].starts_line : (line_pack)})
                line_pack = []
        except StopIteration:
            ## append the last group
            print(line_pack)
            disassembled_dict.update({line_pack[0].starts_line : (line_pack)})
            line_pack = []
            break
    return disassembled_dict

The code works on Python 3.5, but I'm sure there are many chances to make it more idiomatic, cleaner and more readable...

Suggestions?

This is a superfluous use of a list comprehension: [instruction for instruction in dis._get_instructions_bytes(…)].  You can just call dis._get_instructions_bytes() directly.

As indicated by the _ prefix, dis._get_instructions_bytes() is an undocumented and unsupported method.  If you choose to write code that relies on undocumented behaviour, you have a duty to point it out with an obvious apologetic comment.

I'm not convinced that you need to use an undocumented function, though.  dis.get_instructions() seems to work just fine.

The use of next() to iterate through disassembled_raw is awkward, and I'm sure that is what prompted you to post this question.  Fundamentally, what you want to achieve is a "group by" operation, so you want to use itertools.groupby().  The only tricky thing is that you want instruction.starts_line being None to mean that that instruction should be lumped with the previous batch.  For that, you could use a hack: make the key function stateful.

You seem to have a stray print() call at the end, left over from debugging.

Suggested solution

import dis
import itertools

def pretty_dis(function):
    last_seen_starts_line = None
    def stateful_starts_line(instruction):
        """Extract line number for the instruction, which is either
           instruction.starts_line if there is one, or the previously
           seen instruction.starts_line if there isn't."""
        nonlocal last_seen_starts_line
        last_seen_starts_line = instruction.starts_line or last_seen_starts_line
        return last_seen_starts_line

    return {
        line_num: list(instructions)
        for line_num, instructions in itertools.groupby(
            dis.get_instructions(function), key=stateful_starts_line
        )
    }


Finding the max error of a set of training data I wrote a neural net, and a Trainer class that automates training the net using a training set; a set of input, and the expected output value.

Originally, I was just returning currentError from train to be the highest error so far in the training routine. Obviously though, currentError isn't actually the worst error, and returning it lead to partially trained nets being marked as "trained" due to a single input giving a very good result.

To fix this, I realized that I needed to keep track of the worst error for all of the inputs; the net shouldn't be considered trained just because 1 input has a low error. To achieve this, I included in the class a Map between an input set, and its current error.

The problem with this approach is to find the worst error of all of the input sets, I need to iterate the entire set of values (the errors). I'm only training logic gates right now, so the training sets are very small, but doing a complete iteration of the errors would get very costly with higher numbers of training sets (the training routine currently does ~30,000 training passes per second).

Is there a better way of achieving this?

I'm only going to include the Training class, so it won't be runnable. The entire project is huge (for my standards; 14 classes), so it would be impractical to include it all here. It should also be fairly easy to infer the contents of the Trainer interface.

RandomOrderTrainer.java:

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Random;
import java.util.function.UnaryOperator;

import neuralNet.NeuralNet.BackpropAlgorithm;

public class RandomOrderTrainer
        implements Trainer<Double> {

    final private NeuralNet net;
    final private BackpropAlgorithm backProp;
    final private Map<List<Double>, List<Double>> answerMap = new HashMap<List<Double>, List<Double>>();
    final private Map<List<Double>, Double> errorsMap = new HashMap<List<Double>, Double>();
    final private Random randGen;
    final private UnaryOperator<Double> actF;
    final private UnaryOperator<Double> actDeriv;

    public RandomOrderTrainer(NeuralNet net, Random randGen, UnaryOperator<Double> actF, UnaryOperator<Double> actDeriv) {
        this.net = net;
        this.backProp = net.getBackpropagator();
        this.randGen = randGen;
        this.actF = actF;
        this.actDeriv = actDeriv;
    }

    @Override
    public void addPair(List<Double> input, List<Double> expectedOutput) {
        answerMap.put(input, expectedOutput);
    }

    private double sumErrors(Collection<Double> errors) {
        double sum = 0;
        for (Double error : errors) {
            sum += Math.abs(error);
        }

        return sum;
    }

    @Override
    public void train() {
        List<Double> randInput = randInputSet();
        List<Double> expectedOutput = answerMap.get(randInput);

        net.fireNet(randInput, actF);
        backProp.backpropagateError(expectedOutput, actDeriv);

        double currentError = sumErrors( backProp.calcOutputError(expectedOutput, actDeriv).values() );

        Double previousError = errorsMap.get(randInput);

        if (previousError == null || currentError < previousError) {
            errorsMap.put(randInput, currentError);
        }
    }

    @Override
    public Double getHighestError() {
        Double highestError = 0.0;

        for (Double error : errorsMap.values()) {
            if (error > highestError) {
                highestError = error;
            }
        }

        return highestError;
    }

    private List<Double> randInputSet() {
        ArrayList<List<Double>> keyList = new ArrayList<List<Double>>(answerMap.keySet());

        return Util.randomElement(keyList, randGen);
    }

}

Current approach
Right now, you train on randomly selected inputs from the set of all possible inputs and keep track of the highest error per input using a map.  There are a couple of things I don't like about this:

By randomly selecting inputs, you might fail to cover all inputs.  Then you might think the net is finished training when it actually might fail to handle an input that was never randomly picked.
Keeping track of the highest error per input is slow.

New approach - training in rounds
I suggest changing your approach in the following way:

When you train, you must train in "rounds".
At the beginning of each round, you shuffle the order of the possible inputs.
One round consists of training on each input one by one in the shuffled order.
During training, the highest error is tracked for the round.  This is now just a single variable instead of a map.
At the end of each round, training either stops if the highest error is good enough, or continues to the next round if the error is too high.


SASS/SCSS mixin for button states I have created several mixins for a button to attempt to reduce duplication and contribute to the overall improvement of performance. I am looking for guidance on how to tighten this up so it is as DRY (Don't Repeat Yourself) as possible. Or is it already as DRY as possible?

Here is the code:

// mixin - button shape
@mixin buttonShape {
    vertical-align:middle;
    line-height:1.333rem;height:2.166rem;
    color:#282a2e;
    border-radius:0;
    margin-top:1.083rem;
    margin-bottom:.333rem;
    margin-right:1.073rem;
    border:none;
    transition: background-color .2s;
}

// mixin - button gradients
@mixin gradientNormal {
    background-color:#efefee;
    background:linear-gradient(to bottom,#efefee 0,#d0d0ce 100%);
    box-shadow:inset 0 0 0 .083rem #fff;outline:1px solid silver;
}

@mixin gradientHoverActive {
    background:linear-gradient(to bottom,#ffffff 0,#ffffff 100%);
    box-shadow:inset 0 0 0 .083rem #fff;outline:2px solid #e87722;
}

// normal state
.ui-button {
    font-size: 12px;
    @include buttonShape;
    @include gradientNormal;
}

// hover
.ui-button:enabled:hover,
.ui-button:focus {
    @include buttonShape;
    @include gradientHoverActive;
}

// active
.ui-button:enabled:active {
    @include buttonShape;
    @include gradientHoverActive;
}

Formatting

Format your code better. Put a space after the colon after properties. Don't put multiple properties in one line.

Use of SASS

Put your colors and lengths into variables so you can reuse them at other places, when they are supposed to be identical.

Why are the numbers not rounded? There is probably no visible difference between 1.073rem and 1.083rem and they probably could even be rounded to 1.1rem. If you are calcualting the values from something, then use SASS to do the calculations.

CSS itself

The buttons don't have a left margin.

The font-size either also belongs into the mixin, or removed entirely.

The mixin buttonShape shouldn't be repeated in the :hover/:active/ :focus rules.

If the rules .ui-button:enabled:hover, .ui-button:focus and .ui-button:enabled:active are supposed to be identical, then join them into one rule.

Unless you are explicitly declaring rules for disabled buttons the addition of the :enabled pseudo-class does not make sense.

Design

From a design and usability stand point it's not a  great idea to have :hover, :focus and :active all look the same.


Mimicking Django filter function I really appreciate the way filtering is applied to queries in Django. I have a list of items (each item is an Item instance which contains a few attributes) that I would like to provide a similar API for filtering.

For reference, the Item class contains the following attributes:

Name
Description
Price
Stock

Currently, Item instances are stored in a list contained inside an Inventory instance. The end-goal is to have code like the following:

>>> inv = Inventory('some id')
>>> inv.load()
>>> inv.find(name='Tomato Paste')
[Item <Tomato Paste>]
>>> inv.find(name__startswith = 'T', name__contains='paste')
[Item <Tomato Paste>]
>>> inv.find(price__gt=200)
[Item <Some Expensive Item>, Item <Some Other Expensive Item>]

Here is an excerpt from my Inventory class with my attempt at accomplishing this:

def find(self, **kwargs):
    matches = []

    for item in self.data:
        match = 0
        for key in kwargs.keys():
            try:
                # Determine if this query has extra arguments
                if '__' in key:
                    arg = key.split('__')[1]
                    atrib = key.split('__')[0]
                    value = getattr(item, atrib)

                    if arg not in self.query_types:
                        continue

                    if arg == 'contains' and type(value) is str:
                        if kwargs[key].lower() in value.lower():
                            match += 1
                    elif arg == 'startswith' and type(value) is str:
                        if value.lower().startswith(kwargs[key].lower()):
                            match += 1
                    elif arg == 'endswith' and type(value) is str:
                        if value.lower().endswith(kwargs[key].lower()):
                            match += 1
                    elif arg == 'gt' and self._is_int(value):
                        if int(value) > int(kwargs[key]):
                            match += 1
                    elif arg == 'lt' and self._is_init(value):
                        if int(value) < int(kwargs[key]):
                            match += 1
                else:
                    value = getattr(item, key)
                    if type(kwargs[key]) is str:
                        if kwargs[key].lower() == value.lower():
                            match += 1
                    else:
                        if kwargs[key] == value:
                            match += 1
            except Exception:
                continue

        if match == len(kwargs.keys()):
            matches.append(item)
    return matches

A few notes:

self._is_init does what it implies
self.data is the list of Item instances

The above code does what I need it to, but it seems to have a serious boilerplate issue. I feel like there's a much more efficient and easier to understand solution. Optimization and feedback is welcomed.

Here's some improvements you can make and some (untested) code with those improvements:

Instead of checking if the argument key contains '__' then splitting it, just split it. If it didn't contain '__' the second part will be an empty string.
Instead of through all the items and each keyword argument each time, loop through the arguments and use list comprehensions to find the matches. This way the matches list is shrinking and you don't need to look at values that matched the previous arguments.
Instead of checking if the type of a value is string, use isinstance(obj, basestring). This will allow unicode strings also.
Instead of catching all exceptions, catch only the exceptions you expect. (others are actually errors and should be propagated to the caller).

...

def find(self, **kwargs):
    matches = self.data

    for key, key_value in kwargs.iteritems():
        # Determine if this query has extra arguments
        key_parts = key.split('__')
        attrib = key_parts[0]

        if (len(parts) > 1):
            arg = key_parts[1]
            if arg not in self.query_types:
                continue

            if arg == 'contains':
                matches = [ item for item in matches if isinstance(getattr(item,arg, None), basestring) and
                            ( key_value.lower() in getattr(item,arg, "").lower() ) ]
            elif arg == 'startswith':
                matches = [ item for item in matches if isinstance(getattr(item, arg, None), basestring) and
                            ( getattr(item,arg, "").lower().startswith(key_value.lower()) ) ]
            elif arg == 'endswith':
                matches = [ item for item in matches if isinstance(getattr(item, arg, None), basestring) and
                            ( getattr(item,arg, "").lower().endswith(key_value.lower()) ) ]
            elif arg == 'gt':
                matches = [ item for item in matches if self._is_int((getattr(item, arg, None)) and
                            ( int(getattr(item, arg, 0)) > int(key_value) ) ]
            elif arg == 'lt':
                matches = [ item for item in matches if self._is_int((getattr(item, arg, None)) and
                            ( int(getattr(item, arg, 0)) < int(key_value) ) ]
        else:
            attr_value = getattr(item, key)
            if isinstance(value, str):
                if key_value.lower() == attr_value.lower():
                    matches = [ item for item in matches if
                                ( isinstance(getattr(item,arg, None), str) and
                                    key_value.lower() == isinstance(getattr(item,arg,"")).lower() )
                                or ( key_value == isinstance(getattr(item,arg,None)) ) ]

    return matches


Python lexicon class First time around Code Review so please be gentle (and I am happy for comments on which angles of this post are a good fit and which less so). I'm not pretty much used to software engineering in Python, and so I figured this might be a match:

import pickle

class Lexicon:
    'a lexicon holding each term and providing some lexicon services like term frequency, and term ids'

    def __init__(self):
        self.dictt = {}

    def size(self):
        return len(self.dictt)

    def add(self, token):
        if token in self.dictt:
            self.dictt[token] = self.dictt[token] + 1
        else:
            self.dictt[token] = 1
            return token # for traceability by the caller

    def remove_low_frequency(self, min_frequency):
        'removes low frequency tokens'
        self.dictt = dict(filter(lambda kv: kv[1] >= min_frequency, self.dictt.items()))

    def pickle(self, output_file_name):
        with open(output_file_name, 'wb') as handle:
            pickle.dump(self.dictt, handle, protocol=pickle.HIGHEST_PROTOCOL)

    @staticmethod
    def unpickle(input_file_name):
        with open(input_file_name, 'rb') as handle:
            new_lexicon = Lexicon()
            new_lexicon.dictt = pickle.load(handle)
            return new_lexicon

    def equals(self, other):
        return self.dictt == other.dictt

Am I being non-idiomatic above, or, just cumbersome anywhere?
Does my unpickle method actually represent potential for a huge memory leak as I'm reusing the name but putting a different dictionary into it?
What might be the Pythonic idiom for a correct implementation in these cases?
And what is the Pythonic way of making this class safe for concurrent usage? (thread-safe)

First of all, the Lexicon class reinvents Counter from collections. Maybe subclassing can help.

Second, I'd separate persistence from the class because it's completely orthogonal to the Counter functionality (see below about shelve).

Pickle/unpickle does not represent any memory leaks. As soon as noone uses the dict, it's gone.

Then, using file (with pickled data) is not thread-safe. Depending on how you wish to implement storage for the counter, you may be interested in file locking.

There is also shelve module in Python, which may help with storing lexicons (though, it is not thread-safe).

Hard to say what is your case now or in the future, but you may want to use some special purpose file-based storages, which are designed for performance and concurrency. For example, sqlite.

Minor notes:

in Python, it is possible to make equality comparison by defining __eq__ method (and the mentioned Counter class just does it - no need to make any special provisions), same for the size - in Python it's usually len(), and in case of a Counter there is no need to make another method - size (may need __len__ method if Lexicon is not inherited from some dict)
thread-safety of the add (if you still use it) can be fixed by using setdefault method with consequent += 1 (actually, not sure - you may need thread lock - https://stackoverflow.com/questions/23547604/python-counter-atomic-increment ). Atomicity of collections Counter's update needs to be checked and wrapped.
as for performance, you can check with Python's timeit

If you insist on preserving your add method, you can consider a rewrite:

self.dictt.setdefault(token, 0)
self.dictt[token] += 1

(again, check if this is faster than self.dictt[token] = self.dictt.get(token, 0) + 1 ). Also this discussion may be relevant: It provides defaultdict implementation, which looks even simpler than the Counter one.

Note on concurrency: It's a bit fragile to assume multi-threading-only. For example, some misconfiguration can make your application multiprocess one. In that case you will have arbitrary results in the file, even if you will have both thread locks and file locks. Maybe, this concern is not a problem in your settings, but if you care about having terms counted right, you may need to make only single process to control the file, all others just requesting adds to that process. Actually, a counter is something, which despite simplicity of the operation (just add one!), may cause concurrency problems and inefficiency problems with high-velocity data. All kinds of optimizations may be needed to implement a high-performance counter correctly (just one of the first examples google gives: three components in the solution, uses relational database and memcached). But elaborating on that is out of scope in this answer.


Summation of primes takes forever Project Euler Problem 10 asks: "What is the sum of all prime numbers below 2 million?"

How do I speed up my code? It's taking forever and I have never gotten the answer. However, I am sure that my code is right; I cheated the answer from online as I couldn't wait for an hour.

def isPrime(number):

    for n in xrange (2,number):
        if number%n == 0:
            return False
    return True

result = 2

for k in xrange (3,2000000,2):
    if isPrime(k):
        result += k
        print result,"     ",k

print "The total is %d" % result

The result is 142913828922.

When the performance of your program in the face of large inputs is not even in the right ballpark, it's time to look for a new algorithm, even if your code is correct.  Factoring numbers is considered a "hard" problem — most public-key cryptography relies on the fact that factoring numbers takes a long time.

The Sieve of Eratosthenes would be perfect for this problem.  It should be possible in about 10 lines of code, and run in about one second.

Implementation hint:

# Build a big list
prime_list = range(limit + 1)

# Strike non-prime numbers 0 and 1 from the list
prime_list[0] = prime_list[1] = None

# TODO: Strike all multiples of prime numbers from the list
for k in xrange(len(prime_list)):
    ...

# Sum the numbers that have not been stricken out
return sum(filter(None, prime_list))

Alternatively, strike out numbers by setting them to 0 instead of None.  That removes the need to call filter(), but I think the clarity would suffer a tiny bit.


10 green bottles assignment This code was for a university assignment.  It's well passed now so don't be scared of giving me false grades, for example. I was wondering what could be done better, or bits of code that are horribly inefficient or are bad practices.

def main():
    import time
    while True:
        try:
            num1 = int(input("Pick a number between 10 and 30: "))
        except ValueError:
            print("That's not a number!")
            continue
        if num1 > 30 or num1 < 10:
            print("Not Valid!")
            continue
        break
    while True:
        try:
            hue = str(input("Pick a colour; Red, Green, Blue: "))
        except ValueError:
            print("Letters not numbers!")
            continue
        if  (hue == "Red") or (hue == "red") or (hue == "Green") or (hue == "green") or (hue == "Blue") or (hue == "blue"):

            break
    print("Generating File")
    time.sleep(3)
    numbers ='no', 'One', 'Two', 'Three', 'Four', 'Five', 'Six', 'Seven', 'Eight', 'Nine', 'Ten', 'Eleven', 'Twelve', 'Thirteen', 'Fourteen', 'Fifteen', 'Sixteen', 'Seventeen', 'Eighteen', 'Nineteen', 'Twenty', 'Twentyone', 'Twentytwo', 'Twentythree', 'Twentyfour', 'Twentyfive', 'Twentysix', 'Twentyseven', 'Twentyeight', 'Twentynine', 'Thirty'
    text_one = hue +' bottle%s\nHanging on the wall'
    text_two = "And if one " + hue + " bottle\nShould accidentally fall\nThere'll be"
    text_three =' \n'

    with open(numbers[num1] + ' ' + hue + ' Bottles.txt', 'w') as a:
        for l in range(num1, 0, -1):
            a.write(numbers[l]+ ' ')
            if l == 1:
                a.write(text_one % '' +'\n')
            else:
                a.write(text_one % 's' +'\n')

            a.write(numbers[l] + ' ')
            if l == 1:
                a.write(text_one %  '' + '\n')
            else:
                a.write(text_one % 's' +  '\n')
            a.write(text_two + ' ')
            a.write(numbers[l-1] + ' ')
            if (l - 1) ==1 :
                a.write(text_one % ''+'\n')
            else:
                a.write(text_one % 's'+'\n')
            a.write('\n' + '\n')

if __name__ == '__main__':
    main()

This should be saved to a folder and ran from the Python file it's saved as, not ran in an IDE.

A good place to start is to break your big main method into smaller chunks of work. Then you can focus on improving the code in each section.

Currently your main method does many things, it reads user input, builds up a song and writes it to a file. Here's how you might rewrite it:

def main():
  num_bottles = get_num_bottles_from_user()
  hue = get_hue_from_user()
  song = build_song(num_bottles, hue)
  out_filename = create_out_filename(num_bottles, hue)
  write_song_to_file(song, out_filename)

The function is easier to read if all the calls it makes are at a similar level of abstraction. The main method doesn't do low level operations like input reading or file writing, it tells a story.


Parsing a basic scripting language I'm working on writing a scripting language with ANTLR and C++. This is my first actual move from ANTLR grammars into the C++ API, so I'd like to know if this would be a good way to structure the grammar (later I will be adding a tree parser or tree rewriting rules though).

grammar dyst;

options
{
  language = C;
  output = AST;
  ASTLabelType=pANTLR3_BASE_TREE;
}

program : statement*;

statement : stopUsingNamespaceStm|usingNamespaceStm|namespaceDefineStm|functionStm|defineStm|assignStm|funcDefineStm|ifStm|whileStm|returnStm|breakStm|eventDefStm|eventCallStm|linkStm|classDefStm|exitStm|importStm|importOnceStm|directive;

namespaceDefineStm : 'namespace' ident '{' statement* '}';

usingNamespaceStm : 'using' 'namespace' ident (',' ident)* ';';

stopUsingNamespaceStm : 'stop' 'using' 'namespace' ident (',' ident)* ';';

directive : '@' directiveId argList? ';';

directiveId : ID (':' ID)*;

importOnceStm : 'import_once' expression ';';

importStm : 'import' expression ';';

exitStm : 'exit' expression? ';';

classDefStm : 'class' ident ('extends' ident (',' ident)*)? '{' (classSection|funcDefineStm|defineStm|eventDefStm)* '}';

classSection : ('public'|'private'|'protected') ':';

linkStm : 'link' ident 'to' ident (',' ident)* ';';

eventCallStm : 'call' ident (',' argList)? ';';

eventDefStm : 'event' ident '(' paramList? ')' ';';

returnStm : 'return' expression ';';

breakStm : 'break' int ';';

ifStm : 'if' '(' expression ')' '{' statement* '}';

whileStm : 'while' '(' expression ')' '{' statement* '}';

defineStm : 'global'? 'def' ident ('=' expression)? ';';

assignStm : ident '=' expression ';';

funcDefineStm : 'function' ident '(' paramList? ')' ('handles' ident (',' ident)*)? '{' statement* '}';

paramList : param (',' param)?;

param : ident ('=' expression)?;

functionStm : functionCall ';';

functionCall : ident '(' argList? ')';

argList : expression (',' expression)*;

//Expressions!
term : functionCall|value|'(' expression ')';

logic_not : ('!')* term;

bit_not : ('~')* logic_not;

urnary : '-'* bit_not;

mult : urnary (('*'|'/'|'%') urnary)*;

add : mult ('+' mult)*;

relation : add (('<='|'>='|'<'|'>') add)*;

equality : relation (('=='|'!=') relation)*;

bit_and : equality ('&' equality)*;

bit_xor : bit_and ('^' bit_and)*;

bit_or : bit_xor ('|' bit_xor)*;

logic_and : bit_or ('&&' bit_or)*;

logic_or : logic_and ('||' logic_and)*;

expression : logic_or;

value : ident|float|int|string|boolean|newObject|anonFunc|null_val;

anonFunc : 'function' '(' paramList? ')' '{' statement* '}';

newObject : 'new' ident ('(' argList ')')?;

ident : ID (('.'|'::') ID)*;

float : FLOAT;

int : INTEGER;

string : STRING_DOUBLE|STRING_SINGLE;

boolean : BOOL;

null_val : NULL_VAL;

FLOAT : INTEGER '.' INTEGER;
INTEGER : DIGIT+;

BOOL : 'true'|'false';

NULL_VAL : 'null'|'NULL';

STRING_DOUBLE : '"' .* '"';
STRING_SINGLE : '\'' .* '\'';

ID : (LETTER|'_') (LETTER|DIGIT|'_')*;

fragment DIGIT : '0'..'9';
fragment LETTER : 'a'..'z'|'A'..'Z';

NEWLINE : ('\n'|'\r'|'\t'|' ')+ {$channel = HIDDEN;};
COMMENT : '#' .* '\r'? '\n' {$channel = HIDDEN;};
MULTI_COMMENT : '/-' .* '-/' {$channel = HIDDEN;};

If you are wondering about exactly what it is I'm using this for, you can take a look here.

The grammar itself is pretty unreadable "as is". A rule like:

statement : stopUsingNamespaceStm|usingNamespaceStm|namespaceDefineStm|functionStm|defineStm|assignStm|funcDefineStm|ifStm|whileStm|returnStm|breakStm|eventDefStm|eventCallStm|linkStm|classDefStm|exitStm|importStm|importOnceStm|directive;

would be far more readable when declared like this:

statement
  :  stopUsingNamespaceStm
  |  usingNamespaceStm
  |  namespaceDefineStm
  |  functionStm
  |  defineStm
  |  assignStm
  |  funcDefineStm
  |  ifStm
  |  whileStm
  |  returnStm
  |  breakStm
  |  eventDefStm
  |  eventCallStm
  |  linkStm
  |  classDefStm
  |  exitStm
  |  importStm
  |  importOnceStm
  |  directive
  ;

You'll want to explicitly end the entry point of your parser, the rule program, with the end-of-file token, otherwise your parser might stop parsing prematurely. With EOF, you force the parser to read the entire tokens stream.

program
  :  statement* EOF
  ;

Make explicit tokens for keywords, don't mix them inside your parser rules.

Instead of:

importStm
  :  'import' expression ';'
  ;

it's better to do:

importStm
  :   Import expression ';'
  ;

Import
  :  'import'
  ;

This will make your life easier at a later (tree walking) stage. Without explicit lexer tokens, it is unclear for you when debugging what tokens there actually are in your tree.
Your lexer rules:

STRING_DOUBLE : '"' .* '"';
STRING_SINGLE : '\'' .* '\'';

can never contain either double- or single quotes. So, it's impossible to have a string literal with a double- and single quote in it.

Better to do something like this:

STRING_DOUBLE
  :  '"' ('\\' ('\\' | '"') | ~('\\' | '"'))* '"'
  ;

which will allow a double quoted string to contain double quotes as well.

That's all I saw at a first glance. I didn't look real close, so there might be more that can be improved.


Filtering Array Based on Element Values Required to splice specific element value in an array based on the condition.
Both the below code does the same job and yields the correct results. But couldn't figure out how to reduce coding structure with ECMAScript and which is performance oriented considering an array would have atleast 1.5k or even larger
This is with ECMAScript
this.candidateColumns.filter(item => item === 'Certificate').map(m => {
    if (!this.showControlsBasedOnPrivileges('CERTIFICATEPRINT')) {
        this.candidateColumns = this.candidateColumns.filter(item => item !== 'Certificate')
    }
});

This is simple forEach Loop code
this.candidateColumns.forEach((currentValue, index) => {
    if (currentValue === 'Certificate' && !this.showControlsBasedOnPrivileges('CERTIFICATEPRINT')) {
        this.candidateColumns = this.candidateColumns.filter(item => item !== 'Certificate')
    }
});

Any other Built In function Approach could be done for better performance or readability

.map is only for the construction of new arrays - for example, arr.map(num => num / 2), to transform every element of one array into another array. If you aren't returning anything from the .map callback, or if you aren't using the resulting array, then .map isn't appropriate. If all you're interested in is something that can be accomplished with side-effects, use a generic iteration method like for..of or forEach (like with your second snippet).
But:
Select the proper iteration method - forEach can be used to accomplish pretty much any task while iterating over an array, but that's somewhat of a weakness, because it makes the intent of the code somewhat less clear. In this case, your logic is:

If CERTIFICATEPRINT showing is off, remove any Certificate items from the candidateColumns array.

There's no need for a nested loop, which will increase the computational complexity to O(n ^ 2) (a big jump). This can easily be accomplished by putting a .filter inside an if, O(n):
if (!this.showControlsBasedOnPrivileges('CERTIFICATEPRINT')) {
  this.candidateColumns = this.candidateColumns.filter(item => item !== 'Certificate')
}

performance oriented considering an array would have atleast 1.5k or even larger

With thousands of elements, O(n ^ 2) would indeed be a problem, but the O(n) operation should be quite reasonable; a few thousand operations is next to nothing on modern hardware.
Is reassignment necessary? You assign the result of the filter to this.candidateColumns. Is that required? Could you, for example, allow the filtered array to be used by returning it instead, or by passing it along to whatever needs the data? If possible, consider doing so, if it doesn't make the code convoluted - code is generally easier to make sense of when mutations are only done when necessary.


Find and display the position of a word in a sentence How can I simplify this code:

Public Class Form1
Private Sub BtnPositions_Click(sender As Object, e As EventArgs) Handles BtnPositions.Click
    ' 'txtSentence.Text = "The cat sat on the mat" ' Temp for testing

    If TxtAddSentence.Text = "" Then
        MsgBox("The text you are searching is empty, so there are no matches." & vbCrLf & "Please enter some text and retry.")
    End If

    Dim WordArray() = {""}
    Dim strSearchWord As String

    BuildWordArray(TxtAddSentence.Text, WordArray)
    strSearchWord = TxtAddWord.Text
    FindPositionOfWords(strSearchWord, WordArray)

    If TxtAddSentence.Text = "" Then
        MsgBox("The sentence box is empty, so there are no matches.")
    End If

End Sub
Private Sub BuildWordArray(ByVal strText, ByRef strArray)
    Dim strTextArray1() = Split(strText, " ")
    strArray = strTextArray1
End Sub

Private Sub FindPositionOfWords(ByVal strSearchWord, ByVal strWordList)

    Dim intIndex As Integer = 1
    Dim strPosn As String = ""
    For Each strWord As String In strWordList
        If UCase(strWord) = UCase(strSearchWord) Then
            If strPosn = "" Then
                strPosn = strPosn & Str(intIndex)
            Else
                strPosn = strPosn & "," & Str(intIndex)
            End If
        End If
        intIndex = intIndex + 1
    Next
    If strPosn = "" Then
        MsgBox("The Search Word Here Box Is Empty", MsgBoxStyle.Exclamation)
    Else
        MsgBox("The word '" & strSearchWord & "' was found at positions" & strPosn)
    End If
End Sub
End Class

please note I am not that great at vb.net

Variables - Define them! You haven't defined

   TxtAddWord
   TxtAddSentence (though I see txtSentence commented out)

are they defined on the form design?

Testing for something to be "" usually has a better way. I think something like this -

If String.IsNullOrEmpty(txtaddsentence) Then

Speaking of that if - you have it twice - why?

If TxtAddSentence.Text = "" Then
    MsgBox("The sentence box is empty, so there are no matches.")
End If

I can't see TxtAddSentence changing after the first check as there aren't any functions to return a string.

Speaking of strings, why are you always using

TxtAddSentence.Text

When you can just

    Dim TxtAddSentence = "The cat sat on the mat" ' Temp for testing
    Dim TxtAddWord = "sat"

Which are string type.

Naming - use variable names to describe what is happening, and avoid that hungarian notation.

Variable -> can be ->
TxtAddSentence      sentenceAddition
TxtAddWord          wordToAdd
strSearchWord       searchWord
strText             why? it's already sentenceAddition
strArray            why? it's already wordArray
strWordList         what is this? a List Type? An Array Type? A String?
intIndex            index
strPosn             searchPosition
strWord             listedWord

Additionally, you're not giving a type to your variables:

ByVal strText, ByRef strArray)
ByVal strSearchWord, ByVal strWordList

Can be, as far as I can tell

Private Sub FindPositionOfWords(ByVal searchWord As String, ByVal wordArray() As String)
Private Sub BuildWordArray(ByVal sentenceAddition, ByRef wordArray)

It would be best practice to give a type to the variables in BuildWordArray - but I'm not entirely sure what type they should be.

Here -

Dim strPosn As String = ""

If this is position, why isn't it an integer? I would expect an integer, not a string.


Optimization fo code snippet including multiple np.multipy statements I working on optimizing some code for speed in a python module. I have pinned down the bottleneck and is a code snippet which calculates three np.ndarrays. Namely the following code:

xh = np.multiply(K_Rinv[0, 0], x )
xh += np.multiply(K_Rinv[0, 1],  y)
xh += np.multiply(K_Rinv[0, 2],  h)
yh = np.multiply(K_Rinv[1, 0],  x)
yh += np.multiply(K_Rinv[1, 1],  y)
yh += np.multiply(K_Rinv[1, 2],  h)
q = np.multiply(K_Rinv[2, 0],  x)
q += np.multiply(K_Rinv[2, 1],  y)
q += np.multiply(K_Rinv[2, 2],  h)

where x, y and h are np.ndarrays with shape (4206, 5749) and K_Rinv is a np.ndarray witch shape (3, 3). This code snippet is called multiple times and takes more than 50% of the time of the whole code. Is there a way to speed this up ? Or is it just as it is and can't be speed up.

Every idea and criticism is welcome.

First a quick non-performance related mode: np.multiply could simply be replaced by * in your case since it's basically scalar x array. That would make the code less verbose.

xh = K_Rinv[0, 0] * x
xh += K_Rinv[0, 1] * y
xh += K_Rinv[0, 2] * h

My first intuition on your problem was that it lends itself to be rewritten as scalar product. See the following example:

import numpy as np

K_Rinv = np.random.rand(3, 3)

x = np.random.rand(4206, 5749)
y = np.random.rand(4206, 5749)
h = np.random.rand(4206, 5749)
xyh = np.stack((x, y, h))

xh_dot = np.dot(xyh, K_Rinv[0, :])

But it turns out, that this is 4x slower than what you have.

The "vectorized" version below still turns is about 1.5x slower than what you have for my toy dataset.

xh_vec = (xyh * K_Rinv[0, :]).sum(axis=2)

A quick check with np.allclose(...) does however confirm, that the results are indeed (numerically) equivalent.

You might see some improvement by putting your code into a separate function and apply the @jit(nopython=True) decorator from numba if that's an option. numba is a Just-in-Time compiler for Python code, and is also aware of some, but not all, numpy functions. For the simple test above, the time was a bit more than half than that without using numba. When measuring, take care to exclude the first run, since that's when the compilation takes place.

Unfortunately, I'm a little bit short on time at the moment to look into this further.


Codechef: Prime Number Generator Peter wants to generate some prime numbers for his cryptosystem. Help
him! Your task is to generate all prime numbers between two given
numbers!
Input
The input begins with the number t of test cases in a single line
(t<=10). In each of the next t lines there are two numbers m and n (1
<= m <= n <= 1000000000, n-m<=100000) separated by a space.
Output
For every test case print all prime numbers p such that m <= p <= n,
one number per line, test cases separated by an empty line.
Example
Input:
2
1 10
3 5

Output:
2
3
5
7

3
5

My code
x = raw_input()

for i in range(int(x)):
    a = map(int, raw_input().split())
    if a[0] == 1:
        a[0] = 2
    for num in range(a[0],a[1]+1):
        if num == 2:
            print num
        for j in range( 2, num):
            if num%j == 0:
                break
            elif j == num - 1:
                print num
    print " "
    i = i + 1

I think Peilonrayz's answer is misleading (and might also timeout if you implement it): yes, you should exploit the limits, and yes, you can use the Sieve of Eratosthenes to speed things up, but not in the way he suggested.

Let's examine the time/space complexities of a bunch of algorithms and see how they compare. Below, we'll use \$N\$ to denote the maximum of \$m\$ and \$n\$, \$D\$ to denote the maximum difference between \$m\$ and \$n\$, and \$T\$ to denote the number of test cases. For the problem at hand, \$N = 10^9\$, \$D=10^5\$, and \$T=10\$.

OP's algorithm. Here for every input pair \$(m, n)\$, we loop over all numbers \$x\$ in this range, and check whether it is prime by checking whether it is divisible by any number between \$1\$ and \$x\$ inclusive. In the worst case, this requires time \$O(TDN)\$ and space \$O(1)\$.
Sieve the entire range (Peilonrayz/Billal). Here we use the trick of precomputing the set of primes in the entire allowed interval of length \$N\$, which allows us to answer each input query in time \$O(D)\$. If we use a fast sieving algorithm to do this precomputation [f1] this will take time around \$O(N)\$ (suppressing small logarithmic factors). This gives us an algorithm with time \$O(N + TD)\$ and space \$O(N)\$.

This is already a good deal faster than the original algorithm (at the cost of a lot more space). However, you can already do about as well in terms of time (and much better in terms of space) by just making some simple optimizations to OP's algorithm:

OP's algorithm with faster primality checking. One very simple improvement to make to the original algorithm is to remove a lot of the redundant work in checking whether a number is prime. A nice "folklore" result about primality checking is that to check whether a number \$x\$ is prime, you only need to check whether it is divisible by anything less than or equal to \$\sqrt{x}\$ (if it is composite, at least one factor will be less than this). This means you can check whether each number is prime in time \$O(\sqrt{N})\$ [f2] instead of \$O(N)\$ as in the original code. This improved algorithm requires worst-case time \$O(TD\sqrt{N})\$ and space \$O(1)\$. In practice, this runs pretty fast [f3], so it will probably outperform method 2 and might be sufficient for solving this problem.
Sieving only the relevant interval. Sieving methods like the Sieve of Eratosthenes are great, but it is important to understand why rather than apply them blindly. One reason why they are so great is that they let you eliminate many composite numbers at once very cheaply; for example, once you notice \$2\$ is prime, you can very efficiently loop over all larger even numbers and mark them as composite.

It would be nice to do this for the interval \$[m, n]\$ without having to do it for the entire interval \$[1, N\$]. By using our earlier observation that every composite number in this range is divisible by a prime less than \$\sqrt{N}\$, we can use the following strategy:

Precompute all the primes less than \$\sqrt{N}\$ by using the Sieve of Eratosthenes. This takes time approximately \$O(\sqrt{N})\$ and space approximately \$O(\sqrt{N})\$.
Make an empty boolean array of length \$n-m+1\$ with all values initialized to true, corresponding to the numbers between \$m\$ and \$n\$ inclusive.
For each prime in the list from 1., mark all multiples of that prime as false in the array from 2. For a prime \$p\$, this takes time \$O(D/p)\$.
The remaining values in the array set to true are the primes. Iterate over the array and print them out.

The tricky part of analyzing this is figuring out how long step 3 should take. The runtime of this step is roughly \$O(D\sum_{p < \sqrt{N}}\frac{1}{p})\$, where the sum is over all primes less than \$\sqrt{N}\$. Simple bounds from harmonic series show this is at most \$O(D\log N)\$; more careful bounds from analyzing sums of reciprocals of primes show that this is actually \$O(D\log\log N)\$. (Incidentally, this is also where the \$\log\log N\$ comes from in the Sieve of Eratosthenes).

Overall, this algorithm takes time \$O(\sqrt{N} + TD\log\log N)\$ and space \$O(\sqrt{N} + D\$), and is a clear winner for the constraints set in the problem.

In terms of coding style advice, I think Coal_ covered a lot of the relevant points. One thing which is sometimes important for algorithmic problems in Python 2.7 is to use xrange instead of range; the main difference being that range actually stores the list in memory and can lead to increased space complexity (for example, even though your original algorithm should use space \$O(1)\$, it actually uses space \$O(N)\$ because of your use of range).

[f1] It doesn't really matter whether we use Sieve of Eratosthenes or Sieve of Atkin; the only theoretical difference is small logarithmic terms in the complexities, and in practice it shouldn't make much of a difference for numbers of this size.

[f2] In fact, there are very efficient primality tests which run in time and space polynomial in the logarithm of $N$. Using one of these would give you an algorithm that runs in time \$O(TD\log^{k}N)\$ and space \$O(\log^{k}N)\$ (here \$k\$ is some small integer, like 4). Theoretically this is better than the above methods (especially for large \$N\$), but the overheads from these primality tests make it not as fast as sieving for numbers this small.

[f3] One reason for this is that not every primality check takes time \$\sqrt{N}\$; for example, the primality check catches that all even numbers > 2 are not prime in the first step. You end up saving about a $\log N\$ factor over the naive time bound.


PPT generator using SOLID principles I want to use SOLID more and more but still learning, I'm improving a PPT generator and would like your help applying SOLID principles in it.

That generator should print two photos per slide, first one to the left, second to the right;
If the last photo is a left photo, it should be placed in center;
Each photo has a text right below it;
I'll use a third party PPT lib to do the low level stuff.

This is what I came up with, a little bit abstracted just to pass the idea, the sample is in PHP but I'll accept answers in any programming language:

class Controller
{
    $ppt = new Ppt(Ppt::DEFAULT_RESOLUTION, $background, $header, $footer);

    $ppt->writeSlides($photoCollection);

    $pptFile = $ppt->saveToFile();

    $ppt->deleteLocalTemporaryFiles();

    readfile($pptFile);
    unlink($pptFile);
}

class Ppt extends thirdPartyPptLib
{
    const DEFAULT_RESOLUTION = 'x';

    private $resolution;
    private $background;
    private $header;
    private $footer;

    public function __construct($resolution = null, $background = null, $header = null, $footer = null)
    {
        $this->resolution = $resolution;
        $this->background = $background;
        $this->header     = $header;
        $this->footer     = $footer;
    }

    public function writeSlides(\Traversable $photoCollection)
    {
        $i     = 0;
        $count = count($photoCollection);

        foreach ($photoCollection as $photoInfo) {
            $photoToLeft = $i % 2 == 0;
            $isLastPhoto = $i == ($count - 1);

            if ($isLastPhoto) {
                $slide = $this->newSlide();
                $slide->addCenterPhotoInfo($photoInfo);

            } else if ($photoToLeft) {
                $slide = $this->newSlide();
                $slide->addLeftPhotoInfo($photoInfo);

            } else {
                $slide->addRightPhotoInfo($photoInfo);
            }

            $i++;
        }
    }

    private function newSlide()
    {
        $slide = parent->newSlide();

        if ($this->hasBackground()) {
            $slide->setBackground();
        }

        if ($this->hasHeader()) {
            $slide->setHeader();
        }

        if ($this->hasFooter()) {
            $slide->setFooter();
        }

        return $slide;
    }
}

class Slide extends thirdPartyPptLibSlide
{
    public function addLeftPhotoInfo($photoInfo)
    {
        $this->addLeftPhoto($photoInfo->getPhoto());
        $this->addUnderlyingText($photoInfo->getText());
    }

    // functions addRightPhotoInfo and addCenterPhotoInfo very similar
}

Should I separate it more into different classes? Or methods? Should I create Interfaces?

You can remove the overloading of properties from constructor.
This way you can improve the code maintaning and readability.

An example of improvement:

class Style {
    // Properties here
}

class Ppt {
    public function __construct(Style $style) {
    }
}

Now you pass just the object of style with all the needed properties.


Async function with slow web reference(web service) slowing down webpage Is there a way to make a threading/async post with a web reference not slow down the whole website just because the web reference takes a long time to return a response?

I have a send email function that gets posted async with a threading system that basically looks like this. This whole .cs file is called Mail.cs (that's where the Mail. name comes from).

public static bool SendMail(var alotOfVariablesHere)
{
    MailParameters param = new MailParameters(alotOfVariablesHere);
    Thread thread = new Thread(Mail.QueueSendMailAsyc);
    thread.Start(param);
    return true;

}

static Object locker = new object();
public static void QueueSendMailAsyc(object Objparam)
{
    lock (locker)
    {
        SendMailAsyc(Objparam);
    }
}

Inside SendMailAsyc() we run our web reference:

WebReference.Service1 Converter = new WebReference.Service1();
Converter.Timeout = 3 * 60 * 1000;

byte[] file = Converter.ConvertFunction(File.ReadAllBytes(attachfilepath), fileName);

The only thing the WebReference does is receiving a byte array and a file name and then returns a new byte array.

Pretty simple and it all works fine.

But recently the server were the web reference is sitting on (a totally different server from the webpage) has started getting really slow.
This means that the web reference takes a long time to respond (it does respond and it does return everything like the way we want it. just that it's now a lot slower)

And this has started causing a lot of issues, like:
The webpage also get really slowed down, it takes ages to navigate through the pages and so on.

Plus the fact that this one single web reference threading/async post takes so long time means that there is plenty of time to put up even more threading/async posts meaning it'll put an even bigger pressure on the server, causing it to slow down a lot.

This issue is primary that the web reference has gotten really slow lately, and that our webpage server cannot handle the pressure of too many threads/async posts.

I can't do much about the server the web reference is sitting on has started getting really slow recently.

But is there anything I can do code-wise to make my system handle this really slow web reference better?

There's not enough information here to tell you what's wrong, but I'll tell you what I would do differently regardless.

Manually spawning threads is something you should avoid doing, especially if there's a chance that thread is just going to wait on a lock.  As the remote service gets slower and your app gets busier, you'll have more and more idle threads taking up resources.  You can make a single mail thread that has new params passed to it (in a thread-safe way).


Calculating Luhn-algorithm checksum digit Today I decided to learn some basic Haskell, and for starters I made a program for calculating the checksum of a Swedish personal identification number. It uses the Luhn-algorithm, aka. IBM MOD-10.

Explanation of this code can be found on Swedish Wikipedia and English Wikipedia

Here's a description of how the algorithm works:

Given a string of 9 digits, abcdefghi compute:

array = [a*2, b, c*2, d, e*2, f, g*2, h, i*2]

Then you computer the sum of the digits in this array, so for example if a*2 is 16, then that part counts as 1 + 6.

Then the result is how much more you have to add to make it evenly divisible by 10. For example, if sum == 54 then the result is 6 as 60 - 54 = 6.

import Data.Char

sumOfChars :: String -> Int
sumOfChars "" = 0
sumOfChars str = digitToInt(str !! 0) + sumOfChars(tail str)

twoMult :: Char -> String
twoMult c = show (digitToInt(c) * 2)

identificationSum :: String -> Int
identificationSum "" = 0
identificationSum str = if length str `mod` 2 == 1 then sumOfChars(twoMult(str !! 0)) + identificationSum(tail str)
  else digitToInt(str !! 0) + identificationSum(tail str)

remainingToTens :: Int -> Int
remainingToTens x = ceiling(fromIntegral x / 10) * 10 - x

determineLastDigit :: String -> Int
determineLastDigit str = remainingToTens(identificationSum(str))

To test the code:

determineLastDigit("811228987")

Prints:

4

As this is the first time ever I managed to make something in Haskell, any comments are welcome. Even though it is the first time though, feel free to rip my code apart as much as you would like and suggest any advanced things, I am always eager to learn.

This is a good start! I'll do two passes over your code, one to address issues of style, and another to leverage functions from the Haskell Prelude to decrease code length and bring it more in line with typical Haskell usage and idioms.

Style

Your sumOfChars function is a good example of tail-recursion, but in Haskell we would take that String apart through pattern matching. A Haskell String is really a list of characters, or [Char], so we'll decompose it as a list.

sumOfChars :: [Char] -> Int -- String is a synonym for [Char]
sumOfChars [] = 0 -- The empty list is the same value as the empty string, i.e. [] == ""
sumOfChars (c:cs) = digitToInt c + sumOfChars cs

The : is the list cons operator, c is the head of the list and cs is the tail. So, c == str !! 0 and cs == tail str. (And head is the function we usually use for getting the element at index 0 in a list.)

twoMult isn't wrong, but function application in Haskell doesn't require any parentheses. Parentheses are for grouping since function application has the highest precedence of any operation.

twoMult :: Char -> String
twoMult c = show (digitToInt c * 2)

Using an if statement at the top level of a function is usually an indication that you can use a guard. Guards are a bit like a multi-way if.

identificationSum :: [Char] -> Int
identificationSum [] = 0
identificationSum s@(c:cs) -- The 's@' part is an as-pattern, explained below
    | length s `mod` 2 == 1 = sumOfChars (twoMult c) + identificationSum cs
    | otherwise             = digitToInt c + identificationSum cs

Each guard begins with a pipe (|) and evaluates to a Bool value. First pattern matching takes place, then guards are evaluated in order. otherwise is just a synonym for True, i.e., a guard that always succeeds when evaluation reaches it.

I used an as-pattern there to bind the value of c:cs to an identifier. This is a handy shortcut and much preferable to writing c:cs all over the place if you need both the original value and its decomposition.

remainingToTens is a little odd, and I'd guess that you got to that answer when the compiler complained about there not being an instance for Fractional Int, yeah? Integer division uses a function called div, / is for Fractional values (like Float or Double) (yeah it's a little strange the first time you come across this).

remainingToTens :: Int -> Int
remainingToTens x = (x `div` 10 + 1) * 10 - x -- Edit: This is incorrect, use version below

And determineLastDigit just has a few extra parentheses.

determineLastDigit :: String -> Int
determineLastDigit s = remainingToTens (identificationSum s)

Idiomatic Haskell

Many of the operations you implemented can be expressed using some of the higher-order functions we have in the Prelude and coding in a functional style.

Instead of using primitive recursion on the elements of a list in sumOfChars, typical Haskell usage would see us using functions like sum and map to manipulate the entire list without getting into the weeds.

sumOfChars :: [Char] -> Int
sumOfChars cs = sum (map digitToInt cs)

maps type is (a -> b) -> [a] -> [b], that is, it takes a function - that itself takes an element of type a and returns something of type b - and a list and returns a list where that function has been applied to every element of the first list. sum adds all the values in a list of numbers.

One further thing we could do to that function is to write it in pointfree style. Writing functions pointfree is definitely an aspect of Haskell style, but don't concentrate on it overmuch until you have a solid grasp on the basics of the language. I've included an Appendix A at the bottom of this post where functions have been written pointfree for your reference.

identificationSum could be written many, many ways (some as in the appendices below) but given the context of sumOfChars, twoMult, and the specification of the algorithm I would favor separating out the functionality into two phases. First, constructing a new string based on doubling every other digit, and secondly summing all of the digits. To do this I'll introduce a new function.

doubleAlternating :: [Char] -> [Char]
doubleAlternating []       = []
doubleAlternating (c:[])   = twoMult c
doubleAlternating (c:d:cs) = twoMult c ++ [d] ++ doubleAlternating cs

And then identificationSum is a composition of doubleAlternating and sumOfChars.

identificationSum :: String -> Int
identificationSum s = sumOfChars (doubleAlternating s)

remainingToTens could really just benefit from some modular arithmetic. Use the rem function.

remainingToTens :: Int -> Int
remainingToTens x = negate x `mod` 10

I'll take it a little further with inlining in the appendices, but as it stands this is a very readable translation of the problem as it was stated. Having a close correspondence to the problem domain can be much more valuable than terse code for the sake of terseness.

Appendix A: Pointfree

One of the things I like about pointfree style is that it forces you to think in terms of functions, higher order abstractions, and data pipelines. Presented without further comment.

sumOfChars :: [Char] -> Int
sumOfChars = sum . map digitToInt

twoMult :: Char -> String
twoMult = show . (* 2) . digitToInt

identificationSum :: String -> Int
identificationSum = sumOfChars . doubleAlternating

determineLastDigit :: String -> Int
determineLastDigit = remainingToTens . identificationSum

Appendix B: Code golf

Understanding this will help you explore the Prelude and hopefully have a stronger grasp of function composition. I'll admit it's a little showy though. ;-)

checksum :: String -> Int
checksum s = negate (sum . map digitToInt . concatMap show . zipWith ($) (cycle [(* 2), id]) . map digitToInt $ s) `mod` 10


Check site inclusion and exclusion I have two string variables site_inclusion and site_exclusion.  If site_inclusion has some values, then I don't care what values site_exclusion contains.  This means site_inclusion overrides site_exclusion if site_inclusion has some values. But if site_inclusion is null and site_exclusion has some values, then go into the site_exclusion loop.

Requirements:

If site_inclusion and site_exclusion both are null then set useTheSynthesizer as true;
If site_inclusion is not null and it matches with the regexPattern then set useTheSynthesizer as true. And I don't care what values are there in site_exclusion.
If site_inclusion is null and site_exclusion is not null and site_exclusion does not match the regexPattern, then set useTheSynthesizer to true.

I wrote the below code but somehow I think I am repeating some stuff here in the if/else loop. Any code improvements that fulfill my conditions will be appreciated.

String site_inclusion = metadata.getSiteInclusion();
String site_exclusion = metadata.getSiteExclusion();

// fix for redundant data per site issue
if(site_inclusion != null && site_inclusion.matches(regexPattern)) {
    useTheSynthesizer = true;
} else if(site_exclusion != null && !(site_exclusion.matches(regexPattern))) {
    useTheSynthesizer = true;
} else if(site_inclusion == null && site_exclusion == null ) {
    useTheSynthesizer = true;
}

Please spinning it to a separate function. Here is what I think is a little more simpler implementation.

public boolean checkInclusion(String site_i, String site_e, Regex re) {
    if (site_i != null)
        return site_e.matches(re);
    else if(site_e != null)
        return !site_e.matches(re)))
    else
        return true;
}

In your implementation, what happens when the site inclusion has values but does not match, and site_exclusion is null? It seems to not match any of the conditions, which is not in sync with the requirements from my reading.

Also what happens when the site inclusion has values but does not match, and site_exclusion has values that does not match? In your implementation, it seems like site exclusion takes precedence. But that does not seem to be the requirement.

Perhaps I am reading either requirement wrongly or your implementation conditions wrongly?

A rather terse implementation that is same as above is,

public boolean checkInclusion(String site_i, String site_e, Regex re) {
    if (site_i != null) return site_i.matches(re);
    return site_e == null || !site_e.matches(re)
}


Conversion into hexadecimal using C++ I want to read a vector of eight bytes, convert them into hexadecimal, store them in a std::string and finally write them into a binary file.
I am looking for suggestions and recommendations to improve the efficiency and readability of the code.
#include <vector>
#include <string_view>
#include <iostream>
#include <fstream>
#include <cassert>

std::string hexify(std::vector<unsigned char> const & v) {

std::string str(2 * v.size(), 'x');
auto k = str.begin();

for(auto i{v.begin()}; i < v.end(); ++i) {
    *k++ = "0123456789ABCDEF"[*i >> 4];
    *k++ = "0123456789ABCDEF"[*i & 0x0F];
}
return str;

}

void writeFile(std::string_view str, std::string strToRead) {

assert(str.data() != nullptr);
std::ofstream f(str.data(), std::ios::binary);

if(f.is_open()) {
    f << strToRead;
}
f.close();
}

int main() {

std::vector<unsigned char> const v{'5', '1', '5', '7', '9'};
writeFile("text.bin", hexify(v));
}

First off, your formatting is not very good.  It makes it difficult to read.  I would suggest looking at your IDE's menu options and use the format command.

It seems like you're trying to reinvent the wheel here. The <ios> header contains that option already.  To write the vector to a file as base 16 numbers, you can do that directly in the writeFile function:

void writeFile(std::string_view str, std::vector<unsigned char> const & v)
{

    assert(str.data() != nullptr);
    std::ofstream f(str.data(), std::ios::binary);
    for(auto c : v)
    {
        if(f.is_open())
        {
            f << std::hex << (int)c;
        }
    }
    f.close();
}

If creating the string is more to the point than writing to the file, you can use a stringstream and do the same thing:

#include <sstream>

std::string hexify(std::vector<unsigned char> const & v)
{
    std::stringstream ss;
    for(auto c : v)
    {
        ss << std::hex << (int)c;
    }
    return ss.str();
}

If characters with single digit character codes are to be used and the leading 0 is required, it's simply a matter of adding a couple of functions from the <iomanip> header:

<< std::setw(2) << std::setfill('0') << std::hex << (int)c

If reinventing the wheel is the point, first there's a tag for that.

When you have a need for literal (magic) use a constant variable. It gives meaning to anonymous values.

Generally speaking when you're using iterators in a loop, since end() is past the last element, it's better to use not equals(!=) instead of less than.