Python3 conditionally moving files and string formatting I have written the below code while helping one of the colleague over the SO and this code working as desired, but I feel there may be some improvement specially on the string formatting syntax's which can be better reviewed by experts here.
Code snippet:
#!/python3/bin/python
import pandas as pd
pd.set_option('expand_frame_repr', True)
########################################################################################
## Importing SMTP for the e-mail distribution                                        ###
########################################################################################
from email.mime.text import MIMEText
from email.mime.multipart import MIMEMultipart
import smtplib
from smtplib import SMTP
import requests
import time
import glob
import os
from os import path
import shutil

# Set the timestamp for file based on the timestamp_src and timestamp_dst
timestamp_src = time.strftime("%Y%m%d")
timestamp_dst = time.strftime("%Y%m%d-%H:%M:%S")
# Set the Destination file Variable
src = "/home/user2/.working_scripts/hpe_out/"
dst = "/home/user2/script_logs/"
file_name = "hpeCriticalAlert"
outfile = (f"{src}/{file_name}.{timestamp_src}.csv")

csv_files = [f_extn for f_extn in os.listdir(src) if f_extn.endswith(".csv") and path.isfile(path.join(src, f_extn))]
for file in csv_files:
    shutil.move(path.join(f"{src}/{file}"), f"{dst}/{file}-{timestamp_dst}.log")

session = requests.Session()
url = "https://synergy.example.com/rest/login-sessions"
credentials = {"userName": "administrator", "password": "myPass"}
headers = {"accept": "application/json",
           "content-type": "application/json",
           "x-api-version": "120",
          }
response = session.post(url, headers=headers, json=credentials, verify=False)
session_id = response.json()["sessionID"]
#############################################################################################
url = "https://synergy.example.com/rest/resource-alerts"
headers = {"accept": "application/json",
           "content-type": "text/csv",
           "x-api-version": "2",
           "auth": session_id,
          }
# stream=True on the request avoids reading the content at once into memory for large responses.
response = session.get(url, headers=headers, verify=False, stream=True)
with open(outfile, 'wb') as f:
    for chunk in response.iter_content(chunk_size=1024):
        f.write(chunk)
#################################################################################################
for file in glob.glob(f"{outfile}"):
#for file in glob.glob(f"{src}/{file_name}.*.csv"):
   df = pd.read_csv(file)
##################################################################################################
recipients = ['karn.kumar@example.com']
mail_receivers = [elem.strip().split(',') for elem in recipients]
msg = MIMEMultipart()
msg['Subject'] = "HTML OneView"
msg['From'] = 'hpe-dashboard@example.com'
#########################################################################
html = """\

     Dear Team,

     Please Find the Report!

     {0}

    Kind regards.

    Synergy Global Dashboard.

  """.format(df.to_html(index=False))

part1 = MIMEText(html, 'html')
msg.attach(part1)
server = smtplib.SMTP('smtp.example.com')
server.sendmail(msg['From'], mail_receivers, msg.as_string())

Since this looks like a script that might get onto some production server at some point, I'm going to give some recommendations from that point of view and how I'd do it if I were to implement this for my employer.
In this review I'll mostly focus on the project structure, requests, exceptions and credentials management. I'll first paste the code that I came up with and then I'm going to explain in detail why did I do most of the changes.
# config.py

from getpass import getpass
import time

TIMESTAMP_SRC = time.strftime("%Y%m%d")
TIMESTAMP_DST = time.strftime("%Y%m%d-%H:%M:%S")

DST = "/home/user2/script_logs/"
SRC = "/home/user2/.working_scripts/hpe_out/"
FILE_NAME = "hpeCriticalAlert"
OUTFILE = f"{SRC}/{FILE_NAME}.{TIMESTAMP_SRC}.csv"

BASE_URL = "https://synergy.example.com/rest"
USERNAME = "administrator"
PASSWORD = getpass("Input your password: ")

# EMAIL CONFIG
SMTP_SERVER = 'smtp.example.com'

FROM = 'hpe-dashboard@example.com'
TO = ['user1@example.com', 'user2@example.com']
SUBJECT = 'HTML OneView'

EMAIL_TEMPLATE = """\

     Dear Team,

     Please Find the Report!

     {}

    Kind regards.

    Synergy Global Dashboard.

"""

# main.py

import os
import shutil
import smtplib
from email.message import EmailMessage

import pandas as pd

pd.set_option('expand_frame_repr', True)
import requests

from .config import (
    BASE_URL,
    DST,
    OUTFILE,
    PASSWORD,
    SRC,
    TIMESTAMP_DST,
    USERNAME,
    SUBJECT,
    FROM,
    TO,
    EMAIL_TEMPLATE,
    SMTP_SERVER,
)

class FileMoveFailure(Exception):
    pass

class SynergyRequestFailure(Exception):
    pass

class SessionIdRetrievalFailure(SynergyRequestFailure):
    pass

class ResourceAlertsRetrievalFailure(SynergyRequestFailure):
    pass

def move_csv_files():
    for csv_file in os.listdir(SRC):
        if csv_file.endswith(".csv") and os.path.isfile(os.path.join(SRC, csv_file)):
            try:
                shutil.move(
                    os.path.join(f"{SRC}/{csv_file}"),
                    f"{DST}/{csv_file}-{TIMESTAMP_DST}.log"
                )
            except OSError as os_error:
                raise FileMoveFailure(
                    f'Moving file {csv_file} has failed: {os_error}'
                )

def get_session_id(session):
    try:
        response = session.post(
            url=f"{BASE_URL}/login-sessions",
            headers={
                "accept": "application/json",
                "content-type": "application/json",
                "x-api-version": "120",
            },
            json={
                "userName": USERNAME,
                "password": PASSWORD
            },
            verify=False
        )
    except requests.exceptions.RequestException as req_exception:
        # you should also get this logged somewhere, or at least
        # printed depending on your use case
        raise SessionIdRetrievalFailure(
            f"Could not get session id: {req_exception}"
        )

    json_response = response.json()
    if not json_response.get("sessionID"):
        # always assume the worse and do sanity checks & validations
        # on fetched data
        raise KeyError("Could not fetch session id")

    return json_response["sessionID"]

def get_resource_alerts_response(session, session_id):
    try:
        return session.get(
            url=f"{BASE_URL}/resource-alerts",
            headers={
                "accept": "application/json",
                "content-type": "text/csv",
                "x-api-version": "2",
                "auth": session_id,
            },
            verify=False,
            stream=True
        )
    except requests.exceptions.RequestException as req_exception:
        # you should also get this logged somewhere, or at least
        # printed depending on your use case
        raise ResourceAlertsRetrievalFailure(
            f"Could not fetch resource alerts: {req_exception}"
        )

def resource_alerts_to_df(resource_alerts_response):
    with open(OUTFILE, 'wb') as f:
        for chunk in resource_alerts_response.iter_content(chunk_size=1024):
            f.write(chunk)
    return pd.read_csv(OUTFILE)

def send_email(df):
    server = smtplib.SMTP(SMTP_SERVER)
    msg = EmailMessage()

    msg['Subject'], msg['From'], msg['To'] = SUBJECT, FROM, TO
    msg.set_content("Text version of your html template")
    msg.add_alternative(
        EMAIL_TEMPLATE.format(df.to_html(index=False)),
        subtype='html'
    )

    server.send_message(msg)

def main():
    move_csv_files()

    session = requests.Session()
    session_id = get_session_id(session)

    resource_alerts_response = get_resource_alerts_response(session, session_id)
    df = resource_alerts_to_df(resource_alerts_response)

    send_email(df)

if __name__ == '__main__':
    main()

config.py
The first major thing that I did was to split apart the configuration data from the main script. This will make your life easier when it comes to modifying the values since they're altogether.
Another very important and sensitive thing to notice is the PASSWORD field. Do NOT, EVER, expose credentials in plain text. There are so many cases where this happened at big companies and resulted in loss of millions of $$ due to hackers exploiting this...
There are multiple ways of avoiding this. None is perfect but it's definitely better than storing passwords in plain-text:

Ask for user's input;
Store them as ENV VARIABLES;
Use some secrets management tool like Ansible Vault

This config file doesn't necessarily have to be a .py file. It can be a yaml, ini, .env, json file. I made it like this for convenience.
main.py
Honestly, when I first saw your script, I remembered how I used to write code when I started programming. It's messy, you understand it at the moment of writing and it kinda does the job you need. But when it comes to maintaining it, it's really, really tedious and frustrating: all those useless comments, unused imports, no spacing, nothing... You wouldn't even know where to start next time you have to add some new feature. Because of this you'd probably need to completely rewrite it.
Improvements brought by my implementation:

code is definitely easier to read and understand, even if I haven't added any docstrings.
it's a lot easier to modify any part of the code since it's now broken into smaller pieces. You can read more about the SRP principle here. I honestly could've done better on this one but for a first review, it'll do.

What is still to be improved (by you)

