Counting unordered factorizations with distinct parts Trying to solve this combinatorics problem, I wrote some functions to count the number of unordered distinct factorizations of an integer n into k distinct parts.

The following function builds up a list of unordered distinct factorizations of n with largest part at most m.

duf' :: Integer -> Integer -> [[Integer]]
duf' m 1 = [[]]
duf' 1 n = []
duf' 0 n = []
duf' m n | isPrime n = if m < n then [] else [[n]]
         | otherwise = concatMap (\x -> map (\y -> x : y) $
             duf' (x-1) (n `div` x) ) $ tail $
             takeWhile (<= m) $ toList $ divisors n

It is then used below to generate the list of all unordered distinct factorizations of n.

duf :: Integer -> [[Integer]]
duf n = duf' n n

To count the number of factorizations of k parts only I use a simple (and inefficient ?) filter.

pd n k = length . filter (\x -> k == length x) $ duf n

As a beginner, I have the feeling this is not really Haskell-ish. Any help to improve the syntax is welcome !

My concern is that I would like to avoid filtering and (if possible) to count without generating an enormous list. Is there a way to "merge" pd and duf' ? Or at least optimizing the thing ?

I use this two external functions : Math.NumberTheory.Primes.Factorisation.divisors and Data.Numbers.Primes.isPrime. Are there better choices ?

I took a couple passes (gist) at this and managed to improve the style a bit, but I couldn't move the needle on performance at all.

Style

My thoughts and justifications on important changes are written as in-line comments below.

module Main where

import qualified Math.NumberTheory.Primes.Factorisation as Factorisation
import           Data.Numbers.Primes                                     (isPrime)
import           Data.Set                                                (toAscList)

duf' :: Integer -> Integer -> [[Integer]]
duf' _ 1 = [[]] -- Underscores in place of unused variable names
duf' 1 _ = [  ]
duf' 0 _ = [  ]
duf' m n | isPrime n = if m < n then [] else [[n]]
         | otherwise = concatMap (\x -> map (x:) $ duf' (x-1) (n `div` x)) -- Sectioning cons operator
                                 (tail . takeWhile (<= m) $ divisors n) -- Reflowed for legibility
  where
    divisors = toAscList . Factorisation.divisors -- Locally bound to decrease datatype mismatch line noise

duf :: Integer -> [[Integer]]
duf n = duf' n n

pd :: Integer -> Int -> Int
pd n k = length . filter ((== k) . length) $ duf n -- Function composition for ordering

Some reorganizing of layout in duf' made it a lot more comprehensible.

Performance

After rewriting duf', I noticed that the inner lists it returns are only ever really used for their length, so I came up with this version that tosses out the actual elements in favor of keeping a tally whenever a new element would be added instead.

duf' :: Integer -> Integer -> [Int]
duf' _ 1 = [0]
duf' 1 _ = [ ]
duf' 0 _ = [ ]
duf' m n | isPrime n = if m < n then [] else [1]
         | otherwise = concatMap (\x -> map (+1) $ duf' (x-1) (n `div` x))
                                 (tail . takeWhile (<= m) $ divisors n)
  where
    divisors = toAscList . Factorisation.divisors

duf :: Integer -> [Int]
duf n = duf' n n

pd :: Integer -> Int -> Int
pd n k = length . filter (== k) $ duf n

Unfortunately, this doesn't net us any wins in run time as I only really smeared out the inner call to length from filter in pd across duf' (this is less idiomatic, in my opinion). One potential positive is that this version may be more space efficient due to not keeping lists of elements around when all we really care about is the list spine anyway. I wasn't testing for that though, so I don't have any cold hard facts at hand and I suspect you'd have to use an argument strict version of (+1) to prevent building up chains of thunks before realizing any real decrease in memory.


VBA, Transferring of Data from one File to another I have this code that:

Clears my worksheet from row 12 and onward in Workbook A
Opens a new file Workbook B
Copys the sheet to my Workbook A. and Close Workbook B
Renames sheet in Workbook A
Transfers data from New Sheet to another Tab.

This seems to take me 10 minutes+, it is roughly 35k Rows.
Is there a faster way of doing this?
Thanks.
    Sub Mainsub()

    Worksheets("Main Data").Rows("12:" & Rows.Count).ClearContents

    'Copy data from the CM File to Template
    Application.ScreenUpdating = False

    'Open CM file
    Set MainDataCM = Workbooks.Open(Sheets("Input").Range("B16") & Sheets("Input").Range("B19"))
    'Copy main data tab to EPM file workbook
    MainDataCM.Sheets("Main Data").Copy After:=ThisWorkbook.Sheets(1)
    'Close CM Comm file
    MainDataCM.Close SaveChanges:=False
    Application.ScreenUpdating = True

    Sheets("Main Data (2)").Name = "CM_MainData"

    Worksheets("CM_MainData").Visible = False

    'Read the CM_MainData tab and copy the required columns in the MainData tab
    Dim k As Long
    k = Sheets("CM_MainData").Range("A1", Sheets("CM_MainData").Range("A1").End(xlDown)).Rows.Count
    Debug.Print (k)

    i = 12
    j = 2

    While j <= k
        Sheets("Main Data").Range("A" & i) = Sheets("CM_MainData").Range("A" & j)
        Sheets("Main Data").Range("B" & i) = Sheets("CM_MainData").Range("B" & j)
        Sheets("Main Data").Range("C" & i) = Sheets("CM_MainData").Range("C" & j)
        Sheets("Main Data").Range("D" & i) = Sheets("CM_MainData").Range("D" & j)
        Sheets("Main Data").Range("E" & i) = Sheets("CM_MainData").Range("E" & j)
        Sheets("Main Data").Range("F" & i) = Sheets("CM_MainData").Range("F" & j)
        Sheets("Main Data").Range("G" & i) = Sheets("CM_MainData").Range("G" & j)
        Sheets("Main Data").Range("H" & i) = Sheets("CM_MainData").Range("H" & j)
        Sheets("Main Data").Range("I" & i) = Sheets("CM_MainData").Range("I" & j)
        Sheets("Main Data").Range("J" & i) = Sheets("CM_MainData").Range("J" & j)
        Sheets("Main Data").Range("K" & i) = Sheets("CM_MainData").Range("K" & j)
        Sheets("Main Data").Range("L" & i) = Sheets("CM_MainData").Range("L" & j)
        Sheets("Main Data").Range("M" & i) = Sheets("CM_MainData").Range("M" & j)
        Sheets("Main Data").Range("N" & i) = Sheets("CM_MainData").Range("N" & j)
        Sheets("Main Data").Range("O" & i) = Sheets("CM_MainData").Range("O" & j)
        Sheets("Main Data").Range("P" & i) = Sheets("CM_MainData").Range("P" & j)

        i = i + 1
        j = j + 1
    Wend

    Worksheets("Input").Activate
    Worksheets("Input").Select

    MsgBox "Step 1 Completed"

End Sub

Seems like instead of transferring, Copying the Data and using ThisWorkbook was able to get the job done easily.
Sub Mainsubtwo()

Call Reset
Worksheets("Main Data").Rows("12:" & Rows.Count).ClearContents

Application.ScreenUpdating = False
Application.DisplayAlerts = False

'Open CM file
Set MainDataCM = Workbooks.Open(Sheets("Input").Range("B16") & Sheets("Input").Range("B19"))

With Sheets("Main Data")
lastrow = Sheets("Main Data").Range("A" & .Rows.Count).End(xlUp).Row
Sheets("Main Data").Range("A2:P" & lastrow).Copy
End With
'Paste into this workbook
ThisWorkbook.Sheets("Main Data").Range("A12").PasteSpecial xlPasteValues

'Close CM file
MainDataCM.Close SaveChanges:=False
Application.ScreenUpdating = True
Application.DisplayAlerts = True

Worksheets("Input").Activate
Worksheets("Input").Select

MsgBox "Step 1 Completed"

End Sub


Feedback on a small PHP function that generates JSON data I wrote a PHP function that generates JSON data to populate a line graph. Through jQuery, my code uses a GET request to the PHP script to fetch the JSON data. An example of the JSON data is as follows:

{"labels":[{"x":"2013-10-30","a":"1","b":"8"},{"x":"2013-10-31","a":"2","b":"14"},{"x":"2013-11-01","a":"5","b":"12"}]}

The data is used to generate a time graph showing two values per date:

x: date
a: value1
b: value2

I'm still a beginner programmer and want to have some peer review on the code I wrote. Please give me some feedback on how I can improve it, and fix any errors or inefficient methods if present.

Function

function generateGraphData($start, $end, $format, $api, $server, $pdo)
{
    //Fetch unique entries between the given dates
    $query = "SELECT DISTINCT DATE(traffic_date) FROM traffic WHERE traffic_date BETWEEN ? AND ? AND traffic_api = ? AND traffic_server = ? ORDER BY traffic_date DESC";
    $dbc = $pdo->prepare($query);
    $dbc->execute(array($start, $end, $api, $server));
    $results = $dbc->fetchAll(PDO::FETCH_ASSOC);

    //Count unique and total views for each result
    foreach ($results as $key => $value)
    {
        $date = "%".$value['DATE(traffic_date)']."%";

        $dbc = $pdo->prepare("SELECT count(*) FROM traffic WHERE traffic_date LIKE ? AND traffic_api = ? AND traffic_server = ?");
        $dbc->execute(array($date, $api, $server));
        $total = $dbc->fetchColumn();
        $dbc = null;

        $dbc = $pdo->prepare("SELECT count(*) FROM traffic WHERE traffic_date LIKE ? AND traffic_api = ? AND traffic_server = ? GROUP BY traffic_ip");
        $dbc->execute(array($date, $api, $server));
        $unique = $dbc->fetchColumn();
        $dbc = null;

        $data[$key]["x"] = $value['DATE(traffic_date)'];
        $data[$key]["a"] = $unique;
        $data[$key]["b"] = $total;
    }

    //Create filler dates
    $fillerDates = new DatePeriod(new DateTime($start), new DateInterval('P1D'), new DateTime($end));

    foreach($fillerDates as $date)
    {
        $filler_date_array[] = array('x' => $date->format("Y-m-d"), 'a' => '0', 'b' => '0');
    }

    $data = array_merge($data, $filler_date_array);

    //Remove filler dates if real date exists
    $filter_unique_array = array();
    $keysArray = array();

    foreach ($data as $innerArray)
    {
        if (!in_array($innerArray["x"], $keysArray))
        {
            $keysArray[] = $innerArray["x"];
            $filter_unique_array[] = $innerArray;
        }
    }

    //Sort by date
    array_sort_by_date($filter_unique_array, "date");

    //Format JSON
    $json = json_encode($filter_unique_array);
    $jsonlabel = '{"labels":'.$json.'}';
    echo $jsonlabel;
}

The first problem is what @Kinjal identified in your comment. You should only run your query once (in this particular case. You can do this because your querys basically return the same result).

So you run your query and load it into a table, and then you can iterate and work with that table. This will be much faster than running your query 3 times. Even so because 2 of your queries runs in a loop.

The second problem are your JSON members. using "a" and "b" as member names are a bad idea - name these what they should represent. I'd suggest substituting them for "unique" and "total". So your final JSON should looke like

{"date": "yyyy-mm-dd", "unique": 0, "total": 0 }


JavaScript navigation UI I think that the slice() is a bit hacky, and prone to breaking if I change my mind about the id names, but I wanted this to be generic so I could add types easily.

Is there maybe another / better way to do this just using CSS nesting the 'panels' inside the li elements?

Markup:

1

2

    ...

blah

blah

   ...

JavaScript:

function switchType(type){
    $("#type-select li a").removeClass("selected-type");
    $("#type-"+type).addClass("selected-type");
    $(".panel").removeClass("selected-panel");
    $("#panel-"+type).addClass("selected-panel");
}

// initial state
$("#no-results").hide();
switchType("1");
$("#type-select li a").click(function(){switchType(this.id.slice(5))});

This kind of navigation is usually referred to as tabs or tabbed navigation. There are loads of ready scripts on the net for this - for exampe - since you are using jQuery - jQuery UI. But since this is really a quite trivial functionality, it's quite okay to implement it yourself.

To make the implementation cleaner, you can add a direct reference to the matching panel in the link. I would use the href attribute and refer to the panel using a URL hash. This has the advantage, that it will work without JavaScript or CSS by scrolling to the chosen panel.

You can drop the ids on the links and use a direct reference to them in the function.

1

2

Consider using the CSS :target pseudo class for the functionality:

.panel { display: none; }
.panel:target { display: block; }

And add JavaScript, for browsers that don't support it and to set the classes:

$("#type-select li a").click(function(){return switchType(this)});

function switchType(link){
    var hashPos = link.href.indexOf("#");
    if (hashPos >= 0) {
      // Parsing out the hash and removing '#', in case the href
      // attribute contains a complete URL, and in case some other
      // method than a CSS selector is used to find the panel
      var panelId = link.href.substring(hashPos + 1);

      $("#type-select li a").removeClass("selected-type");
      $(link).addClass("selected-type");
      $(".panel").removeClass("selected-panel");
      $("#" + panelId).addClass("selected-panel");
    }
    return false;
}

Don't forget to return false so the the default action of the link (which will lead to unnecessary scrolling) is canceled.


Returning a collection inside of an IEnumerable method The following scenario came out of a discussion I was having with a fellow developer on this project. He likes to execute an Ienumerable as soon as possible, hence why he’s doing it inside of the method. Whereas, I’m thinking that if you wait until the object is actually needed, using differed execution, you’re eliminating wasted objects in the memory.

Knowing that PricingInformation is a List, is my return statement consider best practice or not? Should I defer the execution to a later time? Or is it wise to ToArray() the Select during the return statement?

Which of these approaches do you guys think is more appropriate?

public IEnumerable<ProductPrice> GetPrices(IEnumerable<string> skus, string accountNumber)
{
    if (skus.Any())
    {
        TesscoModel.PricingQuery priceQuery = new TesscoModel.PricingQuery();
        foreach (string sku in skus)
            priceQuery.Skus.Add(new TesscoModel.SkuDetail() { Sku = sku, Quantity = 1 });
        priceQuery.Account = accountNumber;
        TesscoModel.PricingInformationBatch pricingInfoBatch = RestHelper.PostJsonAsync<TesscoModel.PricingInformationBatch>(Settings.PricingServiceEndpoint, priceQuery).Result;

        return pricingInfoBatch?.PricingInformation?.Select(pi => new ProductPrice { Sku = pi.Sku, Price = pi.OnSalePrice != default(decimal) ? pi.OnSalePrice : pi.UnitPrice, ListPrice = pi.ListPrice })?.ToArray() ?? Enumerable.Empty<ProductPrice>();
    }
    else
    {
        return Enumerable.Empty<ProductPrice>();
    }
}

This method then gets called by another method inside of a helper. Inside of which, I am using FirstOrDefault to find the first sku. FirstOrDefault is again an enumerator.

I think the differed execution is more appropriate here, because I shouldn't execute the pointer until I absolutely have to. I'm thinking that by using ToArray() too early, I'm keeping the object on memory for too long. Even though, it might not be needed.

//Get sku Pricing in bulk
IEnumerable<ProductPrice> productPrices = productService.GetPrices(skus, (!account.IsNull()) ? account.AccountNumber : string.Empty);

foreach (var line in lines)
{
    var productPrice = productPrices.FirstOrDefault(p => p.Sku.Equals(line.Sku));
    line.ListPrice = productPrice?.ListPrice ?? default(decimal);
    line.YourPrice = productPrice?.Price ?? default(decimal);
}

There is no 0 and 1 answer

X likes to execute an IEnumerable as soon as possible, hence why X is doing it inside of the method.

Whereas

Y is thinking that if you wait until the object is actually needed, using differed execution, you’re eliminating wasted objects in the memory.

They are both wrong. The answer isn't digital, this is, there is no 0 or 1 answer. It depends. Sometimes you must execute a collection and return a non-IEnumerable.
Materialize when working with databases
Such a case would be querying a database. In this scenario you'd open a database connection (or create a context if it's some ORM) and dispose it at the end. If you don't execute it with ToList or ToArray etc. you won't be able to do it later. The runtime would wrap your enumerator with an anonymous type containing the context but when the execution leaves the method, the context gets disposed and it'll throw an excepiton the moment you try to get the data. In this situation you should return a non-IEnumerable type.
The reason the return type shouldn't be an IEnumerable in this case is that IEnumerable tells me that the return type is lazy so if I wanted to use it multiple times I'd call ToList myslef (again) to get the result which would unnecessrily enumerate the collection again.
So, the bottom line is: if you don't have to materialize the result (because you won't be able to get it later when the service/provider is disposed etc), don't do it but be consistent with the API (return types).

When the result value it's materialized I expect you to communicate this fact clearly by using ICollection, IList or anything else but IEnumerable.

Materialize for easier debugging when calling resources
As far as your code is concerned you call a web-service there

RestHelper.PostJsonAsync(..)

So you should ask yourself: Is it ok to be done multiple times if someone else uses Any and then starts enumerating it again or is better to prevent such situations (for whatever reason)?
I would use IList here for one more reason: it's quite difficult to debug lazy code that is calling web-services, databases or other resources. When something goes wrong here you'll be having a lot of fun.

Consider materialized parameters
The same priciples apply to parameters. In your example you execute skus multiple times: with Any and with the foreach loop.
Here, I'd requested an ICollection or an IList to tell the caller: Look out! I'll be using your collection more then once (so you might consider materializing it for better performance before givit it to me).


Recursively delete empty folders in Lua I have written a script that run in WingFTP Server which has a Lua scripting engine built in. The script recursively deletes empty folders except the specified root folder.

--Walks a file tree and delete empty folders except the root folder
function EmptyFolderDeleter(options)
    local rootPath = options.path
    local path = options.path

    function walk(path)
        local currentPath = path

        --cycle through the current directory, walking directories
        for dir in c_GetDir(currentPath) do
            local fullPath = currentPath..dir.."/"
            walk(fullPath)
        end

        --if we've got this far then we're at the bottom of the tree
        --there are no more directories to visit
        --now check if there is ANYTHING (file or folder) in the current folder
        local empty = true
        for isdir, file in c_GetFileDir(currentPath) do
            empty = false
        end

        --now check the 'empty' flag and if the folder is the root
        if empty == true and currentPath ~= rootPath then
            c_RemoveFileDir(currentPath)
        end

    end
    walk(path)
 end

The script uses three custom Lua functions that are built into WingFTP:

c_GetDir() = an iterator that returns directory names as string

c_GetFileDir() = an iterator that returns directory and file names as strings. Also returns a boolean whether the returned string is directory or not.

c_RemoveFileDir() = Deletes the specified file or folder

My script can be called like so like so:

local root = "D:/Clients/"
for dir in c_GetDir(root) do
    EmptyFolderDeleter{path=root..dir.."/"}
end

Is there is simpler or more efficient way to recursively delete empty sub folders in Lua?

Is there anything I can do to improve code readability?

Since you're not updating the variables path, rootPath and currentPath anywhere, use a single variable.

Define the function walk locally.

You can also break out of your c_GetFileDir loop once you've updated the empty flag. This will remove the time to iterate through all children of a directory.

  The help pages for WFTPServer have a
  different return values for c_GetFileDir than
  yours!

function EmptyFolderDeleter( options )
    local root = options.path

    local walk = function( path )

        --cycle through the current directory, walking directories
        for dir in c_GetDir( path ) do
            walk( path..dir.."/" )
        end

        --if we've got this far then we're at the bottom of the tree
        --there are no more directories to visit
        --now check if there is ANYTHING (file or folder) in the current folder
        local empty = true
        for isdir, file in c_GetFileDir( path ) do
            empty = false
            break
        end

        --now check the 'empty' flag and if the folder is the root
        if empty and path ~= root then
            c_RemoveFileDir( path )
        end
    end

    walk( root )
 end


Apply additions on a base array based on date ranges please roast my code, I hope this is the good place to look for some advice on where I can improve.

Problem definition:

Apply modifications on values depending on date ranges.

Data:

Base array - holds dictionaries in the following format:

[{'date': 20200101, 'value': 1}, {'date': 20200102, 'value': 2}]

Modifier array - holds dictionaries with similar format:

[{'date': 20200101, 'value': 1}, {'date': 20200201, 'value': 2}]

Edit: Both arrays are holding values with dates sorted in ascending order.

Goal:
Add the respective value of the modifier array to the base array lining up the date ranges. Dates are exclusive, for example when the modifier array contains 2020-01-01 you have to add the value '1' to all values in the base array that have a date less than 2020-01-01. Base array has a lot of elements while modifier array relatively few. In practice this splits the base array into a couple of date ranges. If the last modification date is less than the date in base array no modification is required.

My solution:

This is assuming the comparing of dates will work, I have translated this from perl for an easier read.

mod_index = 0
mod_size = len(mod_arr)

for elem in base_arr:
  if elem['date'] > mod_arr[mod_size - 1]['date']:
    break
  else:
    if elem['date'] < mod_arr[mod_index]['date']:
      elem['value'] += mod_arr[mod_index]['value']
    else:
      elem['value'] += mod_arr[mod_index + 1]['value']
      mod_index += 1

To preface, PEP8 recommends 4 spaces instead of 2 spaces for indentation, so I've written all below code as such.

There is a subtle bug in your program (and may be nonexistent at all, the requirements are ambiguous here), but:

if elem['date'] < mod_arr[mod_index]['date']:
    elem['value'] += mod_arr[mod_index]['value']
else:
    elem['value'] += mod_arr[mod_index + 1]['value']
    mod_index += 1

assumes that mod_array[mod_index + 1]["date"] > mod_array[mod_index]["date"], though your requirements never say that "date" is strictly increasing (and mod_array being sorted doesn't imply this).

We can handle this by changing the if / else to a while:

while elem['date'] >= mod_arr[mod_index]['date']:
    mod_index += 1

elem['value'] += mod_arr[mod_index]['value']

Note that this also increments mod_index before the mutation of elem["value"], allowing us to de-duplicate its mutation.

Since we are only moving forward in mod_arr, we actually don't need mod_index at all. We can do mod_array_iter = iter(mod_array), and replace mod_index += 1 with mod = next(mod_array_iter). This also helps us avoid any off-by-one errors.

Another benefit of using iter here is it opens up an easy way to exit the function if mod_array is empty (implying no mutations to base_arr should be done, the original program didn't account for this) or if we've traversed through all of mod_array_iter (implying necessary mutations to base_arr have been done, original program accounted for this with the if and inner break). We can use a try / except for both of those cases, and get rid of the initial if with inner break.

Final code ends up looking like (wrapped is a function for reusability):

def mutate_base_arr(base_arr, mod_arr):
    mod_arr_iter = iter(mod_arr)

    try:
        mod = next(mod_arr_iter)

        for elem in base_arr:
            while elem['date'] >= mod['date']:
                mod = next(mod_arr_iter)

            elem['value'] += mod['value']
    except StopIteration:
        return

If you really wanted to take this a step further, you could get rid of the while and duplicate nexts by using itertools, yielding this version of the function:

def mutate_base_arr(base_arr, mod_arr):
    mod_arr_iter = iter(mod_arr)

    try:
        for elem in base_arr:
            valid_mod = next(mod for mod in mod_arr_iter if elem["date"] < mod["date"])
            mod_arr_iter = itertools.chain([valid_mod], mod_arr_iter)

            elem['value'] += valid_mod['value']
    except StopIteration:
        return

Though I'm not sure if I'd recommend this, as having to prepend valid_mod with mod_arr_iter each iteration so it can be re-checked next iteration of the for is a bit unusual.


Count number of bits to convert one integer to another integer Return the number of bits (N) that will need to be changed in order to convert an integer(X), into another integer(Y).

  Constraint: 0 <= X <= Y <= 2**128

  Example:

  X = 1, Y = 2 => N = 2

  X = 2, Y = 3 => N = 1

  X = 0, Y = 4 => N = 1

I have written following code, how to improve it?

def bit_diff(n1,n2):
    cmn_d = n1 & n2
    all_d = n1 | n2
    diff = all_d - cmn_d
    cnt = 0
    while diff:
        if diff & 1:
            cnt+=1
        diff>>=1
    return cnt

print bit_diff(1,2)

cmn_d = n1 & n2
all_d = n1 | n2
diff = all_d - cmn_d

There is an operator for that:

diff = n1 ^ n2

Having found the bits that differ, you want to count the set bits of diff, also known as the population count or the Hamming weight. The approach you use is simple and valid, but there are higher performing approaches which parallelise it (see here and following sections).

However, the standard approaches for the fastest parallel implementations without direct hardware support are based on fixed-width integers, and they typically go to 32 bits or 64 bits; since your specification says that you want to support 128-bit integers, you'll have to extrapolate, although it's not that hard.


Separate scrap and production by area I'm looking for feedback on things that can be streamlined in each section. The data it creates is all needed whether it's used in a graph or not so I can't get rid of anything, just combine steps maybe.

import pandas as pd
from matplotlib import pyplot as plt
import seaborn as sns
data = pd.read_csv("C:\\Users\\eb2547\\Desktop\\python code for work\\2017 TB.csv")

# Scrap separation
noNan = data[['Quantity','Scrap Cause']].dropna()
scrap = noNan.groupby('Scrap Cause').sum()

# Hanging production
hangdata = data[(data['Transaction Description'].str.contains('INVM') == True) &
                (data['Location No'] == 'SF HANK STAGING')]
hangprod = hangdata['Quantity'].sum()

# Tapered production

# Wrapper production
wrapdatatobug = data[(data['Transaction Description'] == 'INVM-ISS') &
                (data['Location No'].str.contains('BUG') == True)]
wrapprodtobug = wrapdatatobug['Quantity'].sum()
wrapdatatobox = data[(data['Transaction Description'] == 'OOREC') &
                     (data['Location No'].str.contains('BOX ASSEMBLY-WRAP') == True)]
wrapprodtobox = wrapdatatobox['Quantity'].sum()
wrapscrap = data[(data['Scrap Cause'].str.contains('W') == True)]
wrapscraptotal = wrapscrap['Quantity'].sum()
wrapprodtotal = wrapprodtobug + wrapprodtobox

# Cut Piece production
cpdata = data[(data['Transaction Description'] == 'OOREC') &
               (data['Location No'].str.contains('BOX ASSEMBLY-CP') == True)]
cpprod = cpdata['Quantity'].sum()
cpscrap = data[(data['Scrap Cause'].str.contains('C') == True)]
cpscraptotal = cpscrap['Quantity'].sum()

# Graphs of scrap data
wrapscrap2 = scrap[(scrap.index.str.contains('W') == True)]
cpscrap2 = scrap[(scrap.index.str.contains('C') == True)]
spinscrap = scrap[(scrap.index).str.contains('S') == True]
def Wrap_Scrap():
    fix, ax = plt.subplots(figsize=(10,4))
    sns.barplot(data=wrapscrap2,x=wrapscrap2.index,y='Quantity')
def CP_Scrap():
    fix, ax = plt.subplots(figsize=(10,4))
    sns.barplot(data=cpscrap2,x=cpscrap2.index,y='Quantity')

# Graph of production
prodoverview = pd.DataFrame([hangprod,wrapprodtotal,cpprod],
                            index=['Hanging','Wrapping','CP'],
                            columns=['Quantity'])
def Prod_Graph():
    fix, ax = plt.subplots(figsize=(10,4))
    sns.barplot(data=prodoverview,x=prodoverview.index,y='Quantity')

efficiency

Overall, the code looks idiomatic and sensible, no major issues. But you want to tweak it for efficiency. Here are some things you might try.

Most importantly, instrument elapsed time, so you can tell if a tweak is useful:

    t0 = time.time()
    ...
    elapsed = time.time() - t0

And definitely use the profiler, so you know where to focus your efforts.

Ok, with that in hand, it looks like you need to synthesize some boolean columns. There are several columns that you repeatedly inspect, e.g. Transaction Description. Perhaps you could add them to the spreadsheet, to avoid repeatedly processing unchanging data? Another approach would be to build up a list of rows and then do data = pd.DataFrame(rows). This would let you examine each Transaction Description once and derive a couple of columns. But frankly, I'm skeptical this would improve your timings at all, the code looks fine as is.

style

Do use a pep8 linter: flake8, and follow its advice.

In hard-to-read identifiers like spinscrap, introduce an underscore so it is clear to the Gentle Reader where word breaks are.


Simple CubeMap opengl wrapper class I recently added this CubeMap class to my engine to make handling skyboxes easier. I would like to ask if my OpenGL coding semantics are good, and if I could improve this class in any way.

CubeMap.h:

/*
 * CubeMap.h
 *
 *  Created on: Aug 9, 2015
 *      Author: mattmatt
 */

#pragma once

#include <GL/glew.h>
#include <string>

static const GLenum GL_TEXTURE_CUBE_MAP_TYPES[6] =
                    {
                            GL_TEXTURE_CUBE_MAP_POSITIVE_X,
                            GL_TEXTURE_CUBE_MAP_NEGATIVE_X,
                            GL_TEXTURE_CUBE_MAP_POSITIVE_Y,
                            GL_TEXTURE_CUBE_MAP_NEGATIVE_Y,
                            GL_TEXTURE_CUBE_MAP_POSITIVE_Z,
                            GL_TEXTURE_CUBE_MAP_NEGATIVE_Z,
                    };

class CubeMap {
public:
    CubeMap(
                const char* posXFile,
                const char* negXFile,
                const char* posYFile,
                const char* negYFile,
                const char* posZFile,
                const char* negZFile
            );
    ~CubeMap();

    void Bind(GLenum unit = 0);

    void operator=(const CubeMap& other) = delete;
    CubeMap(const CubeMap& other) = delete;

private:
    GLuint m_texture;
    std::string m_fileNames[6];
};

CubeMap.cpp:

/*
 * CubeMap.cpp
 *
 *  Created on: Aug 9, 2015
 *      Author: mattmatt
 */

#include <alpha/CubeMap.h>
#include "alpha/libs/stb_image.h"
#include "alpha/LogManager.h"
#include <cassert>

CubeMap::CubeMap    (
                        const char* posXFile,
                        const char* negXFile,
                        const char* posYFile,
                        const char* negYFile,
                        const char* posZFile,
                        const char* negZFile
                    )
    :   m_texture(0)
{

    m_fileNames[0] = posXFile;
    m_fileNames[1] = negXFile;
    m_fileNames[2] = posYFile;
    m_fileNames[3] = negYFile;
    m_fileNames[4] = posZFile;
    m_fileNames[5] = negZFile;

    glGenTextures(1, &m_texture);
    glBindTexture(GL_TEXTURE_CUBE_MAP, m_texture);

    for (unsigned i = 0; i < (sizeof(GL_TEXTURE_CUBE_MAP_TYPES) / sizeof(GL_TEXTURE_CUBE_MAP_TYPES[0])); ++i){
        int w, h, bytesPerPixel;
        unsigned char*  data = stbi_load((m_fileNames[i].c_str()), &w, &h, &bytesPerPixel, 4);

        if(data == nullptr){
            LOG_ERROR_INFO("CubeMap(Texture)", "Failed to load Texture from file", m_fileNames[i]);
                assert(0);
        }
        glTexImage2D(GL_TEXTURE_CUBE_MAP_TYPES[i], 0, GL_RGB, w, h, 0, GL_RGBA, GL_UNSIGNED_BYTE, data);
        stbi_image_free(data);
    }

    glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
    glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
    glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
    glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
    glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_WRAP_R, GL_CLAMP_TO_EDGE);

    LOG_INFO("CubeMap(Texture)",  ": Has init successfully : ID is", m_texture );
}

CubeMap::~CubeMap() {
    if(m_texture != 0){
        glDeleteTextures(1, &m_texture);
        LOG_INFO("CubeMap(Texture)",  ": Delete texture object successfully : ID was", m_texture );
    }
}

void CubeMap::Bind(GLenum unit){
    assert(unit >= 0 && unit <= 31);
    glActiveTexture(unit);
    glBindTexture(GL_TEXTURE_CUBE_MAP, m_texture);
}

It looks fine. Simple but gets the job done. A few things to look into:

GL_TEXTURE_CUBE_MAP_TYPES constant array could be moved to the source file, to hide that implementation detail, assuming it is not meaningful outside the class. I would also use a different name. It looks too much like an OpenGL constant, so it might be misleading. ALL_UPPERCASE is also usually associated with macro names. Not the case here.
This #include <alpha/CubeMap.h>, it should probably be between quotes, since it is a local header file of your project.
Since you are storing all those pos...File parameters of the constructor into std::strings, you might as well take an array of strings, or even a vector/array to be more C++-strict. Or at least replace the char*s by strings.
The correct signature of the assignment operator is:

CubeMap& operator = (const CubeMap& other) = delete;
^^^^^^^^

Returning a reference to the class type. Even though deleted, declare it properly to make sure the compiler recognises it as the default assignment op.
To avoid having to write the lengthy and error prone (sizeof(GL_TEXTURE_CUBE_MAP_TYPES) / sizeof(GL_TEXTURE_CUBE_MAP_TYPES[0])) you can use std::array (C++11), which has a size() method, or define a template function to make it less verbose:

template<class T, std::size_t N>
constexpr std::size_t arrayLength(const T (&)[N])
{
    return N;
}

That assert(0) in the error case after stbi_load seems a bit haphazard. If the image fails to load on "release", you'll pass a null pointer to glTexImage2D. That will still allocate a texture image with undefined contents. That's not really helpful to the user. The error is already being logged, so you could either set a default placeholder image to visually signify the error, or simpler, just throw an exception and let the caller decide what to do.
That 31 is very hardcoded in assert(unit >= 0 && unit <= 31). There's actually a maximum value you can query with glGetInteger, which I don't recall now, but it is quite large, so you can indeed be lazy and use a constant instead if you want. But it would still be better to define a MaxTextureUnits somewhere to make the value self-explanatory for the non-OpenGL versed.


Looking for meta data which matches certain criteria and was created after a certain date I've got the below SQL statement which runs in MySQL on a WordPress installation. I'm looking for meta data that matches certain criteria and was created after a certain date.

However, WordPress doesn't store the date that metadata was added to posts, so I've created the metadata itself (wpcf-post-likes), the date it was added (wpcf-post-like-date) and a third piece of metadata which is created at the same time that contains the IDs of the previous two delimited by an exclamation mark (wpcf-post-like-date-link).

I then use the following SQL statement to get likes on posts written by the current author (example ID included in the SQL code below) after a certain date. It works, but it takes about 7 - 8 seconds to run, which is far from ideal. Is there a more efficient version of the same statement?

SELECT `meta_id` FROM `wp_postmeta`
WHERE   `post_id` IN (
            SELECT `ID` FROM `wp_posts` WHERE post_author = $user_id
        ) AND
        `meta_value` IN (

        SELECT CONCAT(combined_ids_a.`meta_id`, '!', combined_ids_b.`meta_id`) AS `combined_meta_id` FROM (

            SELECT `meta_id`, 'like_id' AS `meta_type` FROM `wp_postmeta`
            WHERE `meta_key` LIKE 'wpcf-post-likes'
            AND `meta_value` NOT LIKE '1837'
            AND `meta_id` IN
            (SELECT SUBSTRING_INDEX(`meta_value`, '!', 1) FROM `wp_postmeta` WHERE `meta_key` LIKE 'wpcf-post-like-date-link' ORDER BY `meta_id` DESC)

            UNION

            SELECT  `meta_id`, 'like_date' AS `meta_type`  FROM `wp_postmeta`
            WHERE `meta_key` LIKE 'wpcf-post-like-date'
            AND `meta_value` > '01-02-2016 09:20:34'
            AND `meta_id` IN
            (SELECT SUBSTRING_INDEX(`meta_value`, '!', -1) FROM `wp_postmeta` WHERE `meta_key` LIKE 'wpcf-post-like-date-link' ORDER BY `meta_id` DESC)

        ) AS combined_ids_a JOIN

        (

            SELECT `meta_id`, 'like_id' AS `meta_type` FROM `wp_postmeta`
            WHERE `meta_key` LIKE 'wpcf-post-likes'
            AND `meta_value` NOT LIKE '1837'
            AND `meta_id` IN
            (SELECT SUBSTRING_INDEX(`meta_value`, '!', 1) FROM `wp_postmeta` WHERE `meta_key` LIKE 'wpcf-post-like-date-link' ORDER BY `meta_id` DESC)

            UNION

            SELECT  `meta_id`, 'like_date' AS `meta_type`  FROM `wp_postmeta`
            WHERE `meta_key` LIKE 'wpcf-post-like-date'
            AND `meta_value` > '01-02-2016 09:20:34'
            AND `meta_id` IN
            (SELECT SUBSTRING_INDEX(`meta_value`, '!', -1) FROM `wp_postmeta` WHERE `meta_key` LIKE 'wpcf-post-like-date-link' ORDER BY `meta_id` DESC)

        ) AS combined_ids_b

        );

Suggest staring with this:

AND `meta_id` IN
        (SELECT SUBSTRING_INDEX(`meta_value`, '!', 1)
         FROM `wp_postmeta`
         WHERE `meta_key`
         LIKE 'wpcf-post-like-date-link'
         ORDER BY `meta_id`
         DESC)

LIKE text searching, sorting with the ORDER, additional subquery on the same wp_postmeta table are sources of the lengthy query execution. Rewriting subqueries as joins


Checking percentage of free memory using top and awk The following Awk code was created in order to verify if free memory from the top command is less than 20% of the total.

I've noticed that I actually do not need the top -n1 | grep Mem |, because the FREEM and TOTALM parameters are inside the third awk line.

But on the other hand, I could not run the Awk as:

 awk -v FREEM=$FREE_ME MORY -v TOTALM=$TOTAL_MEMORY '{per=int(FREEM)/int(TOTALM)*100; if(per<=20) print "FAIL" ; else print "OK"}'

My current code is:

 TOTAL_MEMORY=` top -b -n1 | grep Mem: | awk '{print $2}' `

 FREE_MEMORY=` top -n1 | awk -v RS="[, ]" '/free/{print a}{a=$0}' | head -1 `

 top -n1 | grep Mem | awk -v FREEM=$FREE_MEMORY -v TOTALM=$TOTAL_MEMORY '{per=int(FREEM)/int(TOTALM)*100; if(per<=20) print "FAIL" ; else print "OK"}'

I have a feeling that this code is ugly because of the top -n1 | grep Mem |.  Are there any suggestions on running the awk without it?

On my test system, the output of top -b -n1 | head looks like this:

top - 09:33:08 up 9 days, 23:07,  7 users,  load average: 0.01, 0.10, 0.13
Tasks: 125 total,   1 running, 124 sleeping,   0 stopped,   0 zombie
Cpu(s):  0.2%us,  0.0%sy,  0.0%ni, 99.7%id,  0.0%wa,  0.0%hi,  0.0%si,  0.0%st
Mem:   4054892k total,  3938964k used,   115928k free,       56k buffers
Swap: 10485756k total,   536176k used,  9949580k free,   438836k cached

  PID USER      PR  NI  VIRT  RES  SHR S   %CPU %MEM    TIME+  COMMAND
    1 root      20   0 10548   36    8 S      0  0.0   0:02.69 init
    2 root      20   0     0    0    0 S      0  0.0   0:00.06 kthreadd
    3 root      20   0     0    0    0 S      0  0.0   0:01.22 ksoftirqd/0

I suppose yours is similar.
From your code it looks like you are extracting the values from the Mem: line,
in this example 4054892k and 115928k.
Those values are in the 2nd and 6th column (in Awk terms), so you can get them easily in one command with:

top -b -n1 | awk '/^Mem:/ {total=int($2); free=int($6);}'

Now it's easy to add the percentage calculation and print the result:

top -b -n1 | awk '/^Mem:/ {total = $2; free = $6; per = free / total * 100; print per <= 20 ? "FAIL" : "OK"; exit}'

Some additional points of interest:

After printing, we exit to stop processing unnecessarily the rest of the output coming from top
The ternary operator ?: can simplify the printing, no need for an if-else
I dropped the int(...) calls, as Awk seems to drop the trailing non-numeric values anyway
The pattern /^Mem:/ is more strict than in the original code, just to be safe

UPDATE

If you the position of free memory can be in different columns depending on the system, you have (at least) 2 options to work around that:

If you can know the position in advance (by detecting based on the system / runtime environment), you could pass that in as a variable, for example:

i=6; top -b -n1 | awk -v i=$i '/^Mem:/ {total = $2; free = $i; per = free / total * 100; print per <= 20 ? "FAIL" : "OK"; exit}'

Or, you could use regular expressions, for example:

top -b -n1 | awk '/^Mem:/ {total = $2; match($0, /[0-9]+[a-z] free/); free = substr($0, RSTART); per = free / total * 100; print per <= 20 ? "FAIL" : "OK"; exit}'


Format conversion of localization files | 3txt to xliff I'm trying to build an xliff for localization from 3 specific files: one contains a list of IDs, the other a list of source strings and the last, a list of translated strings.

Basically, each file contains 200,000 strings, and the process is taking so much time. How can I speed up this loop?

I use sed to replace < with &lt; and the same for >. If you have better ideas, please tell me.

FILE_ID=$1
FILE_SOURCE=$2
FILE_TARGET=$3
TOT_STRING=$(wc -l < $FILE_ID)
time=$(date +"%Y-%m-%d %H:%M")

echo "<?xml version='1.0' encoding='utf-8'?>"
echo "<xliff xmlns=\"urn:oasis:names:tc:xliff:document:1.1\" version=\"1.1\">"
echo " <file source-language=\"en-US\" datatype=\"plaintext\">"
echo "    <body>"
echo "      <trans-unit xml:space=\"preserve\" id=\"1\" approved=\"no\" restype=\"x-gettext-domain-header\">"
echo "        <source>Project-Id-Version: 1.0"
echo " Report-Msgid-Bugs-To: email@example.com"
echo "POT-Creation-Date: $time+0200"
echo "PO-Revision-Date: $time+0200"
echo "Last-Translator: JohnnyKing"
echo "Language-Team: JohnnyKing"
echo "MIME-Version: 1.0"
echo "Content-Type: text/plain; charset=UTF-8"
echo "Content-Transfer-Encoding: 8bit"
echo "X-Generator: csv2xliff.sh"
echo "</source>"
echo "        <target state=\"translated\">Project-Id-Version: 1.0"
echo " Report-Msgid-Bugs-To: email@example.com"
echo "POT-Creation-Date: $time+0200"
echo "PO-Revision-Date: $time+0200"
echo "Last-Translator: JohnnyKing"
echo "Language-Team: JohnnyKing"
echo "MIME-Version: 1.0"
echo "Content-Type: text/plain; charset=UTF-8"
echo "Content-Transfer-Encoding: 8bit"
echo "X-Generator: csv2xliff.sh"
echo "</target>"
echo "      </trans-unit>"

    COUNTER=1

        while [  "$COUNTER" -le "$TOT_STRING" ]; do

        ROW_ID=$(sed -n $(( $COUNTER ))p $FILE_ID)
        ROW_SOURCE=$(sed -n $(( COUNTER ))p $FILE_SOURCE)
        ROW_TARGET=$(sed -n $(( COUNTER ))p $FILE_TARGET)

        if [ "$ROW_SOURCE" = "$ROW_TARGET" ]; then
            echo "      <trans-unit xml:space=\"preserve\" id=\"$ROW_ID\">"
            echo "        <source>$(echo $ROW_SOURCE | sed 's/</\&lt;/g' | sed 's/>/\&gt;/g')</source>"
            echo "        <target></target>"
            echo "      </trans-unit>"
        else
            echo "      <trans-unit xml:space=\"preserve\" id=\"$ROW_ID\" approved=\"yes\">"
            echo "        <source>$(echo $ROW_SOURCE | sed 's/</\&lt;/g' | sed 's/>/\&gt;/g')</source>"
            echo "        <target state=\"translated\">$(echo $ROW_TARGET | sed 's/</\&lt;/g' | sed 's/>/\&gt;/g')</target>"
            echo "      </trans-unit>"
        fi

        COUNTER=$(( $COUNTER + 1 ))
        done

echo "       </trans-unit>"
echo "    </body>"
echo "  </file>"
echo "</xliff>"

exit

Improving speed

For each line, running 3 sed commands to extract the n-th line from 3 files,
and then running further 2-4 sed commands is of course slow.

My first recommendation would be to implement this in another scripting language, say, Python.

If you really want to do this in Bash, you could:

Combine the 3 files into one, with their lines interleaved. That is, take the 1st line from each file, then the 2nd from each file, and so on. And then in each iteration of your loop, read 3 lines.
Instead of transforming the < and > by running sed for each line, run sed just once for the entire input

If we can assume that all 3 files have the same number of lines,
then you can create the input with lines interleaved like this:

paste -d '\n' "$FILE_ID" "$FILE_SOURCE" "$FILE_TARGET"

When replacing < and > using sed, you can do it with a single sed command using multiple -e flags, like this:

sed -e 's/</\&lt;/g' -e 's/>/\&gt;/g'

Putting it together:

paste -d '\n' "$FILE_ID" "$FILE_SOURCE" "$FILE_TARGET" | sed -e 's/</\&lt;/g' -e 's/>/\&gt;/g' | \
for ((COUNTER = 1; COUNTER <= TOT_STRING; ++COUNTER)); do
    read ROW_ID
    read ROW_SOURCE
    read ROW_TARGET
    # ...
done

Simplify

This is unnecessarily complicated:

ROW_ID=$(sed -n $(( $COUNTER ))p $FILE_ID)
ROW_SOURCE=$(sed -n $(( COUNTER ))p $FILE_SOURCE)
ROW_TARGET=$(sed -n $(( COUNTER ))p $FILE_TARGET)

You can write much simpler:

ROW_ID=$(sed -n ${COUNTER}p $FILE_ID)
ROW_SOURCE=$(sed -n ${COUNTER}p $FILE_SOURCE)
ROW_TARGET=$(sed -n ${COUNTER}p $FILE_TARGET)

Counting loops in Bash

Instead of this:

COUNTER=1
while [ "$COUNTER" -le "$TOT_STRING" ]; do
    # do something
    COUNTER=$(( $COUNTER + 1 ))
done

This is equivalent, but cleaner and simpler:

for ((COUNTER = 1; COUNTER <= TOT_STRING; ++COUNTER)); do
    # do something
done

Naming

TOT_STRING is a strange name for a variable with an integer value.

Printing large text

Instead of this:

echo "<?xml version='1.0' encoding='utf-8'?>"
echo "<xliff xmlns=\"urn:oasis:names:tc:xliff:document:1.1\" version=\"1.1\">"
echo " <file source-language=\"en-US\" datatype=\"plaintext\">"
echo "    <body>"
echo "      <trans-unit xml:space=\"preserve\" id=\"1\" approved=\"no\" restype=\"x-gettext-domain-header\">"
echo "        <source>Project-Id-Version: 1.0"
echo " Report-Msgid-Bugs-To: email@example.com"
echo "POT-Creation-Date: $time+0200"
echo "PO-Revision-Date: $time+0200"
echo "Last-Translator: JohnnyKing"
echo "Language-Team: JohnnyKing"
echo "MIME-Version: 1.0"
echo "Content-Type: text/plain; charset=UTF-8"
echo "Content-Transfer-Encoding: 8bit"
echo "X-Generator: csv2xliff.sh"
echo "</source>"
echo "        <target state=\"translated\">Project-Id-Version: 1.0"
echo " Report-Msgid-Bugs-To: email@example.com"
echo "POT-Creation-Date: $time+0200"
echo "PO-Revision-Date: $time+0200"
echo "Last-Translator: JohnnyKing"
echo "Language-Team: JohnnyKing"
echo "MIME-Version: 1.0"
echo "Content-Type: text/plain; charset=UTF-8"
echo "Content-Transfer-Encoding: 8bit"
echo "X-Generator: csv2xliff.sh"
echo "</target>"
echo "      </trans-unit>"

A simpler way to write:

cat << EOF
<?xml version='1.0' encoding='utf-8'?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.1" version="1.1">
 <file source-language="en-US" datatype="plaintext">
    <body>
      <trans-unit xml:space="preserve" id="1" approved="no" restype="x-gettext-domain-header">
        <source>Project-Id-Version: 1.0
 Report-Msgid-Bugs-To: email@example.com
POT-Creation-Date: +0200
PO-Revision-Date: +0200
Last-Translator: JohnnyKing
Language-Team: JohnnyKing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-Generator: csv2xliff.sh
</source>
        <target state="translated">Project-Id-Version: 1.0
 Report-Msgid-Bugs-To: email@example.com
POT-Creation-Date: +0200
PO-Revision-Date: +0200
Last-Translator: JohnnyKing
Language-Team: JohnnyKing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-Generator: csv2xliff.sh
</target>
      </trans-unit>
EOF

Indentation

The indentation here is odd:

echo "X-Generator: csv2xliff.sh"
echo "</target>"
echo "      </trans-unit>"

    COUNTER=1

        while [  "$COUNTER" -le "$TOT_STRING" ]; do

        ROW_ID=$(sed -n $(( $COUNTER ))p $FILE_ID)
        ROW_SOURCE=$(sed -n $(( COUNTER ))p $FILE_SOURCE)
        ROW_TARGET=$(sed -n $(( COUNTER ))p $FILE_TARGET)

        if [ "$ROW_SOURCE" = "$ROW_TARGET" ]; then
            echo "      <trans-unit xml:space=\"preserve\" id=\"$ROW_ID\">"
            echo "        <source>$(echo $ROW_SOURCE | sed 's/</\&lt;/g' | sed 's/>/\&gt;/g')</source>"
            echo "        <target></target>"
            echo "      </trans-unit>"
        else
            echo "      <trans-unit xml:space=\"preserve\" id=\"$ROW_ID\" approved=\"yes\">"
            echo "        <source>$(echo $ROW_SOURCE | sed 's/</\&lt;/g' | sed 's/>/\&gt;/g')</source>"
            echo "        <target state=\"translated\">$(echo $ROW_TARGET | sed 's/</\&lt;/g' | sed 's/>/\&gt;/g')</target>"
            echo "      </trans-unit>"
        fi

        COUNTER=$(( $COUNTER + 1 ))
        done

echo "       </trans-unit>"
echo "    </body>"
echo "  </file>"
echo "</xliff>"

It would be more natural this way:

echo "X-Generator: csv2xliff.sh"
echo "</target>"
echo "      </trans-unit>"

COUNTER=1

while [  "$COUNTER" -le "$TOT_STRING" ]; do
    ROW_ID=$(sed -n $(( $COUNTER ))p $FILE_ID)
    ROW_SOURCE=$(sed -n $(( COUNTER ))p $FILE_SOURCE)
    ROW_TARGET=$(sed -n $(( COUNTER ))p $FILE_TARGET)

    if [ "$ROW_SOURCE" = "$ROW_TARGET" ]; then
        echo "      <trans-unit xml:space=\"preserve\" id=\"$ROW_ID\">"
        echo "        <source>$(echo $ROW_SOURCE | sed 's/</\&lt;/g' | sed 's/>/\&gt;/g')</source>"
        echo "        <target></target>"
        echo "      </trans-unit>"
    else
        echo "      <trans-unit xml:space=\"preserve\" id=\"$ROW_ID\" approved=\"yes\">"
        echo "        <source>$(echo $ROW_SOURCE | sed 's/</\&lt;/g' | sed 's/>/\&gt;/g')</source>"
        echo "        <target state=\"translated\">$(echo $ROW_TARGET | sed 's/</\&lt;/g' | sed 's/>/\&gt;/g')</target>"
        echo "      </trans-unit>"
    fi

    COUNTER=$(( $COUNTER + 1 ))
done

echo "       </trans-unit>"
echo "    </body>"
echo "  </file>"
echo "</xliff>"


Dynamically generating Strings (or other variables) I've got a Python script which is meant to launch several other shell scripts with the appropriate setup and parameters.  I construct the file paths from other variables so that it's easier to change one setting and have it apply everywhere, rather than hardcoding everything.

So, for example:

HOME_DIRECTORY = os.path.dirname(os.path.realpath(__file__))

ONHAND_ACC_NAME = 'onhand_accuracy'
SHRINK_NAME = 'shrink'
SHELL_SCRIPT_EXTENSION = '.ksh'
CONFIG_FILE_EXTENSION = '.conf'

ONHAND_ACC_SHELL_SCRIPT = HOME_DIRECTORY + '/' + ONHAND_ACC_NAME + SHELL_SCRIPT_EXTENSION
SHRINK_SHELL_SCRIPT = HOME_DIRECTORY + '/' + SHRINK_NAME + SHELL_SCRIPT_EXTENSION
ONHAND_ACC_CONFIG_FILE = HOME_DIRECTORY + '/' + ONHAND_ACC_NAME + CONFIG_FILE_EXTENSION
SHRINK_CONFIG_FILE = HOME_DIRECTORY + '/' + SHRINK_NAME + CONFIG_FILE_EXTENSION

These are all constants defined at the top of the script.  (Feel free to tell me a more Pythonic way to do this than constants, as well.  I've heard before that "Python should have no constants at all", though I'm not sure how accurate or purist that is.)

In Java, I would do something much cleaner, like use an enum to generate appropriate file paths as needed:

public enum FileType {

    SHELL_SCRIPT   (".ksh"),
    CONFIGURATION  (".conf");

    private static final String HOME_DIRECTORY = "/path/to/scripts";
    private static final String FILE_SEPARATOR = FileSystems.getDefault().getSeparator();

    private final String extension;

    private FileType(String extension) {
        this.extension = extension;
    }

    public String getFilePath(String name) {
        return HOME_DIRECTORY + FILE_SEPARATOR + name + extension;
    }
}

Then I could simply invoke this whenever I wanted to and get a file path, rather than hardcoding them as constants, like so:

FileType.SHELL_SCRIPT.getFilePath("onhand_accuracy");

Any similar tips on how I can improve this practice/technique in Python?  I know I could have a similar function in Python that uses a lot of if/elif on a string input, but that still seems much dirtier than an enum, since the calling code can pass in whatever it wants for that string.

(In the Java, I would probably also have an enum for the associated Script, but that would clutter up my example.)

For something similar to your Java solution, you could have this in filetype.py:

import os.path

class FileType(object):
    HOME_DIRECTORY = "/path/to/scripts"

    def __init__(self, extension):
        self.extension = extension

    def file_path(self, name):
        return os.path.join(self.HOME_DIRECTORY, name + self.extension)

SHELL_SCRIPT = FileType(".ksh")
CONFIGURATION = FileType(".conf")

Then the usage would be

import filetype
filetype.SHELL_SCRIPT.file_path("onhand_accuracy")


Set a UITableViewAccessoryType in a single UITableViewController used for two different purposes I have a UITableViewController that displays a list of lift names (think weightlifting) and it's used for two different things. It can be accessed from view A to select a lift that will be saved as a user default. It can also be accessed from view B to select a lift that will then filter a list of lift events. In each case, I want a checkmark on the appropriate cell to indicate the current default lift (if coming from view A) and a checkmark on the appropriate cell to indicate the currently selected filter (if coming from view B).

I created a view model for the table cells:

struct LiftCellViewModel: SelectionsRepresentable {

    let liftName: String!
    let liftsDictionary: [String: String]!

    var text: String {
        return liftName
    }

    var accessoryType: UITableViewCellAccessoryType {
        let defaultLiftUuid = UserDefaults.lift()
        let defaultLiftName = liftsDictionary[defaultLiftUuid]

        if liftName == defaultLiftName {
            return .checkmark
        } else {
            return .none
        }
    }
}

This works very, but only handles one of the scenarios (coming from view A). So, to try and get it to also work with the other scenario (coming from view B) I'm passing in a bool if the list is being used to select a filter and using that in a switch statement to determine what path to take in order to set the checkmark:

let liftName: String!
let liftsDictionary: [String: String]!
let selectForFilter: Bool!

var text: String {
    return liftName
}

var accessoryType: UITableViewCellAccessoryType {

    switch selectForFilter {
    case true:
        let filterLiftUuid = UserDefaults.logFilterLiftUuid()
        let filterLiftName = liftsDictionary[filterLiftUuid!]

        if liftName == filterLiftName {
            return .checkmark
            } else {
            return .none
            }
    case false:
        let defaultLiftUuid = UserDefaults.lift()
        let defaultLiftName = liftsDictionary[defaultLiftUuid]

        if liftName == defaultLiftName {
            return .checkmark
        } else {
            return .none
        }
    default:
        return .none
    }
}

This view is used to create each cell in didSelectRow:

override func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
    let cellViewModel: SelectionsRepresentable
    let cell = tableView.dequeueReusableCell(withIdentifier: "liftCell", for: indexPath)
    let lift = viewModel.fetchedResultsController?.object(at: indexPath) as! Lift
    let liftName = lift.liftName

    cellViewModel = LiftCellViewModel(liftName: liftName, liftsDictionary: liftsDictionary, selectForFilter: selectForFilter)

    cell.textLabel?.text = cellViewModel.text
    cell.accessoryType = cellViewModel.accessoryType

    return cell
}

This works but the switch statement seems like a lot of code to do something so simple. Is there a Swiftier way to do this?

1 You can clean switch as:

var accessoryType: UITableViewCellAccessoryType {
    if selectForFilter && liftsDictionary[UserDefaults.logFilterLiftUuid()!] != nil {
        return .checkmark
    }
    if liftsDictionary[UserDefaults.lift()] != nil {
        return .checkmark
    }
    return .none
}

2 From your code, I can assume that SelectionsRepresentable is like:

protocol SelectionsRepresentable {
    var text: String { get }
    var accessoryType: UITableViewCellAccessoryType { get }
}

Here, as name suggest, this protocol is only related to cell selection.
It has nothing to do with text.
Try to remove that variable from protocol.


Current and estimated arrival times This is my first COBOL program, and I'd like to get a critique as I am unfamiliar with best practices.

Particularly, I'd like to know how to get the input and output to be more elegant and less dependent on how the file fields are structured.

Some type of analogy to C's scanf(), getch(), or any of the Java console IO routines would be helpful.

https://gist.github.com/anonymous/5427201

        DATA DIVISION.
    WORKING-STORAGE SECTION.
    01 TIME-STORAGE.
        03 CURRENT-TIME-NUMERIC         PIC 9(8).
        03 ETA-NUMERIC PIC 9(8) REDEFINES CURRENT-TIME-NUMERIC.

        03 CURRENT-TIME REDEFINES CURRENT-TIME-NUMERIC.
            05  CURRENTHOUR     PIC 99.
            05  CURRENTMINUTE   PIC 99.
            05  FILLER          PIC 9(4).
        03 ARIVAL-TIME-NUMERIC PIC 9(8).
        03 ARIVAL-TIME REDEFINES ARIVAL-TIME-NUMERIC.
            05  ARRIVALHOUR     PIC 99.
            05  ARRIVALMINUTE   PIC 99.
            05  FILLER          PIC 9(4).
    PROCEDURE DIVISION.
    BEGIN-BUG-NORMAN.
        DISPLAY "when will you be ariving?"
        DISPLAY "HH [enter]"
        ACCEPT ARRIVALHOUR
        DISPLAY "MM [enter]"
        ACCEPT ARRIVALMINUTE
        ACCEPT CURRENT-TIME FROM TIME.
        SUBTRACT ARIVAL-TIME-NUMERIC
            FROM CURRENT-TIME-NUMERIC
            GIVING ETA-NUMERIC.
        DISPLAY "RESULTING ETA:"
        DISPLAY ETA-NUMERIC.
        DISPLAY "HOURS:"
        DISPLAY CURRENTHOUR
        DISPLAY "MINUTES:"
        DISPLAY CURRENTMINUTE

        STOP RUN.

A couple of things I noticed:

No Identification Division. "The IDENTIFICATION DIVISION has no effect on the execution of the program but is, nevertheless, required as a means of identifying the program to the computer" - Stern & Stern
There is no real reason for the first REDEFINES. Also, you seem to be using it wrong. A redefines clause is simply another way to reference the same working storage location. If you want to store different data, that will require different variables. While you can keep redefining the same variable and using it in different way, it can get messy and storing things gets harder.
While you can have level 3 variables, I have found it to be best practice to go up in increments of 5
A lot of the time, if you are working in a real COBOL environment, you will need an environment division. Clearly here you don't, but keep that in mind
You should start all variable names in the WORKING-STORAGE SECTION with WS- because if this program need to interact with any other programs, you will have a LINKAGE-SECTION with its own set of variables.
Only use periods to end paragraphs. Technically, you can use a period to end anything. This leads to messy code. Also, a period closes ALL open statements. If you have a triple nested IF and you end with one period, it will close all 3. When you get to that point, use the proper scoop terminators such as END-IF and END-PERFORM
Spacing is important as well.

The logic problems have already been address in another answer, so I will not address that here. I have included the code below with my suggestions added to it:

        INDENTIFICATION DIVISION
        PROGRAM-ID. THE-TIMER.
        AUTHOR. name.
        DATE-WRITTEN. date.
        DATE-COMPILED. date.
        *****************************************************************
        *header comment about the program change logs ect               *
        *****************************************************************

        DATA DIVISION.
        WORKING-STORAGE SECTION.

        01 TIME-STORAGE.
            05 WS-CURRENT-TIME        PIC 9(8).
               10  WS-CURRENTHOUR     PIC 99.
               10  WS-CURRENTMINUTE   PIC 99.
               10  WS-FILLER          PIC 9(4).

            05 WS-ARIVAL-TIME         PIC 9(8).
               10  WS-ARRIVALHOUR     PIC 99.
               10  WS-ARRIVALMINUTE   PIC 99.
               10  WS-FILLER          PIC 9(4).

            05 WS-ETA                 PIC 9(8).
               10 WS-ETAHOUR          PIC 99.
               10 WS-ETAMINUTE        PIC 99.
               10 WS-FILLER           PIC 9(4).

        PROCEDURE DIVISION.

            BEGIN-BUG-NORMAN.

            DISPLAY "when will you be ariving?"
            DISPLAY "HH [enter]"
            ACCEPT WS-ARRIVALHOUR
            DISPLAY "MM [enter]"
            ACCEPT WS-ARRIVALMINUTE

            ACCEPT WS-CURRENT-TIME FROM TIME
            SUBTRACT WS-ARIVAL-TIME
                FROM WS-CURRENT-TIME
                GIVING WS-ETA

            DISPLAY "RESULTING ETA: "
            DISPLAY ETA-NUMERIC
            DISPLAY "HOURS: "
            DISPLAY CURRENTHOUR
            DISPLAY "MINUTES: "
            DISPLAY CURRENTMINUTE

            IF WS-ETAHOUR > 5
               DISPLAY " It could be a while"
            END-IF

            STOP RUN.


This function for populating the attributes of a class, using a DB model and REST API Recently, I've been trying to transform my code to be more OO and adhere to SOLID principles. I also have been trying to get into the habit of making my code more testable and have been trying to implement a TDD mindset. Here is a snippet of my code from my class ChargebackReport:

/**
 * Populates the properties of this class (ChargebackReport) using CamsModel
 * which is a database model responsible for executing queries and returning
 * the required information. GeoIPUtils is a class used to populate the
 * loginIpInformation attribute by making a GET request to an API.
 */
public function generateReport() {
    $this->transactions = $this->camsModel->getLastIntTransactionsByUser($this->userId, 20);
    $this->loginHistory = $this->camsModel->getLastIntLoginsByUser($this->userId, 100000);
    $this->chargebacks = $this->camsModel->getChargebacksByUser($this->userId);
    $this->chargebackTotal = $this->camsModel->getChargebackValueByUser($this->userId);

    $geoUtils = new GeoIPUtils();
    $loginIps = $this->camsModel->getLoginIpsFromLoginHistory($this->loginHistory);
    $this->loginIpInformation = $geoUtils->getIPAddressInfo($loginIps);

    $resultsToMap = [
        "userId" => $this->camsModel->getIDUrl($this->userId),
        "user"   => $this->camsModel->getUserById($this->userId),
        "cc"     => $this->camsModel->getCCInfoByUser($this->userId, $this->chargebacks)
    ];

    foreach($resultsToMap as $result) { $this->mapResultsToProperties($result); }
}

The responsibility of this function is to populate the various properties of this class. Some properties can be directly populated from the query. Other queries return arrays of information, which needs to be parsed through and assigned via the mapResultsToProperties() method, which maps variable keys to their respective property within the class.

Now, it's apparent that this function has a lot of stuff going on, but its overall goal is to just populate the fields in the class using the CamsModel and another class called GeoIPUtils which returns information on a given IP. The function is not responsible for querying and making GET request to an API, so I delegated those responsibilities to other classes.

Am I applying the SOLID principles properly in this example?

And for my second question, how would I go about making this class unit testable? The function as is doesn't return anything, nor are any exceptions thrown which could potentially halt the execution of it. It seems like it wouldn't be beneficial to have the function return true and just making an assert akin to the following:

$this->assertTrue($chargebackReport->generateReport());

I don't know how beneficial it would be to mock every single method in the CamsModel class (I call 8 different functions from CamsModel), have it return random input, and just checking if it assigned properly.

GeoIPUtils being a utility of methods, with the CamsModel being more of an object? camsModel could then accept an instance of GeoIPUtils rather than assigning in the separate procedural logic below:

$geoUtils = new GeoIPUtils();
$loginIps = $this->camsModel->getLoginIpsFromLoginHistory($this->loginHistory);
$this->loginIpInformation = $geoUtils->getIPAddressInfo($loginIps);

CamsModel always benefit from a GeoIPUtils instance? If so could be in the constructor. If wish to keep CamsModel, GeoIPUtils responsibilities better separated, add more indirection with a separate object which uses instances of CamsModel and GeoIPUtils as needed. Refactor generateReport into a Report object which requires a CamsModel instance, a GeoIPUtils instance, and your current this state. Creating a new Report object would replace:

$this->camsModel->getLastIntTransactionsByUser($this->userId, 20);

with:

report.getTransactions();

or:

report.getTransactions(20);

If the CamsModel class changed, then your current this or class in the question code above wouldn't need to be modified. Suppose the new version of CamsModel did not have a getIDUrl method. Only Report would be concerned with the details of CamsModel. Your this would depend only upon whatever methods Report provides. GeoIPUtils and CamsModel are now divorced from your current this class.

Replace 20, 100000 and any other literals in code with meaningful constants.

report.getTransactions(USERID_LIMIT);

Writing unit tests would be done first before crafting the object or method code, except for empty placeholder stub classes and methods. Get known input, desired output or object state for the non-existent class or method. Write tests which require that output or object state.

unit tests begin with comparison for equality. Simple String or other literals of the desired output or values are compared with your new object's envisioned getter methods, if( report.getChargebackTotal() != 959.30) failTest("Charge back total invalid");. Once unit test is written, then at this point begin writing the actual code to produce that output.

Will rewrite this code repeatedly, adding new objects and methods, painfully deleting code just typed, but writing the unit tests first is well worth the effort. Add unit tests for unexpected input, boundary testing, very small, large, negative values. Use regular expressions to whitelist and reject all input before validating by blacklisting. Protect against buffer overflow, exceeding array boundaries, numeric and String types. Modify code to validate all input, all return values. Fail immediately whenever out of range or unexpected.


PHP Base64 ByteArray Decoder I have a third party system that submits what I am assuming is a byte array data from OBDII port on our fleet vehicles.

I was getting values like:

AA== (bool)
AQ== (bool)
AAAaMA== (int)
AAA0IQ== (int)
Mjow (string)
UzozLTIwNA== (string)

I know what the type is for each field but the regular base64_decode() function wouldn't decode them and neither did the online Base64 decoders.

Here is what I came up with that works, I just curious if this is the right way to decode base64 encoded bytes.

It essentially takes the input down to binary and converts the binary to the type. It works as expected but I feel like I'm over complicating it.

class Decoder
{
    /**
     * Decodes Base64 ByteArray to Number
     *
     * @param string $encoded
     * @return number
     */
    public static function getNumber($encoded)
    {
        $decoded = base64_decode($encoded);

        $byteArray = array();

        foreach(str_split($decoded) as $byte)
        {
            $byteArray[] = sprintf("%08b", ord($byte));
        }

        $raw_bytes = implode(' ', $byteArray);

        return bindec($raw_bytes);
    }

    /**
     * Decodes Base64 ByteArray to ASCII String
     *
     * @param string $encoded
     * @return string
     */
    public static function getString($encoded)
    {
        $decoded = base64_decode($encoded);

        $raw = "";

        foreach(str_split($decoded) as $byte)
        {
            $raw .= chr(bindec(sprintf("%08b", ord($byte))));
        }

        return $raw;
    }

    /**
     * Decodes Base64 ByteArray to Boolean
     *
     * CHEATER FUNCTION
     *
     * Simply interprets Base64 value against known true/false values
     *
     * Value: "AQ==" will return true everything else will be false
     *
     * @param $encoded
     * @return bool
     */
    public static function getBool($encoded)
    {
        return ($encoded == "AQ==");
    }

    /**
     * Decodes Base64 ByteArray to Boolean
     *
     * Actually does decode and comparison on byte level.
     *
     * More expensive but is a true decode.
     *
     * @param $encoded
     * @return bool
     */
    public static function zDecodeBool($encoded)
    {
        $decoded = base64_decode($encoded);

        $byteArray = array();

        foreach(str_split($decoded) as $byte)
        {
            $byteArray[] = sprintf("%08b", ord($byte));
        }

        return $byteArray[0] == "00000001";
    }
}

Your code is generally clean, you have accurate comments & variable naming is mostly consistent.

$raw_bytes and $byteArray use two different types of naming. It's best if you choose one style, and stick with it.

With return ($encoded == "AQ==") and return $byteArray[0] == "00000001", you don't have a consistent use of brackets, it's best to stick to one.

Onto code, in getNumber(), you initialise an array, build to it, and then proceed to implode it.
However, in getString(), you initialise a string, build to it, and then return it.

In getNumber(), it'd be better if you just built to the string instead of an array.

$byteArray = array();
foreach(str_split($decoded) as $byte)
{
    $byteArray[] = sprintf("%08b", ord($byte));
}
$raw_bytes = implode(' ', $byteArray);
return bindec($raw_bytes);

into:

$rawBytes = "";
foreach(str_split($decoded) as $byte)
{
    $rawBytes .= ' ' . sprintf("%08b", ord($byte));
}
return bindec($rawBytes);

Also, zDecodeBool() (could have a better name), runs nearly the same code as getString(), you could consider a compromise between the two functions.

Other than that, your code looks nice and clean.


Get username from full name - PowerShell function I would like your opinion of my PowerShell function. Does it follow conventions, and what could can I do to improve it. It is made to create LDAP friendly username from a full name in Serbian language.
function Get-UsernameFromFullName
{
    [CmdletBinding()]
    param
    (
        [Parameter(Position = 0, Mandatory = $true, ValueFromPipelineByPropertyName = $true)]
        [string]
        $FullName,

        [Parameter(Position = 1, Mandatory = $false, ValueFromPipelineByPropertyName = $true)]
        [ValidateSet('First', 'Last')]
        [string]
        $LastNamePosition = 'First'
    )

    $usernameComponents = $FullName.ToLower().Split(" ")

    switch ($LastNamePosition)
    {
        'First' { $firstName = $usernameComponents.Length -1; $lastName = 0 }
        'Last'  { $firstName = 0; $lastName = $usernameComponents.Length -1 }
        Default { $firstName = $usernameComponents.Length -1; $lastName = 0 }
    }

    $chars = ($usernameComponents.Get($firstName) + "." + $usernameComponents.Get($lastName)).ToCharArray()

    foreach($char in $chars)
    {
        switch($char)
        {
            'ć' { $segment = "c" }
            'č' { $segment = "c" }
            'đ' { $segment = "dj" }
            'š' { $segment = "s" }
            'ž' { $segment = "z" }
            default { $segment = $char }
        }
        $Username += $segment
    }
    return $Username
}

Here is a link to GitHub page of the actual .psm1 file.
This is an example of the function usage:
Get-UsernameFromFullName -FullName "Čedomir Đorđević"

cedomir.djordjevic
Get-UsernameFromFullName -FullName "JANKOV ZORAN"

zoran.jankov
Get-UsernameFromFullName -FullName "Stojić Gradimir"

gradimir.stojic
Get-UsernameFromFullName -FullName "Vučićević R Željko"

zeljko.vucicevic
Get-UsernameFromFullName -FullName "Magda Boškić" -LastNamePosition Last

magda.boskic

i think the comments cover the why of things, but please feel free to ask if you have any questions. [grin]
#region >>> fake reading in a list of names
#    in real life, use Get-Content
$InStuff = @'
Čedomir Đorđević
JANKOV ZORAN
Stojić Gradimir
Vučićević R Željko
Magda Boškić
'@ -split [System.Environment]::NewLine

<# intended output
cedomir.djordjevic
zoran.jankov
gradimir.stojic
zeljko.vucicevic
magda.boskic [lastname @ end]
#>
#endregion >>> fake reading in a list of names

function Get-UserNameFromFullName
    # your "Get-UsernameFromFullName" name leaves the "N" in "Username" in lowercase
    #    that is not consistent with your use of that in other names of items
    {

    <#
    where is the Comment Based Help?
    #>

    [CmdletBinding ()]
    Param
        (
        [Parameter (
            Position = 0,
            # the following are switches that default to "$False"
            #    that means there is no need to add "= $True"
            #    the simple presense of the attribute flips it to "$True"
            Mandatory,
            ValueFromPipeline,
            # the following requres the pipeline object contain a property named ".FullName"
            #    is that always going to be the case?
            #    if not, then the "ValueFromPipeline" above will handle bare values
            ValueFromPipelineByPropertyName
            )]
            [string]
            $FullName,

        [Parameter (
            Position = 1,
            # making this mandatory makes little sense when a default is supplied
            #Mandatory,
            # does it make any sense to use the following attribute?
            ValueFromPipelineByPropertyName
            )]
            # don't use "First" and "Last" for _position_ info when the same words are used for the name parts
            #    it's needlessly confusing
            #[ValidateSet ('First', 'Last')]
            [ValidateSet ('Start', 'End')]
            [string]
            $LastNamePosition = 'Start'
        )

    begin {}

    process
        {
        # properly supporting the pipeline requires one to have a "process" block.
        #    otherwise all code is run in a virtual "end" block ... and that does not correctly support pipeline input

        # mixing camelCase and PascalCase for variable names is confusing [*grin*]
        #    the recommended style for PoSh is PascalCase
        #$usernameComponents = $FullName.ToLower().Split(" ")
        # the 2nd sample name has a trailing space
        #    the ".Trim()" removes that
        # good practice in PoSh is to avoid using double quotes since that can trigger unwanted expansion of $Vars
        $UserNameComponents = $FullName.Trim().ToLower().Split(' ')

        switch ($LastNamePosition)
            {
            'Start' {
                # "index -1, index 0" for the $UserNameComponents skips any middle name or initial
                $UserName = '{0}.{1}' -f $UserNameComponents[-1], $UserNameComponents[0]
                }
            'End' {
                $UserName = '{0}.{1}' -f $UserNameComponents[0], $UserNameComponents[-1]
                }
            # this is a binary choice, so the "default" is not needed
            #Default { $firstName = $UserNameComponents.Length -1; $lastName = 0 }
            }

        # there is no need to assign the output to anything
        #    whatever is left unassigned will be sent out
        # there is also no need for "return"
        #    that is disrecommeded since it gives the false impression that ONLY the item to its right will be returned
        # the "-replace" operator can be chained. so can the ".Replace()" method
        #    that allows us to skip breaking things into an array of chars
        $UserName -replace
            'č', 'c' -replace
            'ć', 'c' -replace
            'đ', 'dj' -replace
            'š', 's' -replace
            'ž', 'z'

        } # end >>> process

    end {}

    } # end >>> function Get-UserNameFromFullName

run with all names, no parameters, and using the pipeline ...
$InStuff |
    Get-UserNameFromFullName

output ...
djordjevic.cedomir
zoran.jankov
gradimir.stojic
zeljko.vucicevic
boskic.magda

run with all parameters listed and only one input value ...
Get-UserNameFromFullName -FullName $InStuff[-1] -LastNamePosition End

output = magda.boskic


Find co-occurrence of elements My input consists of a list of lists (not necessary of the same length), like

[
     ['a', 'b', 'c', 'd', 'b'],
     ['a', 'd', 'e', 'b'],
     ['f', 'a', 'g', 'e', 'b', 'h']
]

and I would like to have a matrix / dictionary  that contains the co-occurences. The keys of the dictionary are the elements of the original matrix, while the values would be a counter. So in the previous example, the result would be

{
    'a' : Counter({'b': 4, 'c': 1, 'd': 2, 'e' : 2, 'f' : 1, 'g' : 1, 'h' : 1})
    ...
}

Here is my code:

import collections
def my_func(data):
    result = collections.defaultdict(collections.Counter)
    for l in data:
        for e in l:
            result[e].update([el for el in l if el is not e])
    return result
my_func(data)

This works, except the fact I am not sure it's the smartest way: when updating the Counter, I am relooping over the elements of l.

EDIT: I should probably clarify that the elements in the list are not necessary chars but Python objects. I have used single letters only for faster typing.

Consider the following potential row in your data:

'abbbbbbbbbbbbbbb'

The bare minimum amount of work necessary would be to add 15 to both results - but with your loop as written, you'd add 15 to result['a']['b'], but then add 1 to result['b']['a'] 15 times. That's less than ideal.

So let's first condense the row into a counter - and then add the counter itself:

result = collections.defaultdict(lambda: collections.defaultdict(int))

for row in data:
    counts = collections.Counter(row)
    for key_from, key_to in itertools.permutations(counts, 2):
        result[key_from][key_to] += counts[key_from] * counts[key_to]

permutations() gives us all the pairs of keys that we will need to update - without repeat to ensure that a letter does not co-occur with itself. Of course, if the row is 'abcdefgh', this won't do us any good - we still make \$N^2\$ passes. But if the row is the first one I suggested, it's a big improvement.

Regardless, this:

result[e].update([el for el in l if el is not e])

Can be improved by removing the []s:

result[e].update(el for el in l if el is not e)

update() can take an iterable, so no need to create a full list.


Rearrange content string From https://www.codeeval.com/browse/115/:

  You have a string of words and digits divided by comma. Write a program which separates words with digits. You shouldn't change the order elements.

I tried to do it using Haskell (learning Haskell).

Can you give me some good practices/shortcuts/advice on how to make this code better?

{-# LANGUAGE OverloadedStrings #-}
import qualified Data.Text as T
import System.Environment (getArgs)

main = do
    args <- getArgs
    content <- readFile(args !! 0)
    let fileLines = lines content
    mapM (\x -> putStrLn (mixedContent (generateContent (wordsWhen (==',') x)) [] [])) fileLines

data Content =  String String | Num Int deriving (Show)

generateContent :: [String] -> [Content]
generateContent [] = []
generateContent (x:xs) = if isNumeric x then [(Num (read x))] ++ generateContent xs else [String x] ++ generateContent xs

mixedContent :: [Content] -> [Int] -> [String] -> String
mixedContent  [] d w = foldl (\r x -> r ++ x) "" $ giveResults (generateCommaDelimitedList w) ["|"]  (generateCommaDelimitedList (intToChar d))
mixedContent  ((String x):xs) d w = mixedContent xs d (w ++ [x])
mixedContent  ((Num x):xs) d w = mixedContent xs (d ++ [x]) w

giveResults :: [String] -> [String] -> [String] -> [String]
giveResults [] _ [] = []
giveResults [] _ d = d
giveResults w _ [] = w
giveResults w delimitor d = w ++ delimitor ++ d

intToChar :: [Int] -> [String]
intToChar l = map (\i -> show i) l

generateCommaDelimitedList :: [String] -> [String]
generateCommaDelimitedList [] = []
generateCommaDelimitedList (x:[]) = [x]
generateCommaDelimitedList (x:xs) = (x ++ ","):(generateCommaDelimitedList xs)

isInteger s = case reads s :: [(Integer, String)] of
  [(_, "")] -> True
  _         -> False

isDouble s = case reads s :: [(Double, String)] of
  [(_, "")] -> True
  _         -> False

isNumeric :: String -> Bool
isNumeric s = isInteger s || isDouble s

-- split string equivalent
wordsWhen :: (Char -> Bool) -> String -> [String]
wordsWhen p s =  case dropWhile p s of
                    "" -> []
                    s' -> w : wordsWhen p s''
                            where (w, s'') = break p s'

EDIT: I follow advice given in the answer. I just wanted to show how the main should be to respect what the exercise was asking.

main = do
    args <- getArgs
    content <- readFile(args !! 0)
    let l = lines content
    let lf = filter (\x -> length x > 0) l
    mapM_ (putStrLn . arrange) lf

The interesting part is mapM_, which makes me able to print the lines re-arranged and ignore the result returned by the mapM

I think you've overcomplicated the problem because you're approaching it the way you would in an imperative language. I'll describe the way I would approach the problem. Notice that I do a lot of my "thinking" in GHCi!

Your code reads the file and splits it up into lines just fine, so I'll move onto the part where we handle each line of text. First, we need to split the line up into tokens, where the tokens are separated by commas. The words function is close to what we want, but it breaks at whitespace, not commas. So we'll write our own:

tokens :: String -> [String]
tokens [] = []
tokens as =
  case break (==',') as of
    (xs,[])   -> [xs]
    (xs,_:ys) -> xs:tokens ys

I put that in the program I'm writing, load it, and try it out:

λ> tokens "wombat,7,789,tiger,33"
["wombat","7","789","tiger","33"]

Now we need to separate the words from the integers. So we need a function that will tell us if a token is an integer. The problem statement implies we only need to deal with integers. So a token is an integer if it only contains digits. (The example given didn't have any spaces, but we could allow them, and even decimal points, if we want to.)

So how do we tell if a character is a digit? Hmm... there's probably a function to do that for us.The best way to answer questions of the form "Is there a Haskell function to do X" is usually:

Figure out what the type signature of the function you want would be.
Search hoogle or hayoo. If you don't find it in one, try the other. Usually it doesn't matter much whether you get the order of the input parameters exactly right, but you may need to experiment.

What would the type of the function we're interested in be? Logically, it has to be:

Char -> Bool

Unfortunately, in the case there are a lot of functions with that signature, but eventually we find isDigit in Data.Char.

λ> import Data.Char
λ> isDigit '7'
True
λ> isDigit 'w'
False

Look at the source for that function. (You'll see that it would be easy to write our own function to allow blanks or decimal points as well as digits.)

So now we have a way to tell if an individual character is valid in an integer. How do we tell if all of the characters in a token pass the test? Again, I suspect there's a function that applies a boolean test to all elements in a sequence, and tells us if they all pass. The type signature of such a function would be one of the following:

(a -> Bool) -> [a] -> Bool
[a] -> (a -> Bool) -> Bool

Checking Hoogle, we find the all function in Data.List.

λ> import Data.List
λ> all isDigit "123"
True
λ> all isDigit "wombat"
False

We could use this expression as is, but would be more readable to define a function.

isInteger :: String -> Bool
isInteger s = all isDigit s

We could also write that last line using pointfree notation.

isInteger = all isDigit

I add that to my program, reload it, and try it out:

λ> isInteger "123"
True
λ> isInteger "wombat"
False

Next we need to divide our tokens up into those that are integers, and those that aren't. Again, there's probably a function that segregates a list into those items that satisfy a test, and those that don't. It would have this signature (possibly with the arguments switched):

(a -> Bool) -> [a] -> ([a],[a])

This time a Hayoo or Hoogle search turns up break and partition. Looking at the documentation, we see that partition is what we want. Let's try it:

λ> partition isInteger ["wombat","7","789","tiger","33"]
(["7","789","33"],["wombat","tiger"])

That preserved the order of the elements, but it gave us the integers first. That's easily fixed:

λ> partition (not . isInteger) ["wombat","7","789","tiger","33"]
(["wombat","tiger"],["7","789","33"])

Let's wrap that in a function to make it more readable:

segregate :: [String] -> ([String], [String])
segregate = partition (not . isInteger)

I'm going to move a little more quickly now, because you're probably getting the hang of how to approach these problems. Next we need to format the results. Let's focus on re-inserting the comma between tokens. I'll write a dual to the tokens function:

untokens :: [String] -> String
untokens = concat . intersperse ","

Let's try it:

λ> untokens ["wombat","tiger"]
"wombat,tiger"

Now all we need to combine the two parts:

format :: ([String],[String]) -> String
format (xs,ys) = untokens xs ++ '|' : untokens ys

Let's try it out:

λ> format (["wombat","tiger"],["7","789","33"])
"wombat,tiger|7,789,33"

Now we can put all the steps together:

parse :: String -> String
parse = format . segregate . tokens

See how simple and readable the parse function is? And it works:

λ> parse "wombat,7,789,tiger,33"
"wombat,tiger|7,789,33"

EDIT: Here's the entire program, with main.

import Data.Char
import Data.List

main = do
    args <- getArgs
    content <- readFile(args !! 0)
    mapM_ (putStrLn . lines) content

tokens :: String -> [String]
tokens [] = []
tokens as =
  case break (==',') as of
    (xs,[])   -> [xs]
    (xs,_:ys) -> xs:tokens ys

untokens :: [String] -> String
untokens = concat . intersperse ","

segregate :: [String] -> ([String], [String])
segregate = partition (not . isInteger)

isInteger :: String -> Bool
isInteger = all isDigit

format :: ([String],[String]) -> String
format (xs,ys) = untokens xs ++ '|' : untokens ys

parse :: String -> String
parse = format . segregate . tokens


Searching a maze using DFS and BFS in Python 3 I solved the problem posted from Make School Trees and Maze article, which asks me to searching a maze using DFS and BFS in Python.

Here's the final output from PyGame:

I would like to ask for code review, as I paste my code snippet below: Here's my DFS and BFS solution for solve_maze.py code review for implementation.

BFS solutino looks like this:

You can find my code for generating_maze here: https://github.com/Jeffchiucp/graph-maze-problem

import maze
import generate_maze
import sys
import random

BIT_SOLUTION = 0b0000010010010110
# Solve maze using Pre-Order DFS algorithm, terminate with solution
def solve_dfs(m):
    stack = []
    current_cell = 0
    visited_cells = 1

    while current_cell != m.total_cells -1:
        print(current_cell)
        unvisited_neighbors = m.cell_neighbors(current_cell)
        if len(unvisited_neighbors) >= 1:
            # choose random neighbor to be new cell
            new_cell_index = random.randint(0, len(unvisited_neighbors) - 1)
            new_cell, compass_index = unvisited_neighbors[new_cell_index]
            # knock down wall between it and current cell using visited_cell
            m.visit_cell(current_cell, new_cell, compass_index)
            # push current cell to stack
            stack.append(current_cell)
            # set current cell to new cell
            current_cell = new_cell
            # add 1 to visited cells
            visited_cells += 1
        else:
            m.backtrack(current_cell)
            current_cell = stack.pop()
            print("run")
        m.refresh_maze_view()
    m.state = 'idle'

# Solve maze using BFS algorithm, terminate with solution
def solve_bfs(m):
    """
    create a queue
    set current cell to 0
    set in direction to 0b0000
    set visited cells to 0
    enqueue (current cell, in direction)

    while current cell not goal and queue not empty
        dequeue to current cell, in direction
        visit current cell with bfs_visit_cell
        add 1 to visited cells
        call refresh_maze_view to update visualization

        get unvisited neighbors of current cell using cell_neighbors, add to queue

    trace solution path and update cells with solution data using reconstruct_solution

    set state to 'idle'
    """
    queue = []
    cur_cell = 0
    in_direction = 0b0000
    visited_cells = 0
    queue.insert(0, (cur_cell, in_direction))
    while not cur_cell == len(m.maze_array) - 1 and len(queue) > 0:
        cur_cell, in_direction = queue.pop()
        m.bfs_visit_cell(cur_cell, in_direction)
        visited_cells += 1
        m.refresh_maze_view()
        neighbors = m.cell_neighbors(cur_cell)
        for neighbor in neighbors:
            queue.insert(0, neighbor)
    m.reconstruct_solution(cur_cell)
    m.state = "idle"

def print_solution_array(m):
    solution = m.solution_array()
    print('Solution ({} steps): {}'.format(len(solution), solution))

def main(solver='dfs'):
    current_maze = maze.Maze('create')
    generate_maze.create_dfs(current_maze)
    if solver == 'dfs':
        solve_dfs(current_maze)
    elif solver == 'bfs':
        solve_bfs(current_maze)
    while 1:
        maze.check_for_exit()
    return

if __name__ == '__main__':
    if len(sys.argv) > 1:
        main(sys.argv[1])
    else:
        main()

Kill the noise

It's great that the solution works, but it's full of elements that seem to serve no purpose, which makes it confusing and hard to read.

visited_cells is modified but never used
BIT_SOLUTION is defined but never used
A comment like # add 1 to visited cells adds no value to a code like visited_cells += 1. Avoid writing such comments.

The same goes for the # Solve maze ... comments.

The doc comment """ ... """ in solve_bfs is inappropriate. Instead of useful documentation, it's pseudo-code of the implementation. It's unnecessary.
The return statement is unnecessary at the end of a function.
Why write 0b0000 instead of simply 0?

Confusion

I'm confused by the different terminating condition in the two implementations. In one of them, reaching the goal is expressed as cur_cell == len(m.maze_array) - 1, in the other it's current_cell == m.total_cells - 1.
It's best when there's one clear way to do something.
I suggest to change the maze implementation so that the terminating condition can be expressed as m.is_goal(cell_id).

Encapsulation

The posted code is a client of the maze library (I guess your own).
It knows too much about how the maze is implemented.
It's not good that it accesses implementation details such as m.maze_array and m.total_cells.
It's not good that the client knows that the last cell in the implementation is the exit of the maze.
It's not good that the client knows that the cells of the maze are stored in a list (maze_array).
This also limits the kind of mazes that can be modeled.

If you change the maze API so that clients can check if they have reached the exit by calling m.is_goal(cell_id),
then the maze library will be free to place the exit to wherever it likes in its storage,
it wouldn't need to be the last.
The client also wouldn't know what data structure was used to store the cells of the maze,
which again would give the maze library the freedom to use whatever it likes,
and the freedom to change the underlying storage as needed,
for example to something more efficient in a future release.

Style

You can replace len(v) > 0 and len(v) >= 1 with simply v,
because a non-empty collection is truthy in Python.

Instead of while 1:,
it's more natural to write while True:.


Iterate between dates and INSERT values in a performant way I have created a query that populates a datapoint with random values.
The logic is simple: Iterate between the START and END dates and INSERT random values.

I want from this query to be very perfomant. For example populate every second of a year with values (what with this code will last ages). I am new to SQL statements of this complexity and I dont know the pitfalls and of it.

Are there some hidden areas in my code that can be improved? If I replace the random function with just a hardcoded value will it cause much boost?
Is a loop with a lot of INSERT INTO time consuming; Is there a better way to Insert (some kind of batch insert)?

DO $$
DECLARE --Variables
    NODE_ID         bigint      :=  11;   -- The node id of the datapoint
    TIMESTAMP_START TIMESTAMP   := '2018-12-06 22:00:00';
    TIMESTAMP_END   TIMESTAMP   := '2018-12-10 00:00:00';
    TS_STEP         INTERVAL    := '30 minute';

    MAX_VALUE integer := 100;

BEGIN
    LOOP
        EXIT WHEN TIMESTAMP_START > TIMESTAMP_END;

        INSERT INTO datapoint_values (dp_id, ts, datatype, source, int_value, float_value)
        VALUES (NODE_ID, TIMESTAMP_START, 2, 0, floor(random()*(MAX_VALUE+1)), 0);

        TIMESTAMP_START := TIMESTAMP_START + TS_STEP;
    END LOOP;
END $$;

Instead of a loop which inserts one row after the other you should use generate_series to create all timestamps at once.

Untested:

DO $$
DECLARE --Variables
    NODE_ID         bigint      :=  11;   -- The node id of the datapoint
    TIMESTAMP_START TIMESTAMP   := '2018-12-06 22:00:00';
    TIMESTAMP_END   TIMESTAMP   := '2018-12-10 00:00:00';
    TS_STEP         INTERVAL    := '30 minute';

    MAX_VALUE integer := 100;

BEGIN
    INSERT INTO datapoint_values (dp_id, ts, datatype, source, int_value, float_value)
    SELECT (NODE_ID, x, 2, 0, floor(random()*(MAX_VALUE+1)), 0)
    FROM generate_series(TIMESTAMP_START, TIMESTAMP_END, TS_STEP) t(x);
END $$;


Returning matching substrings, of n length, at the beginning and end of a string I am going through the Java CodingBat exercises. Here is the one I have just completed:

  Given a string, return the longest substring that appears at both the beginning and end of the string without overlapping. For example, sameEnds("abXab") is "ab".]

Here is my code:

public String sameEnds(String string) {

    int strLength = string.length();

    if (string.substring(0, strLength/2).equals(string.substring(strLength/2, strLength))) {
        return string.substring(0, strLength/2);
    }

    for (int i = 1; i < strLength; i++) {
        if (string.substring(0, i).equals(string.substring(strLength-i, strLength) )) {
            return string.substring(0, i);
        }
    }
    return "";
}

The code passes the majority of the tests without the first if statement, so that was included to ensure it passes circumstances such as an input of xxxx. Without this statement, xxxx returns x, which is incorrect.

My questions are:

Is it acceptable/correct to use this 'cautionary' if statement, or should I somehow be incorporating it into the for loop?
Would it make sense, for readability and conciseness, to be creating some variables for things like string.substring(), as I did with strLength?
Would it make sense to use a StringBuilder? I can't really see how it could be used in this particular case.
Does it make sense to use separate return statements, or would it be more useful to assign the outcome to a resultant string, and use only one return statement at the end of the method? i.e.:

Separate statements of String result = "result here";.

One statement at the bottom of return result;.

Just some answers.

  The code passes the majority of the tests without the first if statement, so that was included to ensure it passes circumstances such as an input of xxxx. Without this statement, xxxx returns x, which is incorrect.

You if-statement solves this case... and nothing more. What about xxxxxx? The problem is that you're starting with the shortest match when you should find the longest. Reversing your loop as rolfl wrote is the way to go.

Be careful with such guard conditions. They're OK for special cases, but there's no special case here.

  Would it make sense, for readability and conciseness, to be creating some variables for things like string.substring(), as I did with strLength?

It depends. If an expression gets used a lot, is complicated, or time-consuming than surely use a local variable. In this case, I probably would not.

  Would it make sense to use a StringBuilder? I can't really see how it could be used in this particular case.

No, you're building nothing.

  Does it make sense to use separate return statements, or would it be more useful to assign the outcome to a resultant string, and use only one return statement at the end of the method?

The single-point-of-return paradigm is pretty obsolete for Java. Separate returns are usually much cleaner.