add some logging
add docstrings / type hints
chose better names for your variables (SRC, DST and so on aren't to descriptive)
create tests to ensure your code works (I haven't really tested my implementation but it should give you an overall idea)

1. Imports
As you can see I don't have any unused imports and it's really easy to recognise stdlib modules, 3rd party modules and local modules because of the grouping:
# stdlib
import os
import shutil
import smtplib
from email.message import EmailMessage

# 3rd party modules
import pandas as pd

pd.set_option('expand_frame_repr', True)
import requests

# local config
from .config import (
    BASE_URL,
    DST,
    OUTFILE,
    PASSWORD,
    SRC,
    TIMESTAMP_DST,
    USERNAME,
    SUBJECT,
    FROM,
    TO,
    EMAIL_TEMPLATE,
    SMTP_SERVER,
)

2. Exceptions
I've seen people creating a new exceptions.py file where they create custom exceptions for different use-cases. I just let them in main.py. Having your own subclasses lets you treat distinct exception cases separately. (such as the difference between not having permissions to run a report, and the report execution failing).
3. CSV files
Since you only want to move the .csv files to a specific location it's not necessary to build a list and iterate against it again to move each file. Instead, do it in one run and also raise an exception if something happens:
4. Requests
It's always a good practice to catch any specific exception that might be raised. I've only handled the generic RequestException which is going to catch any request exception but you might want to treat some exceptions separately: Connection errors, timeout errors, etc.
Also, always make sure the data you receive is what you expect. I've seen cases where certain APIs where changed by some provider without announcing us (the developers) or without stating in their documentation otherwise. This usually breaks everything and takes some time figuring out what the root cause is.
5. Others
I think the rest of it is pretty straight forward. I'm not saying my solution is perfect but it's a better starting point.
Enjoy! :)


Streaming modified lines of a file from a Controller I am writing a piece of software that aims at streaming the modified lines of an external file located at given uri (and retrieved as a QueryString parameter).

I am trying to minimize the impact of big files so that I can maximize the use of asynchronous calls (i.e. async / await). I am using AsyncEnumerable (https://github.com/Dasync/AsyncEnumerable) to have at the same time the feature of enumerable and and async / await, in order to free up threads and memory (I am not going to keep the whole file in memory before sending the modified version back).

I created an extension method that can fetch the lines of a remote file.

public static class StringExtensions
{
    public static AsyncEnumerable<string> ReadLinesAsyncViaHttpClient(this string uri)
    {
        return new AsyncEnumerable<string>(async yield =>
        {
            using (var httpClient = new HttpClient())
            {
                using (var responseStream = await httpClient.GetStreamAsync(uri))
                {
                    using (var streamReader = new StreamReader(responseStream))
                    {
                        while(true)
                        {
                            var line = await streamReader.ReadLineAsync();

                            if (line != null)
                            {
                                await yield.ReturnAsync(line);
                            }
                            else
                            {
                                return;
                            }
                        }
                    }
                }
            }
        });
    }
}

And used that extension method in a Controller action:

using System;
using System.Collections.Async;
using System.IO;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc;

namespace WebApplicationTest.Controllers
{
    [Route("api/[controller]")]
    [ApiController]
    public class LinesController : ControllerBase
    {
        private static readonly Random Random = new Random();

        // GET api/lines
        // POST api/lines
        [HttpGet]
        [HttpPost]
        public async Task<IActionResult> GetAsync([FromQuery] string fileUri)
        {
            using (var streamWriter = new StreamWriter(HttpContext.Response.Body))
            {
                await fileUri.ReadLinesAsyncViaHttpClient().ForEachAsync(async line =>
                {
                    // Some dumb process on each (maybe big line)
                    line += Random.Next(0, 100 + 1);
                    await streamWriter.WriteLineAsync(line);
                });
            }

            return Ok();
        }
    }
}

I used the HttpContext.Response.Body stream instead of the PushStreamContent cause according to this answer: https://stackoverflow.com/a/48115434/4636721 (in .NET Core we don't need to use the PushStreamContent if no need of HttpMessage)

It seems to work I mean, I get a payload with the lines of the initial file passed in the query string and with some random numbers at the end of each line. It seems there is no increase in memory but I would like to have another pair of eyes to check what I have done.

[EDIT 2]
Turns out there is already an issue on the ASP.NET MVC Github about that: https://github.com/aspnet/Mvc/issues/6224

A small improvement you can do to avoid the pretty deep nesting is, stacking all usings without extra brackets.

public static AsyncEnumerable<string> ReadLinesAsyncViaHttpClient(this string uri)
{
    return new AsyncEnumerable<string>(async yield =>
    {
        using (var httpClient = new HttpClient())
        using (var responseStream = await httpClient.GetStreamAsync(uri))
        using (var streamReader = new StreamReader(responseStream))
        {
            while(true)
            {
                 var line = await streamReader.ReadLineAsync();
                 if (line != null)
                 {
                     await yield.ReturnAsync(line);
                 }
                 else
                 {
                     return;
                 }
             }
        }
    });
}

Apart from that I don't know if this the kind of thing that should be a extension method. This should rather be a class.

(I'm currently do not have my hands on a pc, so I cannot comment on the technical side of things >:( )


Obtaining a list of files to be re-encoded (ideally determining the encoding just once) to_convert_files = [FileEnc(filename, getEncoding(filename)) for
                    filename in filenames if getEncoding(filename) != None]

The problem here is that getEncoding gets called twice for each accepted file name: once in the if clause and once in the expression preceding for. I know I can avoid this by splitting this into a generator expression that does the mapping and a list comprehension that does the filtering, but I was wondering if there is a way keeping it condensed in a one-liner.

Often, list comprehensions can aid readability. But here cramming all that behaviour into a single line is obfuscation. The Pythonic way is to do the simplest and most readable thing possible.

Here, this probably means writing a generator function that uses a normal for loop and skips over elements without an available encoding.

def encoded_files(files):
    for file in files:
        encoding = getEncoding(file)
        if encoding is not None:
            yield FileEnc(file, encoding)

to_convert_files = list(encoded_files(filenames))


Bank account number validation in IBAN format I wrote a JavaScript function for validating bank account numbers in IBAN format for my own joy. Could you have a look if you don't see any obvious mistakes or places for improvement?

function isIbanValid(value) {

    var lengthMap = getLengthMap();

    //cleanup
    value = value.toString().toUpperCase().replace(/\s/g, '').replace(/[-]/g, '');
    //check if alphanumeric
    if (!/^[a-zA-Z0-9]+$/.test(value)) return false;
    //extract countrycode
    var countryCode = value.substring(0, 2);
    //check if letter
    if (!/([a-z]+[\s\-]?)*/i.test(countryCode)) return false;
    //check string length
    if (value.length != lengthMap[countryCode]) return false;

    value = value.concat(value.substring(0, 4)).substring(4);
    value = value.replace(countryCode, countryCodeToStringValue(countryCode));

    return modulo(value, 97) == 1;

    function countryCodeToStringValue(countryCode) {
        return "".concat(((countryCode.charCodeAt(0)) - 55).toString() + (countryCode.charCodeAt(1) - 55).toString());
    }

    function modulo(divident, divisor) {
        var quantization = 12;
        while (quantization < divident.length) {
            var part = divident.substring(0, quantization);
            divident = (part % divisor) + divident.substring(quantization);
        }
        return divident % divisor;
    }

    function getLengthMap() {
        var lengthMap = {};
        lengthMap["AD"] = 24;
        lengthMap["AT"] = 20;
        lengthMap["AZ"] = 28;
        lengthMap["BH"] = 22;
        lengthMap["BE"] = 16;
        lengthMap["BA"] = 20;
        lengthMap["BR"] = 29;
        lengthMap["BG"] = 22;
        lengthMap["CR"] = 21;
        lengthMap["HR"] = 21;
        lengthMap["CY"] = 28;
        lengthMap["CZ"] = 24;
        lengthMap["DK"] = 18;
        lengthMap["DO"] = 28;
        lengthMap["EE"] = 20;
        lengthMap["FO"] = 18;
        lengthMap["FI"] = 18;
        lengthMap["FR"] = 27;
        lengthMap["DE"] = 22;
        lengthMap["GR"] = 27;
        lengthMap["GI"] = 23;
        lengthMap["GL"] = 18;
        lengthMap["GT"] = 28;
        lengthMap["HU"] = 28;
        lengthMap["IS"] = 26;
        lengthMap["IE"] = 22;
        lengthMap["IL"] = 23;
        lengthMap["IT"] = 27;
        lengthMap["JO"] = 30;
        lengthMap["KZ"] = 20;
        lengthMap["KW"] = 30;
        lengthMap["LV"] = 21;
        lengthMap["LB"] = 28;
        lengthMap["LI"] = 21;
        lengthMap["LT"] = 20;
        lengthMap["LU"] = 20;
        lengthMap["MK"] = 19;
        lengthMap["MT"] = 31;
        lengthMap["MR"] = 27;
        lengthMap["MU"] = 30;
        lengthMap["MC"] = 27;
        lengthMap["MD"] = 24;
        lengthMap["ME"] = 22;
        lengthMap["NL"] = 18;
        lengthMap["NO"] = 15;
        lengthMap["PK"] = 24;
        lengthMap["PS"] = 29;
        lengthMap["PL"] = 28;
        lengthMap["PT"] = 25;
        lengthMap["QA"] = 29;
        lengthMap["RO"] = 24;
        lengthMap["SM"] = 27;
        lengthMap["SA"] = 24;
        lengthMap["RS"] = 22;
        lengthMap["SK"] = 24;
        lengthMap["SI"] = 19;
        lengthMap["ES"] = 24;
        lengthMap["SE"] = 24;
        lengthMap["CH"] = 21;
        lengthMap["TN"] = 24;
        lengthMap["TR"] = 26;
        lengthMap["AE"] = 23;
        lengthMap["GB"] = 22;

        return lengthMap;
    }
}

I'm pretty sure this will fail for some IBANs. You convert the country code to digits, but I don't see you doing the same for any other letters in the IBAN - and there might well be others. The example Wikipedia gives is GB82WEST12345698765432. The "WEST" should be converted to numbers as well (same procedure as the country code; 'A' = 10, B = 11, etc.).
Not sure why getLengthMap is a function - it's basically a constant value, and might as well be declared as a simple variable.
The regexes have some redundancy.This one, /^[a-zA-Z0-9]+$/, checks for lowercase letters, though the line before has already called toUpperCase(). And the next regex accepts trailing whitespace and dashes, though those have been removed.
And while you're using regexes, you might as well use that to pick the string apart, rather than substring plus magic numbers.
Your modulo function seems fine, though there's little reason to have it accept a divisor argument. Since it's an internal helper function specifically for IBAN, it'll always be mod 97. So in this case, I might actually prefer that that was hard-coded (and the function was named mod97 or something equally explanatory).

In all, I think you can do a lot more with fewer regexes, which would help streamline the code.

To sanitize the input:

var iban = value.toUpperCase().replace(/[^A-Z0-9]/g, '');

To "parse" the IBAN:

var match = iban.match(/^([A-Z]{2})(\d{2})(.+)$/);

if match is null the code's automatically invalid. Otherwise, match[1] will be the 2-letter country code, match[2] will be the check digits, and match[3] will be the rest of the IBAN. Provided there's a match it should then rearranged as match[3] + match[1] + match[2]

Alternatively, you can check and rearrange in one go:

var code = iban.replace(/^([A-Z]{2})(\d{2})(.+)$/, "$3$1$2");

if code === iban, nothing's been rearranged, which means there wasn't a match, and the code's invalid. You'd still have to extract the country code, though, so not much is won by doing this.

Finally, to convert letters to numbers:

 var digits = iban.replace(/[A-Z]/g, function (letter) {
   return String(letter.charCodeAt(0) - 55);
 });

Here's my take

var validIBAN = (function () { // use an IIFE
  // A "constant" lookup table of IBAN lengths per country
  // (the funky formatting is just to make it fit better in the answer here on CR)
  var CODE_LENGTHS = {
    AD: 24, AE: 23, AT: 20, AZ: 28, BA: 20, BE: 16, BG: 22, BH: 22, BR: 29,
    CH: 21, CR: 21, CY: 28, CZ: 24, DE: 22, DK: 18, DO: 28, EE: 20, ES: 24,
    FI: 18, FO: 18, FR: 27, GB: 22, GI: 23, GL: 18, GR: 27, GT: 28, HR: 21,
    HU: 28, IE: 22, IL: 23, IS: 26, IT: 27, JO: 30, KW: 30, KZ: 20, LB: 28,
    LI: 21, LT: 20, LU: 20, LV: 21, MC: 27, MD: 24, ME: 22, MK: 19, MR: 27,
    MT: 31, MU: 30, NL: 18, NO: 15, PK: 24, PL: 28, PS: 29, PT: 25, QA: 29,
    RO: 24, RS: 22, SA: 24, SE: 24, SI: 19, SK: 24, SM: 27, TN: 24, TR: 26
  };

  // piece-wise mod97 using 9 digit "chunks", as per Wikipedia's example:
  // http://en.wikipedia.org/wiki/International_Bank_Account_Number#Modulo_operation_on_IBAN
  function mod97(string) {
    var checksum = string.slice(0, 2),
        fragment;

    for(var offset = 2 ; offset < string.length ; offset += 7) {
      fragment = String(checksum) + string.substring(offset, offset + 7);
      checksum = parseInt(fragment, 10) % 97;
    }

    return checksum;
  }

  // return a function that does the actual work
  return function (input) {
    var iban = String(input).toUpperCase().replace(/[^A-Z0-9]/g, ''), // keep only alphanumeric characters
        code = iban.match(/^([A-Z]{2})(\d{2})([A-Z\d]+)$/),           // match and capture (1) the country code, (2) the check digits, and (3) the rest
        digits;

    // check syntax and length
    if(!code || iban.length !== CODE_LENGTHS[code[1]]) {
      return false;
    }

    // rearrange country code and check digits, and convert chars to ints
    digits = (code[3] + code[1] + code[2]).replace(/[A-Z]/g, function (letter) {
      return letter.charCodeAt(0) - 55;
    });

    // final check
    return mod97(digits) === 1;
  };
}());


Nearly Illegible Prime Number Calculator This is my first reasonably large C# program. Considering my code looks like it was written in an esolang to me, I'm guessing I did something wrong at some point. My main priority is readability, although speed is also a plus.

using System;

public class PrimeSearcher {
    public static string Stringilate (int[] iterable) {
        string returnable="";
        foreach(int i in iterable){
            returnable+=i.ToString()+" ";
        };
        return returnable;
    }
    public static bool isPrime (int[] iterable,float target) {
        foreach(int i in iterable){
            if(target%i==0){
                return false;
            }
            if(i==0){ return true; }
            //reached the "end" (last discovered prime) of the array, hurray!
        }
        return true; //will only do anything for the last element
    }
    static public void Main () {
        int[] primes=new int[1000];
        float j=2; //float because I think it's the smallest datatype that can return a non-zero number when divided, correct me if I'm wrong
        while(primes[primes.Length-1]==0){
            if(isPrime(primes,j)){
                primes[Array.IndexOf(primes,0)]=(int)j;
            }
            j++;
        }
        Console.WriteLine(Stringilate(primes));
        //I don't *think* ToString worked when I tried it.
    }
}

By starting with j = 3 and incrementing j by 2 you could skip a lot of unneeded computations because it will skip even values.
Instead of using a float I would go with int. Both are 32-Bit and you won't need the floating point values.
You should let your variables have some space to breathe.

Consider

while(primes[primes.Length-1]==0){
    if(isPrime(primes,j)){
        primes[Array.IndexOf(primes,0)]=(int)j;
    }
    j++;
}

versus

while (primes[primes.Length - 1] == 0)
{
    if (isPrime(primes, j))
    {
        primes[Array.IndexOf(primes, 0)] = (int)j;
    }
    j += 2;
}

where the later is much more readable. It uses the bracing style most C# developer use as well.
Stringilate() could be improved as well by using string.Join() which would look like so

public static string Stringilate(int[] iterable)
{
    return string.Join(" ", iterable);
}

and as a side note, if you need to concatenate strings in a loop its much better to use a StringBuilder instead.
You should be consitent in your coding style (public static vs. static public).
You are using braces although they might be optional. Good choice !
Instead of a concrete type you could use var instead


x86 strcpy implementation I got about 4 days of assembly knowledge, so I need a review on this strcpy function and if it can be done better (at least I have the feeling).

Full code (with the test included):

.data
    s:
        .asciz "Hello world!"

.bss
    .lcomm destination, 4

.text
.globl main
main:
    nop

    pushl $s
    pushl $destination
    call __strcpy
    addl $8, %esp

    pushl $destination
    call puts
    addl $4, %esp

    ret

.globl __strcpy
.type __strcpy, @function
__strcpy:
    movl $0xFFFF, %ecx
    movl 4(%esp), %edi
    movl 8(%esp), %esi

cpy:
    cmpl $0, (%esi)
    je done

    movsb
    loop cpy

done:
    ret

GitHub

Parts that I feel can be optimized:

Because the done label just executes the ret instruction:

cmpl $0, (%esi)
je done

Because the rep instruction-family seems a like better approach:

movsb
loop cpy

You can use Bit Twidding Hack to determine if int32 or int64 has no zero bytes: http://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord

If it has not you can copy whole int32 or int64. So it will be 4 operations for searching zero byte in 8 bytes (in int64 case). It looks like true optimization.

char * strcpy(char * dst, const char * src)
{
    char * origin = dst;

    while (!((((*(uint64_t *)src) - 0x0101010101010101ULL)
           & ~(*(uint64_t *)src) & 0x8080808080808080ULL)))
    {
        *(uint64_t *)dst = *(uint64_t *)src;
        src += 8;
        dst += 8;
    }

    while (*dst++ = *src++)
       ;

    return origin;
}

Simple strcpy implementation uses 8 compares to zero and 8 byte copyings for each 8 bytes of source string. My implementation uses 4 operation for checking for zeros and 1 operation to copy for 8 bytes. So we have 5ops vs 16ops. Not all ops have same speeds so it is not easy to compare real speedup. We need some benchmarks, is anyone free?


sum of multiples by 3 or 5 using ranges This program calculates the sum of all integers in the range \$[1, 1000)\$ which are multiples of either 3 or 5 or both.
Inspired by x86-64 Assembly - Sum of multiples of 3 or 5 the other day, and also drawing from some of what I learned through Calculate the centroid of a collection of complex numbers I decided to try to tackle this using C++20 ranges.  By heavy use of constexpr, I had hoped to find a solution that would calculate everything at compile time, and indeed this does as you can see if you try it online.
This version is inspired by this talk C++20 Ranges in Practice - Tristan Brindle - CppCon 2020.
I'm interested in general improvements.
euler1.cpp
#include <iostream>
#include <concepts>
#include <ranges>
#include <iterator>
#include <functional>
#include <numeric>

template <std::ranges::input_range R,
    typename Init = std::ranges::range_value_t<R>>
constexpr Init accumulate(R&& rng, Init init = Init{})
{
    return std::reduce(std::ranges::begin(rng), std::ranges::end(rng),
        std::move(init));
}

int main() {
    constexpr auto div3_or_5 = [](int i){ return i % 3 == 0 || i % 5 == 0; };
    std::cout << accumulate(std::ranges::iota_view{1, 1000}
        | std::views::filter(div3_or_5)
        | std::views::common)
    << '\n';
}

I see you already solved your own questions from before your latest edit. Just two very minor things that can be improved: you can omit repeating the type name Init in two places:
template <std::ranges::input_range R, typename Init = std::ranges::range_value_t<R>>
constexpr auto accumulate(R&& rng, Init init = {})
{
    return std::reduce(std::ranges::begin(rng), std::ranges::end(rng), std::move(init));
}

And you don't need to use std::views::common in main().


String tokenizing with arbitrary token count in C I am building a small program for input handling so I can conveniently parse lines and tokenize strings. The token handling works right now but I have to malloc a lot in the main() to make it work. Sadly I can not malloc everything at once because then the strings inside will have NULL pointers. Maybe someone more experienced with C can help me improve this:

#include <stdio.h>

int get_tokens(char *line, char **tokens, size_t number_of_tokens) {

    // Make a copy of line to prevent it from being changed
    char *copy = malloc(strlen(line) + 1);
    if (copy == NULL) {
        perror("Could not allocate memory!");
        exit(EXIT_FAILURE);
    }
    strcpy(copy, line);

    char *delimiter = " ";
    char *token = strtok(copy, delimiter);
    if (token) {
        strcpy(*tokens, token);
        size_t tokens_found = 1;
        while ((token = strtok(NULL, delimiter)) && tokens_found < number_of_tokens) {
            strcpy(*(tokens + tokens_found), token);
            ++tokens_found;
        }
    } else {
        printf("No tokens found! The supplied string was empty.");
        return EXIT_FAILURE;
    }
    free(copy);
    return EXIT_SUCCESS;
}

int main() {

    // this will also be the buffer length for the tokens
    size_t line_length = 10;
    char line[line_length];
    get_line(line, line_length);
    // Above function works just as the name implies

    size_t tokens_wanted = 2;
    char **tokens = malloc(tokens_wanted * sizeof(char**));
    for (size_t j = 0; j < tokens_wanted; ++j) {
        tokens[j] = malloc(sizeof(char) * line_length);
    }

    get_tokens(line, tokens, tokens_wanted);

    // process the tokens
    // free the elements of tokens

    free(tokens);
    return EXIT_SUCCESS;
}

I see some things that I think could help you improve your code.

Use the required #includes

The code uses malloc which means that it should #include <stdlib.h>.  It also uses strtok which is in string.h.

Don't leak memory

This code calls malloc several places but doesn't match each one with a call to free.  This means that the routines are leaking memory. Specifically, just before the call to free(tokens); in main, you need to also free the individually allocated memory:

for (size_t j = 0; j < tokens_wanted; ++j) {
   free(tokens[j]);
}

Use const where practical

The comment about making a copy of line to prevent it from being changed suggests that it should be declared const in the parameter list:

int get_tokens(const char *line, char **tokens, size_t number_of_tokens) {

Also, the delimiter string should also be const and probably also static.

Consider a different interface

In the current code the return value of get_tokens isn't used.  It also prints and exits the program in some circumstances.  Instead, I'd advice changing the interface in a couple of ways.  First, I'd recommend that the caller make a copy of the string if necessary.  This makes the code cleaner and simpler and allows the caller to perhaps do something other than simply abruptly halting the program if an empty string is found.  Second, I'd suggest that because strtok operates by modifying the passed string, there really isn't a need to allocate more memory and copy.  Instead, I'd recommend that the pointers returned can simply point to the passed string.  The much-simplified version might look like this:

void get_tokens(char *line, char **tokens, size_t number_of_tokens) {
    static const char *delimiter = " ";
    for (size_t i = 0; i < number_of_tokens; ++i) {
        tokens[i] = strtok(line, delimiter);
        line = NULL;
    }
}

This works nicely and is much simpler.  If fewer than the requested number of tokens is found, the remaining tokens pointers are set to NULL. The calling function might look like this:

int main() {
    const size_t line_length = 100;
    char line[line_length];
    get_line(line, line_length);

    size_t tokens_wanted = 2;
    char **tokens = malloc(tokens_wanted * sizeof(char**));
    if (tokens == NULL) {
        perror("out of memory!");
        return 1;
    }
    get_tokens(line, tokens, tokens_wanted);
    for (size_t i=0; i < tokens_wanted; ++i) {
        printf("token[%lu] = \"%s\"\n", i, tokens[i]);
    }
    free(tokens);
}

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments:  "I didn't know that." or "That's bad advice!"  My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard.  For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:

  [...] a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.

For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:

  If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;

All versions of both standards since then (C99 and C++98) have maintained the same idea.  We rely on automatically generated member functions in C++, and few people write explicit return; statements at the end of a void function.  Reasons against omitting seem to boil down to "it looks weird".  If, like me, you're curious about the rationale for the change to the C standard read this question.  Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time.

So I advocate omitting it; others disagree (often vehemently!)  In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.


Text centering function in bash I wanted to center some text in my commandline application, so I came up with this function.

centerText(){
textsize=${#1}
spacecount=$(tput cols)
spacecount=$((($spacecount-$textsize)/2))
while [ $spacecount -gt 0 ]
do
        printf " "
        spacecount=$((spacecount-1))
done
printf "$1"
echo
}

If you have suggestions on how to improve the code, and/or know a better way of centering text, please let me know.

Since you are using printf to output the spaces, you may as well use a specific printf format for the output.

Here's a replacement function:

centerQ(){
  textsize=${#1}
  width=$(tput cols)
  span=$((($width + $textsize) / 2))
  printf "%${span}s\n" "$1"
}

What it does is it identifies how long the whole output should be, by adding the text width and screen width together, then dividing by 2. That's a bit complicated to explain, but, the right-side of the text must be half-as-much-of-the-text as the middle of the screen.

For example, if the text is 10 characters, and the screen is 80, then the right side of the output must be at 45, or (10 + 80) / 2

If we know the right-hand side of the output, you can just make that a real printf statement:

printf "%45s" "some text!"

The function above does that, by using the padded %s format, it left-pads the output to 45 characters overall width.

By adding the newline escape \n to the printf you save the echo too.


Simple shared pointer implementation in a single threaded environment (revised) std::shared_ptr does not stop you from having the same resource managed by multiple control-blocks (and thus independent sets of shared-pointers), even though it is illegal and leads to double-deletes. Additionally, atomic reference-counts don't have any advantage in single-threaded environments but are still more expensive. Thus I wrote the following version to avoid these problems.

(This is a follow-up to: simple shared pointer implementation in single threaded environment)

template <typename T>
struct ThreadUnsafeSharedPtr {
private:
    struct Pair {
        T data;
        size_t cnt;
        template <typename ... Args>
        Pair(Args && ... args): data(std::forward<Args>(args)...), cnt(1) {}
    };
    Pair *ptr;

public:
    template <typename ... Args>
    static ThreadUnsafeSharedPtr make(Args && ... args) {
        ThreadUnsafeSharedPtr p;
        p.ptr = new Pair(std::forward<Args>(args)...);
        return p;
    }

    ThreadUnsafeSharedPtr() : ptr(nullptr) {}

    ThreadUnsafeSharedPtr(const ThreadUnsafeSharedPtr & other) : ptr(other.ptr) {
        if (ptr && &other != this) {
            ++(ptr->cnt);
        }
    }
    ThreadUnsafeSharedPtr & operator=(const ThreadUnsafeSharedPtr & other) {
        if (ptr && &other != this) {
            ptr = other.ptr;
            ++(ptr->cnt);
        }
        return *this;
    }
    ThreadUnsafeSharedPtr(ThreadUnsafeSharedPtr && other) : ptr(other.ptr) {
        if (&other != this) {
            other.ptr = nullptr;
        }
    }
    ThreadUnsafeSharedPtr & operator=(ThreadUnsafeSharedPtr && other) {
        if (&other != this) {
            ptr = other.ptr;
            other.ptr = nullptr;
        }
        return *this;
    }

    ~ThreadUnsafeSharedPtr() {
        if (ptr) {
            if(--ptr->cnt == 0) {
                delete ptr;
            }
        }
    }
    T & operator* () {
        return ptr->data;
    }

    T * operator-> () {
        return &(ptr->data);
    }
};

Unless this is just for learning, always remember that adding yet another smart-pointer will seriously hinder interaction with anyone expecting a different one.

The Design:

The maker-function is mandatory:

Cannot use a different allocator.
Cannot generally be used to allow sharing across DLL boundaries, as DLLs (in contrast to SOs) don't do symbol unification.
Cannot be used with already-allocated resources, or those needing different deallocation.

Your shared-pointers must always point to the full object. Interior pointers are not allowed, nor are base-pointers.
You don't allow for weak pointers at all.
As an aside, the overhead of using atomic reference-counts is small enough that the standard library eschews the complication of adding single-threaded shared-pointers. Consider whether that's really the part you should save on.

The Implementation:

Using in-class-initializers allows you to simplify the ctors.
You could use aggregate-initialization with Pair, no need for a custom ctor.
Explicitly testing for self-assignment is a bad idea, as it pessimises the common case. Anyway, it's completely useless in a ctor!
You should know that -> binds stronger than anything but scope-resolution ::. If you need to, there are many places to look up the operator precedence rules.
Your move-assignment-operator leaks the assignee's state. Fix it by just doing an unconditional swap instead.


Setting error reporting based on config public function setErrorReporting() {
    if($this->debugMode == 0) {
        error_reporting(0);
    } elseif($this->debugMode == 1) {
        error_reporting("E_ALL");
    } elseif($this->debugMode == 2) {
        error_reporting("E_ALL");
        // Also log errors
    }

I want to change the error reporting based on a value from a config file. (eithor 0, 1 or 2)
But if I ever wanted to increase the options then it won't scale like this.

Is there a better way of handling this?

I would make the function take an optional argument.
This argument $mode could be your user defined logging level that would skip the normal logic to determine the reporting level.

An example could be:

public function set_error_reporting($mode = null) {

    $reporting = null;

    if(!is_null($mode)) {

        $reporting = $mode;

    }else{

        switch($this->debug_mode) {

            case 0:
                $reporting = 0;
                break;

            case 1:
                $reporting = E_ALL;
                break;

            case 2:
                $reporting = E_ALL;
                ini_set('log_errors', 1);  // Enable error logging
                break;

            default:
                $reporting = null;
                break;

        }
    }

    // Check if no reporting level was selected

    if(is_null($reporting)) {
        // throw exception (preferred) or emit an error.
    }

    error_reporting($reporting);
}

Then to set the error reporting level automatically.

$obj->set_error_reporting();

Or if you would like to specify another reporting level from a config file.

$level = E_ALL & ~E_NOTICE;  // Log all errors except notices.

$obj->set_error_reporting($level);

This will require you to fetch the reporting level configuration from outside the function. You could also add more case statements to the switch if you know they will be used beforehand.

Let me know if you have any questions or there is something you would like to point out.

Best regards.


Bash - Passing arguments with "$@" to either 'sudo su' or 'sh' I have a server where shell commands to the supervisor-service must be executed by the specific user lx. I have written a bash function that does exactly that.

function supervisor {
if [ `id -nu` != lx ]; then
    sudo su - lx -c "supervisor \"\$@\"" -- supervisor "$@"
else
    sh -s "$@" <<EOF
supervisor "$@"
EOF
fi
}

It works, but it's doesn't look clean and I'm not really comfortable with half the things I'm using here.

How can that function be written in a less convoluted way?

The only simpler way to do this I've come up with is just to sudo every time and not worry about whether you're already the right user or not.  But overall it seems pretty straight forward and it should work fine as-is.  Some other suggestions:

Indent your code with the function.  Leaving the heredoc alone is ok.
Use [[ for conditionals.  See https://unix.stackexchange.com/a/306115/79839 for more context.
It is more modern to use $() for subcommands instead of backticks (``).  Parenthesis () are also used for creating subshells so the $() is just substituting the result of a subshell.


Filter if any value in group is null The following code works, but the filter operation is bottlenecking my analysis. Do you have an idea how I can make the filter operation faster?
Do you have other comments about the codestyle?

import sys
import pandas as pd
import numpy as np
from numpy.random import randint, rand, choice, permutation

ID = [value for sublist in
      ((value for _ in range(length))
          for value, length in enumerate(randint(1, 10, 70000)))
      for value in sublist]

data = pd.DataFrame(rand(len(ID), 3), columns=['A', 'B', 'C'])

data['ID'] = ID

data['dt'] = randint(0, sys.maxsize, len(data)).astype('M8[ns]')
data.loc[choice([True, False], len(data), [0.05, 0.95]), 'dt'] = None

data = data.apply(permutation)

# Until here the data was only prepared to be similar to my actual data.
# The following command should be optimized.
data.groupby('ID').filter(lambda x: not x['dt'].isnull().any())

The reason why the filter is slow is because by calling lambda, the groupby objects are then iterated over in Python. In this case there are 70000 ID groups and so that can take some time.

One way to avoid this would be to count the number of non-nan values and the number of total values per ID in pandas, then mask your data like that. This keeps everything vectorized in Pandas.

So you could find IDs containing nans,

contains_nan = data.groupby("ID")["dt"].count() != data.ID.value_counts().sort_index()

Note that to compare the two series, the indices for the two series must be the same, so we sort it. Then you can filter just IDs not containing nan and apply the mask:

ids_without_nan = contains_nan[~contains_nan].index
data = data[data.ID.isin(ids_without_nan)]

Benchmarking this method results in:

81.1 ms ± 5.46 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)

versus using filter:

17.7 s ± 396 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)


Simplifying if statement in a StateT monad transformer While learning the basics of Haskell I decided to challenge myself with seemingly simple task - write a program that would push the user input into a stack if it's not an empty string or pop the value from the stack and print it to user if the input is an empty string. the program would exit if it tries to pop from an empty stack.

Realising that I would have to combine functionality of input/output, stateful computations and failure handling got me looking into Monad Transformers. So here is the code that I ended up with:

import Control.Monad
import Control.Monad.Trans.Class
import Control.Monad.Trans.State
import Control.Monad.Trans.Maybe
import Control.Monad.IO.Class

type Stack = [String]

pop :: Stack -> (Maybe String, Stack)
pop (x:xs)  = (Just x, xs)
pop []      = (Nothing, [])

push :: String -> Stack -> ((), Stack)
push s xs = ((), s:xs)

mainLoop :: StateT Stack (MaybeT IO) ()
mainLoop = do
    input <- liftIO getLine
    if null input
         then do
             Just x <- state pop
             liftIO . putStrLn $ x
         else state $ push input
    mainLoop

main = runMaybeT (runStateT mainLoop [])

It works exactly how I wanted, but can someone please look into how I can simplify this code? Or if it's even possible to simplify it? Especially the then branch.

I don't like your pop and push. They're shaped specially for StateT, but leave off the wrapper. I'd suggest, instead, something like this:

pop :: Monad m
    => StateT [a] m (Maybe a)
pop = do
  s <- get
  case s of
    x : s' -> Just x <$ put s'
    [] -> pure Nothing

push :: Monad m
     => a -> StateT [a] ()
push a = modify (a :)

Of course, you can implement those using state, but I think this way is clearer. Once you've gone that far, you might consider using Control.Monad.State.Strict (from mtl) and changing the type signatures:

pop :: MonadState [a] m
    => m (Maybe a)
push :: MonadState [a] m
     => a -> m a

Side note: Control.Monad.Trans.State implements lazy state transformers, which are rather weird beasts you usually don't need. Unless you have a special reason, reach for Control.Monad.Trans.State.Strict or Control.Monad.State.Strict instead.

Moving on with the mtl approach: StateT s (MaybeT m) a is really

s -> MaybeT m (a, s)

or

s -> m (Maybe (a, s))

What about MaybeT (StateT s m) a? That's

StateT s m (Maybe a)

or

s -> m (Maybe a, s)

Not quite the same, but pretty similar! Their monadic operations are essentially the same. The big difference is that in the latter case you get the last state whether or not the operation stopped short. I think you may end up happier that way, with

mainLoop :: MaybeT (StateT Stack IO) x
mainLoop = do
  input <- liftIO getLine
  if null input
    then do
      -- No pattern match!
      x <- MaybeT pop
      liftIO . putStrLn $ x
    else push input
  mainLoop

Extra side note: we can make mainLoop polymorphic in its return value, because it never returns.

An alternative:

pop :: (MonadState [a] m, MonadFail m)
    => m a
pop = do
  s <- get
  case s of
    x : s' -> x <$ put s'
    [] -> fail "Empty stack"

Now you can use the same mainLoop code whichever order you choose to stack the transformers:

mainLoop :: ( MonadState Stack m
            , MonadIO m
            , MonadFail m )
         => m x
mainLoop = do
  input <- liftIO getLine
  if null input
    then do
      x <- pop
      liftIO . putStrLn $ x
    else push input
  mainLoop

In my opinion, though, this is less clear than the other options, and probably doesn't add much useful flexibility.


Is this a good way to print output? I've learned PHP following only tutorials on the web and I'm quite sure my style is not at top.

I'd like to understand how this code could be written in a more elegant way:

<?
$posts = // array with informations about posts fetched with PHP PDO from DB

$html = "
";
foreach($posts as $post) {
    $html .= "

".$post["title"]."
".$post["text"]."
";
}
$html .= "
";
echo $html;

I'm not sure if this is already a good piece of code or not.
I hope you guys can help me out.

There is nothing overtly incorrect with your code.

However, one thing that makes PHP nice to work with for web development is it's ability to interpolate variables.  When you use double quotes around a string, PHP will interpolate (replace variables with their value).  When you do not want that behavior, then you should use single quotes.  You should also use single quotes to delimit keys in arrays, rather than double quotes, which will also interpolate variables, should they be found.

Here's what I'd do in your case, inside the foreach.

$html .= "

{$post['title']}
{$post['text']}
";

This looks more like html, and doesn't require dropping in and out of the string output or concatenating all over the place.

You might wonder why you need the blocks around the $post array variables.  This is so that you can include the single quotes around the array key without confusing the php parser.  If you were to try this:

$html .= "

$post['title']
$post['text']
";

You'll find you get a parse error.  The block tells php to parse that code and insure it works fine.  You can actually omit the single quotes and the code will parse, but it's better stylistically to always include them when dealing with array keys.  This works without the block.

$html .= "

$post[title]
$post[text]
";

Whilst this might be a micro-optimization, when you omit the single quotes, php has to determine whether or not 'title' or 'text' are actually constants you've defined.  It's also just in general, good practice to always reference variables in the same manner, for consistency and readability.

If you want your html markup to look a bit cleaner, it's also easy enough to add newlines using "\n".

$html = "
\n";
foreach($posts as $post) {
    $html .= "

{$post['title']}
{$post['text']}
\n";

}
$html .= "
\n";

One final note, since this is a code review -- what this code does, is intermix markup and data in a way that they can not be separated.  There is a school of thought in the design pattern world, that data (models) should be pure, and that "views" should be responsible for output.  For example, let's say that you want to provide this same data in json format for an ajax call you're doing someplace else in your code.  This routine is not going to work, because you baked in the html markup.

One of the hallmarks of a spaghetti/php application is rampant intermixing of logic and HTML code, and views are a great way to avoid that from the get-go.

Your code will work, but it inherently has the weakness of intermixing your logic and data with your markup.

Template systems like smarty or twig to name a few (although most all of the PHP frameworks that provide MVC have a template system) encourage you to put your presentation (view) code into separate files.  When it comes time to output your code, you inject the data into the view, and your view code can then create your output.  You can do this in a lot of different ways, including using pure old PHP scripts, and alternative PHP syntax.  I'd encourage you to look at some examples in the manuals of these template/view libraries.

As a simple example, here's how you could have your code in a php "view" file that you could require or include.

/* You might name this file postView.php */
/* You'll start in HTML mode */

<? foreach($posts as $post): ?>

<?=$post['title']?>
<?=$post['text']?>

<? endforeach; ?>

Now your logic looks like:

<?php
// Fill your $post from the $_POST super glob.
// Do whatever other logic you need

// Time for output
require_once(postView.php);


Perl module validate quoted string literal, test with Test::LectroTest I guess for some motivation: I ripped this out of a larger library I'm half-working on to parse S-expressions and cleaned it up a bit.

I'm trying to write a simple function that tests whether a string is a valid quoted string literal. Using Test::LectroTest to compare a fast version of a function using a regex against a slow version that uses a Perl for loop.

I have a couple complaints / style questions right off the bat:

Is the lib/QuotedString.pm the right place for the accept_string_slow function to live? I don't intend for people to actually use this function, it's just there to support the tests. Is there a way to hide it better?
I can't actually figure out how to influence how LectroTest generates strings. Since it picks characters uniformly at random, I decided the clearest thing to do would be to accept the string it produced but add quotations marks to exercise different edge cases.
The general style is probably bad. I don't write Perl professionally.

Directory layout:

.
|-- lib
|   `-- QuotedString.pm
`-- t
    `-- test_accept_string_generative.t

Here is lib/QuotedString.pm.

package QuotedString;

BEGIN {
    require Exporter;
    @ISA = qw[Exporter];
    @EXPORT_OK = qw[
        accept_string_regex
        accept_string_slow
    ];
};

use strict;
use warnings;

use Readonly;

Readonly my $string_pattern => qr/["](?:[^\\"]|[\\].|[\\]\n)*["]/;

sub accept_string_regex {
    my $string = shift;
    ($string =~ m/\A$string_pattern\z/) ? 1 : 0;
}

sub accept_string_slow {
    my $string = shift;
    return 0 if length($string) < 2;
    return 0 unless $string =~ /\A"/;
    return 0 unless $string =~ /"\z/;

    my $index = 1;
    my $final_checked_char = length($string) - 2;
    while ($index <= $final_checked_char) {
        my $char = substr($string, $index, 1);
        # if we encounter an unescaped ", we fail
        return 0 if $char eq '"';
        # \ consumes the next character fail if there is no next character before final "
        return 0 if $char eq '\\' and $index == $final_checked_char;

        # advance the index two positions if the current char is \
        # because the next one is escaped regardless of what it is
        # if the current character is not \, then advance one position
        if ($char eq '\\') {
            $index += 2;
        } else {
            $index += 1;
        }
    }
    return 1;
}

and the test t/test_accept_string_generative.t

#!/usr/bin/env perl
use strict;
use warnings;
use Test::LectroTest (
    trials => 2000
);

use QuotedString qw[accept_string_regex accept_string_slow];

Property {
    ##[ x <- String ]##
    accept_string_slow($x) eq accept_string_regex($x);
};

Property {
    ##[ x <- String ]##
    accept_string_slow('"' . $x) eq accept_string_regex('"' . $x);
};

Property {
    ##[ x <- String ]##
    accept_string_slow('"' . $x . '"') eq accept_string_regex('"' . $x . '"');
};

Location of your helper function

If you place the accept_string_slow function inside of your lib/QuotedString.pm, it's visible to everyone. That means that someone will use it.

There is no concept of private in Perl. We have a convention to prefix things with an underscore _ so people know it's something they should not be messing with. But that does not stop them.

If this function is really only for testing, then you have a few options. It looks like it's your expected output, which because the input is random, you built into a function. That's a good approach.

The easiest and in my opinion most appropriate way is to put it right into your test .t file. It's part of that very specific test and is not used anywhere else. Just place it under the use QuotedString and remove the name from the use line.

use QuotedString qw[accept_string_regex];

sub accept_string_slow {
    my $string = shift;
    return 0 if length($string) < 2;
    return 0 unless $string =~ /\A"/;
    return 0 unless $string =~ /"\z/;

    # ...
}

Property {
    ##[ x <- String ]##
    accept_string_slow($x) eq accept_string_regex($x);
};

And that's it. Your calls to the two functions inside of the Property now still work, because all that the use does is import the function from the QuotedString package into the local package (which is main). You don't need to worry about how that works.

I would also recommend renaming the function to something that tells the maintainer (which might well be future-you) what the heck is going on here. Maybe go with sub expected_output or similar. Since it's now contained in this test you don't need to adhere to any naming conventions that the project might have. It's your code, make it as readable as possible.

Alternative ways would be to either monkeypatch the function into the QuotedString package, which makes sense if you want to overwrite a certain behavior, or to use a separate library and put it e.g. in t/lib/Helper/QuotedString.pm, which makes sense for larger changes or if you need an entire class to help you out during your tests.

Influencing LectroTest

This is a bit more suited for Stack Overflow, but here's a run-down anyway.

You should first read the docs of Test::LectroTest::Generator. That module gives you everything you need. You can use the Paste combinator with the Unit generator and the String generator. Here's an example that just outputs 20 combinations.

use strict;
use warnings;
use feature 'say';
use Test::LectroTest trials => 20;
use Test::LectroTest::Generator ':all';

Property {
    ##[ x <- Paste( Unit('"'), String( charset=>"A-Z", length=>[1,10] ) ) ]##
    say "x = $x";
};

Unit gives you a fixed value that never changes. Kind of like a constant. String you already know, but it seems we need to be more specific if it's used with Paste. I have not figured out why yet. The arguments charset and length should be self-explanatory. You can use single characters, like abc to allow a, b, and c, character ranges like A-F to create ABCDEF automatically. Those can also be combined as in AB-EFg. The length here means between one and ten.

Paste concatenates them. Actually it's more like a join, but if you omit the last argument glue, it uses an empty string and thus it's really a concat. (Beware, there is a Concat that does something else!)

1..1
x = "YSJAQXSG
x = "WWU
x = "IVKJGLWVV
x = "TWQHU
x = "DVXJDTRTX
x = "CC
x = "SULEP
x = "NFBTM
x = "UR
x = "TKS
x = "IUXIMDLUWX
x = "WHYLRV
x = "CWIQLLGO
x = "N
x = "J
x = "FZOLCBCUZX
x = "FN
x = "ZVAQZJSN
x = "TIYUXJV
x = "SVE
ok 1 - 'Unnamed Test::LectroTest::Property' (20 attempts)

This seems to work. You can also add more than one row of input definitions. That way, you can combine several tests into one Property. This is explained in Test::LectroTest::Property.

Property {
    ##[ x <- Paste( Unit('"'), String( charset=>"A-Z", length=>[1,10] ) ) ],
    [ x <- Paste( String( charset=>"A-Z", length=>[1,10] ), Unit('"') ) ]##
     say "x = $x";
};

I've reduced the trials to 5 to make it easier to read. It will run each line of definitions 5 times.

1..1
x = "CIWNIGSNL
x = "YI
x = "GPCAEZYO
x = "EPLKF
x = "ABUD
x = HS"
x = JGW"
x = QLZJUAZ"
x = NUQEKGL"
x = RSGSVOAXAQ"
ok 1 - 'Unnamed Test::LectroTest::Property' (10 attempts)

This could now also be used to do the "foo" check where the enclosing quotes are intact.

If you want to instead of spelling all of this out, pre-build a generator and use the variable in your definitions, you can do the following. The Test::LectroTest::Generator documentation does this, but fails to explain how to use it with the comment-style syntax.

my $gen = Paste( Unit('"'), String( charset => "A-Z", length => [ 1, 10 ] ) );

Property {
    ##[ x <- $gen ]##
    say $x;
};

To go a bit further, it would also be possible to build a definition that sometimes injects " or \\ or \\" with reduced likelihood.

my $overkill = Paste(
    (
        Frequency(
            [ 95, Char( charset => "a-zA-Z0-9'!?., -" ) ],
            [ 4,  Unit('"') ],
            [ 4,  Unit('\\') ],
            [ 2,  Unit('\\"') ],
        )
    ) x 10, # the x operator needs to be used in list context here
    glue => q{},
);

Property {
    ##[ x <- $overkill ]##
    say $x;
};

__END__
1..1
"Aqgz0,n8x
uBoLq"\",hq
"zx!bfJe0l
YW1u'k5"!b
Q,?9UK\\Jw
ok 1 - 'Unnamed Test::LectroTest::Property' (5 attempts)

Now you could combine all of this together to build intricate tests. If you combine those with the advice I give below at the very end of this post, you should be more than covered.

Looking into the regression testing should also prove useful. Unfortunately the documentation is very lacking as to how this works ifyou want to include manual cases. It does talk about adding failures manually, but never says how to do that. But if you turn it on, it should record it, and from that you should be able to figure out the syntax to add cases yourself.

Also take a look at this document that I found while digging through the distribution. I couldn't find the corresponding PerlMonks post, but there is some stuff about LectroTest on PerlMonks that might help, though it's old.

Although the author's website which is supposed to have more information about the whole thing is down, I did find his blog that might also have some more information. It does seem like the author disappeared from the Perl world in 2013 though.

General style

Test conditions

When I wrote this answer I was looking at the code an saw

accept_string_slow($x) eq accept_string_regex($x);

Because I seem to have a low attention span, I then thought that the function we are testing is accept_string_slow. I copied over the _regex one into the first part of the answer and started writing a whole paragraph about getting rid of Readonly because it's then not needed any more. At some point I realized that earlier you said that you wrote the slow function for control, so I facepalmed and deleted all of that.

What I want to show with that little story is that there is a convention to put the thing that is tested first in conditions.

if ($foo eq 42) { ... }

The same goes for tests written with Test::More.

is $foo, 42, 'Foo is the answer';

Doing it the other way around is called a Yoda condition, and with two function calls it's really not obvious what's going on. While that does not affect anything in the code, your maintenance person might see it the same way I did and then stuff might break later. You should turn them around.

Property {
    ##[ x <- String ]##
    accept_string_regex($x) eq accept_string_slow($x);
};

Putting the quotes into the string

While your '"' . $x . '"' approach is valid and works and is okishly readable, it might be better to use the q{} operator, which constructs strings. q is like single quotes '' without interpolation, and qq is like double quotes with interpolation. You can choose the delimiter yourself. I prefer curly braces {}. I use those for empty strings and strings that contain non-letters or digits, especially with lengthy one. You can read more about them in perlop.

Property {
    ##[ x <- String ]##
    accept_string_slow(q{"} . $x . q{"}) eq accept_string_regex(q{"} . $x . q{"});
};

Of course you can also interpolate your $x variable if you feel that's more readable. Then you need to use qq{}.

Property {
    ##[ x <- String ]##
    accept_string_regex(qq{"$x"}) eq accept_string_slow(qq{"$x"});
};

Testing functions from a library

As I said above, the use Foo qw(bar) syntax imports the bar function from the Foo namespace into the local namespace. Importing means that a new reference to the function is created in the current namespace, so that main::bar() is the same Foo::bar() (and so is bar()).

That's useful, but in your test it might not be what you want to do. While that is a lot more typing in the tests, it might be more expressive to actually not import the function, but instead use the fully qualified name. That way, you know exactly what's going on when you look at the code (did I mention tests are code too?).

use strict;
use warnings;
use Test::LectroTest (
    trials => 2000
);

use QuotedString ""; # this now has an empty string, not an empty list

Property {
    ##[ x <- String ]##
    QuotedString::accept_string_regex($x) eq accept_string_slow($x);
};

This is also what the SYNOPSIS in the Test::LectroTest documentation does.

Name your tests

You should always give meaningful names to your test cases. If I run your tests the way they are, this is the output.

  1..3
  ok 1 - 'Unnamed Test::LectroTest::Property' (2000 attempts)
  ok 2 - 'Unnamed Test::LectroTest::Property' (2000 attempts)
  ok 3 - 'Unnamed Test::LectroTest::Property' (2000 attempts)

If one of them breaks, how should we know which one is actually broken? Especially if there are a lot of tests, it gets very confusing quickly. You can name Test::LectroTest Property tests by adding a name parameter like this.

Property {
    ##[ x <- String ]##
    accept_string_slow($x) eq accept_string_regex($x);
}, name => 'String without explicit enclosing quotes';

Try to pick names that describe your test case by explaining the business logic you are testing. A test named try with value foo is a bad name, while passing in foo does not break stuff is a lot better.

Style in your accept_string_slow

This is very good. Your code is clear, readable and well-commented. Your comments make sense, your variable names are speaking and the function fits on one screen. Well done.

The only thing I would say here is actually a moot point because the interface you are mimicking is defined like that. Usually you wouldn't do return 0 (as in return 0 if $foo), but instead just say return if $foo. Without a value, return returns undef. Both 0 and undef are false in Perl, so that's usually fine. But since the interface here explicitly uses 1 and 0, it's perfectly ok to do the same.

Have you verified your code?

Your function has 19 lines of code. That's nicely small, but it's also a lot of space to include bugs. Always remember that tests are also just code. I would include some safety checks that make sure that your function does what it is supposed to do.

The Test::LectroTest approach is a very inclusive, almost all-pairs kind of test. But simply having some predefined test cases should do the trick. At least for your helper.

use Test::More;

is accept_string_slow(q{no quotes here}),          0, 'string without quotes fails';
is accept_string_slow(q{"no quotes at the end}),   0, 'string with no final quote fails';
is accept_string_slow(q{no quotes at the start"}), 0, 'string with no initial quote fails';
is accept_string_slow(q{"too many "quotes"}),      0, 'string with unescaped quote inside fails';
is accept_string_slow(q{"foo \\ bar"}),            0, 'string with stray backslash fails';
is accept_string_slow(q{"foo \\"}),                0, 'string with escaped final quote fails';
is accept_string_slow(q{"happy case"}),            1, 'happy case';

But it seems mixing the Test::Builder (which Test::More is based on) with Test::LectroTest is a bit complicated. It's explained in Test::LectroTest::Compat, but if your whole test suite is based on Test::LectroTest and the output it produces, it's probably not worth using this just for a single function.

So it would make more sense to build your test directly with Test::LectroTest. As far as I understand that's done with the the regression tests file. That seems to be a whole different story though. I suggest you read that doc. It seems to also be the way to go to include your edge-cases.


More elegant solution to create a array of fake data I am creating fake data using faker.js. Out of simplicity I created a for-loop.

const faker = require("faker")
const dataLength = 10
const productArr = []

for (var index = 0; index < dataLength; index++) {
  productArr.push({
    name: faker.commerce.productName(),
    category: faker.commerce.department(),
    price: faker.commerce.price(),
    description: faker.lorem.paragraph(),
  })
}

console.log(productArr.length)
console.log(JSON.stringify(productArr))

Is there a more elegant solution to iterate through an array that has a fixed length?

Since you're iterating for the sole purpose of creating an array, map is somewhat appropriate here.

const productArr = [... Array(dataLength)] // Create a dummy array dataLength elements long
                     .map(_ => ({ // And map over it
                         name: faker.commerce.productName(),
                         category: faker.commerce.department(),
                         price: faker.commerce.price(),
                         description: faker.lorem.paragraph(),
                     }));

I'm mentioning it just in case you aren't aware of map, but it could be argued that this exact usage is an abuse of the function. Really, map is for transforming one list into another list of the same length. In this case though, we're completely ignoring the original contents of the array being iterated over (shown by the fact that the parameter is called _).

I can't say I necessarily recommend map in this case, but I thought I'd mention it.

If however you ever wanted to enumerate the data you're producing, it would be appropriate:

const productArr = [... Array(dataLength).keys()] // [0, 1, 2, 3, ...]
                         .map(i => ({
                             i: i, // And use the parameter this time
                             name: faker.commerce.productName(),
                             category: faker.commerce.department(),
                             price: faker.commerce.price(),
                             description: faker.lorem.paragraph(),
                         }));

Resource for the "range" on the first line..


Counting the number of elements in a sorted array Suppose I have a sorted array, and each element may appear multiple times.

To make it simple, here is an example:

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

The expected output is the number of each element. In this example, the results are:

number of 1 is 17
number of 2 is 6
number of 3 is 9

I want to find a solution where the algorithm time complexity is lower than \$O(n)\$. My idea is similar to binary search:

When I meet with equal neighbour, I will jump the tail pointer by 1, by 2, by 4, etc. and at the same time, I will make cur pointer move to the tail position
When I meet with unequal case, I will shrink the tail pointer to be in the middle of cur and tail
The diff between cur and last element tail is the count of the current elements

Any advice on algorithm time complexity improvement, code bugs or general code style advice is appreciated.

from collections import defaultdict
import random
def generate_count(numbers):
    last_index = -1
    result = defaultdict(int)
    cur_index = last_index + 1
    tail_index = cur_index + 1
    while cur_index < len(numbers):
        while cur_index < tail_index and cur_index < len(numbers) and tail_index < len(numbers):
            if numbers[cur_index] == numbers[tail_index]:
                pre = cur_index
                cur_index = tail_index
                tail_index += (tail_index - pre)
            else:
                tail_index = (cur_index+tail_index) / 2
        result[numbers[cur_index]] = cur_index - last_index
        last_index = cur_index
        cur_index = last_index + 1
        tail_index = cur_index + 1
    return result
if __name__ == "__main__":
    numbers = []
    for i in range(random.randint(1,20)):
        numbers.append(1)
    print 'number of 1 is', i+1
    for i in range(random.randint(1, 20)):
        numbers.append(2)
    print 'number of 2 is', i+1
    for i in range(random.randint(1, 20)):
        numbers.append(3)
    print 'number of 3 is', i+1
    print numbers
    print generate_count(numbers)

I want to find a solution [whose] algorithm time complexity is lower than O(n).

You'll have to be more specific. What you asked for literally isn't possible; but sometimes people use "big-O" notation loosely, either out of laziness or because they haven't yet been exposed to the notation expressing their intended precise gradations of meaning.

Notice that if I give you the input

1 2 3 4 5 6 7 8 9 10 11 12 13 ...

then you must output

number of 1 is 1
number of 2 is 1
number of 3 is 1
number of 4 is 1
...

— that is, if the size of the input is n, then the worst-case size of the output is O(n). Printing those O(n) bytes of output will take O(n) time just by itself. Anything else the program does (besides print its output) can only make the overall program slower, not faster.

On the above worst-case input, your "repeated binary search" algorithm runs in O(n log n) time, which is worse than O(n).

Now, if you're not concerned about worst-case inputs, but merely with the average input... well, then you'd have to define a notion of "the average input". You'll have to come up with some model of what kinds of input you expect. One way to do this is to come up with an algorithm for generating inputs, and then treat it as your model. For example:

def get_random_input(n):
    return sorted([random.randrange(n) for i in xrange(n)])

Or:

def get_random_input(n):
    return sorted([random.randrange(100) for i in xrange(n)])

On these two different models, your algorithm performs very differently. On the latter, you do well: O(log n). On the former, you do poorly: O(n log n).

This is kind of like asking for a compression algorithm that compresses all possible inputs. You can get compression for some inputs only at the expense of others; the art is deciding which kinds of inputs are "interesting" and which can safely be blown up. In this case, you can get sub-O(n) performance for some inputs only at the expense of others; the art is deciding which kinds of inputs are "interesting", which is to say, deciding on a model for your "average" or "expected" input.

EDIT: My analogy above is flawed, as indicated by LinMa's comments below this answer. See, if you can get O(log n) performance on your "expected" inputs while blowing up the "unexpected" inputs by only a constant factor (e.g. "O(n) plus a constant number of comparisons" or "O(n) times two"), then you can keep your algorithm's worst-case big-O "constant" while improving the best-case.

Hope this helps!


Model with complex/simple properties I have a model with a number of complex/simple properties that has a corresponding strongly typed view, that calls EditorFor to a custom editor template view for the model.

One of the form's requirements is to auto fill the form based on a choose of values from a database.

Previously, I had the selectors on the selection popup do a post back to an action with the additional value needed to prepopulate the model, and then the model is bound like normal.

I wanted to see if I could alter the form to do the prefill with Ajax:

I have a JsonResult Action that accepts a unique Id, and returns the JSON version of the model.

When you click the selection button this action is invoked via jQuery $.ajax post.

Each input on the form has an Id in the form Id="ModelName_PropertyName" so parse the keys in the JSON object, using the keys to look up the field in a given selector, and set the values from JSON.

Here is how I invoke from Ajax in CoffeeScript (I can post the JavaScript if requested):

$.ajax
    type: "POST"
    url: $("#SelectFromPolicyContainer").data "url"
    data: vin
    success: (response) ->
        $("#PropertyInfo").bindFormToJson response.Data.Vehicle, true
        $("#SelectFromPolicyResponse").dialog "close"

The Action looks like this:

[HttpPost]
public JsonResult VehicleByVIN(string VinNumber)
{
    return AjaxJsonResponse.Create(UserNotice.NoNotice, new
    {
        Vehicle = Policy.Vehicles.SingleOrDefault(p=>p.Info.Vin.Number==VinNumber)
    });
}

And the definition of bindFormToJson:

$ -> $.fn.bindFormToJson = (JsonObject, OverwritePopulatedFields) ->

    selector = $(this).selector

    iterateJson = (JSonObject, FullKey, Seperator, func) ->
        for key, value of JSonObject
            newKey = ""
            if FullKey == ""
                newKey = key
            else
                newKey = FullKey + Seperator + key

            if typeof(value) == "object"
                iterateJson value, newKey, Seperator, func
            else
                func newKey, value

    iterateJson JsonObject, "", "_", (key,value) ->
        field = $(selector + " #" + key)
        currentValue = field.val()
        if (currentValue and OverwritePopulatedFields) or not currentValue
            field.val(value)

Any tips, or things I may have missed or potential problems I might have overlooked would be wonderful.  In my test environment right now though, this is working beautifully.

Not a JavaScript pitfall per se, but I'd stay away from using two variable names that are only differentiated by the case of the second letter in a single lexical scope like that. (JsonObject vs JSonObject)

Also, the bindFormToJson function is re-defining the iterateJson function every time it is called, which a waste of cycles (not egregious, but less than ideal). Using coffeescripts do notation is a handy way to create a closure and return a value from it:

$ -> $.fn.bindFormToJson = do ->

    iterateJson = (Source, FullKey, Seperator, func) ->
        for key, value of Source
            newKey = ""
            if FullKey == ""
                newKey = key
            else
                newKey = FullKey + Seperator + key

            if typeof(value) == "object"
                iterateJson value, newKey, Seperator, func
            else
                func newKey, value

    return (JsonObject, OverwritePopulatedFields) ->
        selector = $(this).selector
        iterateJson JsonObject, "", "_", (key, value) ->
            field = $(selector + " #" + key)
            currentValue = field.val()
            if (currentValue and OverwritePopulatedFields) or not currentValue
                field.val(value)

The return is unnecessary, but I figured it can't hurt to be explicit about what's happening there.


Add effective error handling to Python Notebook in Azure Synapse Analytics I have some Python code that runs as part of an Azure Synapse Analytics Apache Spark Notebook (or Synapse Notebook) and would like to add effective error handling.  The code simply executes a given SQL script against the database.  The code runs but I sometimes see errors like attempt to use closed connection.  I would like to do the following:

Improve code that I wrote through peer review
Can I improve the error handling?  eg pseudo-code if connection still open close connection
The code using SQL auth works.  I would like to authenticate as the Managed Identity, I've tried using the object id of the MI in the connection string with Authentication=ActiveDirectoryMsi but it didn't work

Cell1 - parameters
pAccountName = 'someStorageAccount'
pContainerName = 'someContainer'
pRelativePath = '/raw/sql/some_sql_files/'
pFileName = 'someSQLscript.sql'

Cell 2 - main
import pyodbc
from pyspark import SparkFiles

try:
    # Add the file to the cluster so we can view it
    sqlFilepath = f"""abfss://{pContainerName}""" + "@" + f"""{pAccountName}.dfs.core.windows.net{pRelativePath}{pFileName}"""
    sc.addFile(sqlFilepath, False)

    # Open the file for reading
    with open(SparkFiles.getRootDirectory() + f'/{pFileName}', 'r') as f:
        lines = f.read()    ## read all text from a file into a string

    # Open the database connection
    conn = pyodbc.connect( 'DRIVER={ODBC Driver 17 for SQL Server};'
                        'SERVER=someServer.sql.azuresynapse.net;'
                        'DATABASE=someDatabase;UID=batchOnlyUser;'
                        'PWD=youWish;', autocommit = True )

    # Split the script into batches separated by "GO"
    for batch in lines.split("GO"):
        conn.execute(batch)  # execute the SQL statement

except:
    raise

finally:
    # Tidy up
    conn.close()

Ignoring your closed-connection and managed-identity questions, since (a) I don't know how to answer them and (b) on their own they're off topic, there is still review to be done:

A Pythonic way to tell the standard library to format your URI instead of you doing it yourself is to call into urlunparse
Consider using pathlib
connect accepts kwargs as an alternative to the conn string, and the former will lay out your connection parameters more nicely
except / raise is redundant and should be deleted
Your try starts too early and should only start after the connection has been established
You're not reading a lines list; you're reading content, which would be a better variable name

Suggested
from pathlib import Path
from urllib.parse import urlunparse, ParseResult
import pyodbc
from pyspark import SparkFiles

pAccountName = 'someStorageAccount'
pContainerName = 'someContainer'
pRelativePath = '/raw/sql/some_sql_files/'
pFileName = 'someSQLscript.sql'

# Add the file to the cluster so we can view it
sql_filename = urlunparse(
    ParseResult(
        scheme='abfss',
        netloc=f'{pContainerName}@{pAccountName}.dfs.core.windows.net',
        path=f'{pRelativePath}{pFileName}',
        params=None, query=None, fragment=None,
    )
)

sc.addFile(sql_filename, False)

# Open the file for reading
sql_file_path = Path(SparkFiles.getRootDirectory()) / pFileName
with sql_file_path.open() as f:
    content = f.read()

# Open the database connection
conn = pyodbc.connect(
    DRIVER='{ODBC Driver 17 for SQL Server}',
    SERVER='someServer.sql.azuresynapse.net',
    DATABASE='someDatabase',
    UID='batchOnlyUser',
    PWD='youWish',
    autocommit=True,
)

try:
    # Split the script into batches separated by "GO"
    for batch in content.split("GO"):
        conn.execute(batch)  # execute the SQL statement
finally:
    conn.close()


Encrypt a byte array This code is a Frankenstein. I found ten examples done in ten different ways and my requirements were slightly different so even if they were reliable they weren't a copy and paste solution.

Encrypt takes in a byte[] and password and returns the byte[] encrypted with the password. It has to tack on the length of the original byte[] and the initialization vector used by the encryption algorithm.

Decrypt knows how to read the encrypted byte[] and will return the original message.

Note: One thing about naming conventions - Since a lot of this is copy and paste from different sources I tended to keep the original authors' conventions so that the code would match up. That's why some variables start with caps and some don't. Not looking to have this post turn into a tangent about that low hanging fruit though. But since it's obvious I thought I should point it out.

Encrypt

// https://social.msdn.microsoft.com/Forums/vstudio/en-US/eab7d698-2340-4ba0-a91c-da6fae06963c/aes-encryption-encrypting-byte-array?forum=csharpgeneral
// https://crypto.stackexchange.com/questions/2280/why-is-the-iv-passed-in-the-clear-when-it-can-be-easily-encrypted
public static byte[] Encrypt(byte[] bytesToEncrypt, string password)
{
    byte[] ivSeed = Guid.NewGuid().ToByteArray();

    var rfc = new Rfc2898DeriveBytes(password, ivSeed);
    byte[] Key = rfc.GetBytes(16);
    byte[] IV = rfc.GetBytes(16);

    byte[] encrypted;
    using (MemoryStream mstream = new MemoryStream())
    {
        using (AesCryptoServiceProvider aesProvider = new AesCryptoServiceProvider())
        {
            using (CryptoStream cryptoStream = new CryptoStream(mstream, aesProvider.CreateEncryptor(Key, IV), CryptoStreamMode.Write))
            {
                cryptoStream.Write(bytesToEncrypt, 0, bytesToEncrypt.Length);
            }
        }
        encrypted = mstream.ToArray();
    }

    var messageLengthAs32Bits = Convert.ToInt32(bytesToEncrypt.Length);
    var messageLength = BitConverter.GetBytes(messageLengthAs32Bits);

    encrypted = encrypted.Prepend(ivSeed);
    encrypted = encrypted.Prepend(messageLength);

    return encrypted;
}

Decrypt

public static byte[] Decrypt(byte[] bytesToDecrypt, string password)
{
    (byte[] messageLengthAs32Bits, byte[] bytesWithIv) = bytesToDecrypt.Shift(4); // get the message length
    (byte[] ivSeed, byte[] encrypted) = bytesWithIv.Shift(16);                    // get the initialization vector

    var length = BitConverter.ToInt32(messageLengthAs32Bits, 0);

    var rfc = new Rfc2898DeriveBytes(password, ivSeed);
    byte[] Key = rfc.GetBytes(16);
    byte[] IV = rfc.GetBytes(16);

    byte[] decrypted;
    using (MemoryStream mStream = new MemoryStream(encrypted))
    {
        using (AesCryptoServiceProvider aesProvider = new AesCryptoServiceProvider())
        {
            aesProvider.Padding = PaddingMode.None;
            using (CryptoStream cryptoStream = new CryptoStream(mStream,aesProvider.CreateDecryptor(Key, IV), CryptoStreamMode.Read))
            {
                cryptoStream.Read(encrypted, 0, length);
            }
        }
        decrypted = mStream.ToArray().Take(length).ToArray();
    }
    return decrypted;
}

Relevant Extension Methods (i'm more worried about security than these)

    public static byte[] Prepend(this byte[] bytes, byte[] bytesToPrepend)
    {
        var tmp = new byte[bytes.Length + bytesToPrepend.Length];
        bytesToPrepend.CopyTo(tmp, 0);
        bytes.CopyTo(tmp, bytesToPrepend.Length);
        return tmp;
    }

    public static (byte[] left, byte[] right) Shift(this byte[] bytes, int size)
    {
        var left = new byte[size];
        var right = new byte[bytes.Length - size];

        Array.Copy(bytes, 0, left, 0, left.Length);
        Array.Copy(bytes, left.Length, right, 0, right.Length);

        return (left, right);
    }

Standard disclaimer: in a production setting it is overwhelmingly better to use somebody else's crypto.

I don't have the expertise to certify "Yes, this is secure" (and as I'm some randomer online, you should assume that even if I was actually a world expert!) There are, however, a few things that strike me as red flags.

byte[] ivSeed = Guid.NewGuid().ToByteArray();

In crypto, if you need a random number, you basically always need a cryptographically secure random number. NewGuid() is not cryptographically secure.

var messageLength = BitConverter.GetBytes(messageLengthAs32Bits);

Block ciphers like AES do not claim to obscure the approximate message length, so this is not strictly a vulnerability. Nevertheless if you are vulnerable to any attacks based on the length, then passing the precise length in plaintext makes those attacks a whole bunch quicker and easier to execute.

byte[] Key = rfc.GetBytes(16);
byte[] IV = rfc.GetBytes(16);

The derivation of your key should not be tied to the derivation of your IV.


Booting up my Ruby social aggregator app Some time ago I started with a small Ruby project. I call it social_aggregator.
This software aggregates information from different networks to one xml-stream, which you can reuse. For instance on your personal website to show some of your activity to your audience or as your personal news feed.

I've a built in Webrick server, while I also want to support any other rack based server. So I have this two entry/start points:

config.ru:

$LOAD_PATH.unshift(File.dirname(__FILE__))

require 'Aggregator'
require 'conf/router'

# Run aggregator
app = Thread.new{ Aggregator.new }
app.run

use ActiveRecord::ConnectionAdapters::ConnectionManagement
use ActiveRecord::QueryCache

run Router.map

And the Aggregator.rb:

$LOAD_PATH.unshift(File.dirname(__FILE__))

require 'conf/initializers/DatabaseInitializer'
require 'conf/initializers/ConsoleInitializer'
require 'app/plugins/PluginManager'
require 'app/utils/ArgumentParser'
require 'app/utils/Logging'
require 'app/utils/Setting'

# Main class
class Aggregator
    include Logging
    include Setting

    # The version constant
    AGGREGATOR_VERSION = '0.0.1' unless const_defined?(:AGGREGATOR_VERSION)

    # Stores the environment
    @@environment = :production

    # Stores the run state
    @@stop = false

    # Initialize the whole system
    def initialize(internal_server = false, arguments = [])
        options = ArgumentParser.parse(arguments)

        Logging::environment options.environment

        # Set up logger
        if options.quiet
            Logging::quiet = true
        end

        logger.info 'Starting up aggregator now'

        # Set environment to setting
        logger.info "Using #{options.environment} environment"
        @@environment = options.environment

        # Connect database with orm
        DatabaseInitializer.new(options.environment)

        unless options.environment == :test || options.console
            # Set up plugin manager
            @@plugin_manager = PluginManager.new

            if internal_server
                require 'conf/initializers/ServerInitializer'

                # Spawn new server
                ServerInitializer.new
            end

            logger.info "Aggregator is up and running"

            Signal.trap("SIGINT") do
                Thread.current.exit
            end

            start
        end

        if options.console
            ConsoleInitializer.new(options.environment)
        end
    end

    # Returns the current environment
    def self.environment
        @@environment
    end

    # Returns the version number
    def self.version
        AGGREGATOR_VERSION
    end

    # Returns the plugin manager
    def self.plugin_manager
        @@plugin_manager
    end

    # Shutdown the system
    def self.shutdown
        @@stop = true
    end

    private

    # Starts the aggregation
    def start
        @@plugin_manager.run

        if @@stop
            logger.info "Stopping aggregation now, due request to stop."
            return
        end

        logger.debug "Aggregation done. Next aggregation in #{setting.aggregate_timer} seconds."

        sleep setting.aggregate_timer
        start
    end
end

# Initialize aggregator
if __FILE__ == $PROGRAM_NAME
    app = Aggregator.new(true, ARGV)
end

Is that a good way to realize a built in server as well as support for other app server?
If you're interested in the project, you can find it here: https://github.com/openscript/social_aggregator

You can see the answer to Implementing plugins in my Ruby social aggregator app to see some general observations on your code style.

The most important thing I have to point out on this code is that your run loop is not a loop, but a recursion. This is especially bad, since it will eventually kill your whole application on a StackOverflow exception (pun?)

You should always use iteration over recursion when the loop is infinite:

def start
  until @@stop
    @@plugin_manager.run

    logger.debug "Aggregation done. Next aggregation in #{setting.aggregate_timer} seconds."

    sleep setting.aggregate_timer
  end
end

Also, you seem to mix instance scope and class scope. You allow for multiple Aggregators, but there is only a single shutdown. Either design your class for multiple uses, in which case state and methods should be of the instance, or that it is a Singleton, for which there are patterns in ruby.

You seem to have a lot of comments in your code. Ruby encourages less comments, and more expressive method names. There is no point in a comment, if it does not add anything to the code itself. This code:

# Initialize aggregator
app = Aggregator.new(true, ARGV)

has no advantage to simply writing

app = Aggregator.new(true, ARGV)

Using static members (@@stop) is also not advisable. If you are writing inside the self scope (inside def self.shutdown) simple members (@stop) are actually in the class scope, being effectively static. To access them from outside this scope, add a getter:

def self.stop
  @stop
end

Which will be accessible by calling Aggregator.stop.

Refactored Aggregator:

class Aggregator
  include Logging
  include Setting

  AGGREGATOR_VERSION = '0.0.1'

  def self.version
    AGGREGATOR_VERSION
  end

  attr_reader :environment, :plugin_manager, :stop

  def initialize(internal_server = false, arguments = [])
    options = ArgumentParser.parse(arguments)

    Logging::environment options.environment
    Logging::quiet = true if options.quiet

    logger.info 'Starting up aggregator now'

    logger.info "Using #{options.environment} environment"
    @environment = options.environment

    DatabaseInitializer.new(options.environment)

    return if options.environment == :test

    if options.console
      ConsoleInitializer.new(options.environment)
    else
      @plugin_manager = PluginManager.new

      ServerInitializer.new if internal_server

      logger.info "Aggregator is up and running"

      Signal.trap("SIGINT") do
        Thread.current.exit
      end

      start
    end
  end

  def shutdown
    @stop = true
  end

  private

  def start
    until stop
      @plugin_manager.run

      logger.debug "Aggregation done. Next aggregation in #{setting.aggregate_timer} seconds."
      sleep setting.aggregate_timer
    end
  end
end


Find offset that minimizes squared difference between two vectors of different size I am working on a project where the hotspot is some code that is supposed to find an offset, such that the squared differences between a reference vector and a given vector becomes minimal. The core function is this:

#include <vector>
#include <map>
typedef std::vector<double> vect_d

double sqDiffSum(const vect_d& ref, const vect_d& x,int offset){
    size_t scale = ref.size() / x.size(); // ref.size() is always n*x.size()
    double sum = 0;
    int counter = 0;
    for (size_t i=0;i<x.size();i++){
        for (size_t j=0;j<scale;j++){
            int bin = i*scale + j - offset;
            if (bin >= 0 && bin < (int)ref.size()){
                double diff = ref[bin] - x[i];
                sum += diff * diff;
                counter++;
            }
        }
    }
    return sum / counter;
}

This is called from some minimization routine, that (for now) I dont want to change. It varies the offset to get the minimum sqDiffSum. I know that I could probably do the task faster by using a FFT, but for now I want to see how much I can get out of the current approach. I could already make it faster (roughly x2.5) by adding some memoization:

double callSqDiffSum(const vect_d& ref, const vect_d& x,int offset,bool firstCall){
    static std::map<int,double> memo;
    if (firstCall){ memo.clear(); }
    else {
        std::map<int,double>::iterator found = memo.find(offset);
        if (found != memo.end()){
            return found->second;
        }
    }
    double result = sqDiffSum(ref,x,offset);
    memo[offset] = result;
    return result;
}

However, it is still too slow. There are several points that in principle I could optimize, but for each I have my doubts whether it will bring any benefit:

The reference vector ref is actually always the same, thus I could avoid passing it around at all.
The size of the vectors is always the same, so actually there is no need for std::vector, but if possible I would like to avoid c-style arrays and I dont expect too much improvement by not using vectors here.
The innermost loop has some branching, that could be avoided

Is there anything that comes to your mind to make this more efficient? Is my assumption that the above points are not really worth to change wrong?

Please note that this is pre-C++11.

Without changing the algorithm, the only things I can see by having a look at the assembly of sqDiffSum:

read x[i] only once in the outer loop (i.e const double x_i = x[i] and then use x_i instead). You'll get a small win by not having to fetch it whenever you enter the if.
compute once i * scale - offset in the outer loop because this invariant. But latest gcc and clang seem smart enough to figure this out themselves.

Also, you might want to test std::unordered_map.

edit: whenever bin >= ref.size(), you know that it will be for all the next j. Maybe you even replace j by bin running (before morning coffee) when \$offset >= i \cdot scale\$ and \$\frac{i \cdot ref\_size}{x\_size} + offset < ref\_size\$