SSE instruction to check if byte array is zeroes C# My fundamental problem is how to check whether byte[] is full of zeroes. I posted a range of implementations (with timings) and one clearly beats others. In fact, it is so fast that I am having trouble believing it even works. I suggest taking a look at SO question page first. This is the winner code:

static unsafe bool IsAllZeros(byte[] data)
{
    fixed (byte* bytes = data) {
        int len = data.Length;
        int rem = len % (16*16);
        Vector16b* b = (Vector16b*)bytes;
        Vector16b* e = b + (len - rem) / (16*16);
        Vector16b zero = Vector16b.Zero;

        while (b+15 < e) {
            if (*(b)+*(b+1)+*(b+2)+*(b+3)+*(b+4)+*(b+5)+*(b+6)+*(b+7)+*(b+8)+
                *(b+9)+*(b+10)+*(b+11)+*(b+12)+*(b+13)+*(b+14)+*(b+15) != zero)
                return false;
            b += 16;
        }

        for (int i = 0; i < rem; i++)
            if (data [len - 1 - i] != 0)
                return false;

        return true;
    }
}

Project references Mono.Simd assembly. I checked the code for following issues so far:

Pointer arithmetic. Adding 1 to pointer moves it 16 bytes ahead. However loop is unrolled 16 times so iteration processes 256 bytes at a time.
Array length is in bytes while b e are in 16 byte vectors.
Loop condition checks if last element b+15 is within the array. End pointer e points to just outside of array.
Loop processes 16 vectors so it moves pointer by 16 elements.
Tail is processed for up to 15 vectors, each 16 bytes, one byte at a time.

Is this code correct?

This code is broken

When unrolling you add all the Vector16b.

From github.com/mono:

[Acceleration (AccelMode.SSE2)]
public static unsafe Vector16b operator + (Vector16b va, Vector16b vb)
{
    Vector16b res = new Vector16b ();
    byte *a = &va.v0;
    byte *b = &vb.v0;
    byte *c = &res.v0;
    for (int i = 0; i < 16; ++i)
        *c++ = (byte)(*a++ + *b++);
    return res;
}

So the vectors are added index to index, and then truncated back to byte. A sequence of adds can then end up as 0, fx:

16 + 16 + 16 + 16 + 16 + 16 + 16 + 16 + 16 + 16 + 16 + 16 + 16 + 16 + 16 + 16 = 256

256 in binary is 0b100000000 and when you truncate that to byte you get 0b00000000.

You need to ensure that each individual Vector16b is equal to zero or use | instead of +.


CountDownTimer that responds to clicks The app: Creates CountDownTimers and displays them in a simple layout. The user can choose how long the timer should be through SharedPreferences and a settings activity. Each timer has a preparation period before it actually starts.

The problem: I'm not sure how, but I feel like the code is messy and needs to be cleaned up. In particular, the CountDownTimer is nested five layers deep:

public void startSession(View v) {
    if (!isTicking) {
        // Runs on play press
        countDownTimer = new CountDownTimer(session.getPrepRemaining() + session.getMindRemaining(), 10) {

            @Override
            public void onTick(long millisUntilFinished) {
                if (millisUntilFinished > session.getMindRemaining()) {
                    // Preparation timer
                    session.setPrepRemaining(millisUntilFinished - session.getMindRemaining());
                    setPrepDigits(millisUntilFinished - session.getMindRemaining());
                } else {
                    // Mindfulness timer
                    prepDigits.setVisibility(View.INVISIBLE);
                    session.setMindRemaining(millisUntilFinished);
                    circularProgressBar.setProgress(((session.getMindLength() - millisUntilFinished) / (float) session.getMindLength()) * 100);
                    setMindDigits(millisUntilFinished);
                }
            }

            @Override
            public void onFinish() {
                session = null;
                isTicking = false;
                circularProgressBar.setProgress(100);
                initializeSession();
            }
        };
        countDownTimer.start();
        isTicking = true;
        startPauseButton.setImageDrawable(getResources().getDrawable(android.R.drawable.ic_media_pause));

        // Alarm
        AlarmReceiver.scheduleAlarm(this);
    } else {
        // Runs on pause press
        countDownTimer.cancel();
        isTicking = false;
        startPauseButton.setImageDrawable(getResources().getDrawable(android.R.drawable.ic_media_play));
    }
}

If you'd like to see the rest of the code, please check out the gist.

It seems like I should have one class managing the counting and another class managing UI, but again, I'm just not sure what steps I should take to make that happen. I also feel like there are several blocks of code that could be pulled out into their own methods, but I feel like they'd be too entwined with, for example, the timer itself.

I've been doing a lot of reading on clean code, and I'm aware of the principles, but I don't know how to put them into practice. So, how can I organize, simplify, and clarify my code?

I think you need to separate the CountDownTimer class logic instead of anonymous implementation.  I just made the below sample code which I try to loose coupled the CountDownTimer from the Activity

import android.annotation.SuppressLint;
import android.annotation.TargetApi;
import android.app.Activity;
import android.os.Build;
import android.os.Bundle;
import android.os.CountDownTimer;
import android.view.View;
import android.view.View.OnClickListener;
import android.widget.Button;
import android.widget.TextView;

import java.util.concurrent.TimeUnit;

@TargetApi(Build.VERSION_CODES.GINGERBREAD)
@SuppressLint("NewApi")
public class MainActivity extends Activity {
    Button btnStart, btnStop;
    TextView textViewTime;

    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
        btnStart = (Button) findViewById(R.id.btnStart);
        btnStop = (Button) findViewById(R.id.btnStop);
        textViewTime = (TextView) findViewById(R.id.textViewTime);
        textViewTime.setText("00:03:00");
        final CounterClass timer = new CounterClass(180000, 1000);
        btnStart.setOnClickListener(new OnClickListener() {
            @Override
            public void onClick(View v) {
                timer.start();
            }
        });
        btnStop.setOnClickListener(new OnClickListener() {
            @Override
            public void onClick(View v) {
                timer.cancel();
            }
        });
    }

    @TargetApi(Build.VERSION_CODES.GINGERBREAD)
    @SuppressLint("NewApi")
    public class CounterClass extends CountDownTimer {
        public CounterClass(long millisInFuture, long countDownInterval) {
            super(millisInFuture, countDownInterval);
        }

        @Override
        public void onFinish() {
            textViewTime.setText("Completed.");
        }

        @SuppressLint("NewApi")
        @TargetApi(Build.VERSION_CODES.GINGERBREAD)
        @Override
        public void onTick(long millisUntilFinished) {
            long millis = millisUntilFinished;
            String hms = String.format("%02d:%02d:%02d", TimeUnit.MILLISECONDS.toHours(millis),
                    TimeUnit.MILLISECONDS.toMinutes(millis) - TimeUnit.HOURS.toMinutes(TimeUnit.MILLISECONDS.toHours(millis)),
                    TimeUnit.MILLISECONDS.toSeconds(millis) - TimeUnit.MINUTES.toSeconds(TimeUnit.MILLISECONDS.toMinutes(millis)));
            System.out.println(hms);
            textViewTime.setText(hms);
        }
    }
}


Service locator for a 2D game I'm currently developing a simple 2D game engine and I was just wondering, what are some things that I can do to improve my service locator class, at the moment it's very basic.

For example, should I be checking for null objects, if so, what exceptions should I throw?

Here's the code, thatnks in advance!

using System;
using System.Collections.Generic;

namespace GameEngine.Core
{
    public sealed class ServiceLocator
    {
        private static ServiceLocator instance;

        private Dictionary<Type, object> services;

        public ServiceLocator()
        {
            this.services = new Dictionary<Type, object>();
        }

        public void AddService(Type serviceType, object serviceObject)
        {
            this.services.Add(serviceType, serviceObject);
        }

        public void RemoveService(Type serviceType)
        {
            this.services.Remove(serviceType);
        }

        public object GetService(Type serviceType)
        {
            return this.services[serviceType];
        }

        public static ServiceLocator Instance
        {
            get
            {
                if (instance == null)
                {
                    instance = new ServiceLocator();
                }

                return instance;
            }
        }
    }
}

I think you are reinventing the wheel a bit too much.  If you need something simple then go with built in .net ones.  If you going to make a simple ServiceLocator then you should implement the IServiceContainer interface.

public sealed class ServiceLocator : IServiceContainer

This is a matter of preference but I usually set static instance property in a static constructor.  There isn't a lot of overhead in creating the object right away that we need to do locking or delayed creation when they access the property.

public static IServiceContainer Instance { get; }

static ServiceLocator()
{
    Instance = new ServiceLocator();
}

Now I would create two constructors one the default and one that takes a IServiceContainer

private readonly IServiceContainer _container;

public ServiceLocator() : this(new ValidationContext(string.Empty).ServiceContainer)
{
}

public ServiceLocator(IServiceContainer container)
{
     if (container == null)
     {
         throw new ArgumentNullException(nameof(container));
     }
    _container = container;
}

I typically use the Servicecontainer from the ValidationContext class.  It's marked internal so we can not just new it up.  The difference between the ServiceContainer class and the one from the ValidationContext is two fold.  One the ServiceContainer class implements IDisposable and two it caches the results from the ServiceCreatorCallback delegate.   It would be up to you to decide which one of the ServiceContainers from .net you would prefer.

The rest of the methods I would just chain down to the container we set in the constructor

public object GetService(Type serviceType)
{
    return _container.GetService(serviceType);
}

public void AddService(Type serviceType, object serviceInstance)
{
    _container.AddService(serviceType, serviceInstance);
}

public void AddService(Type serviceType, object serviceInstance, bool promote)
{
    _container.AddService(serviceType, serviceInstance, promote);
}

public void AddService(Type serviceType, ServiceCreatorCallback callback)
{
    _container.AddService(serviceType, callback);
}

public void AddService(Type serviceType, ServiceCreatorCallback callback, bool promote)
{
    _container.AddService(serviceType, callback, promote);

}

public void RemoveService(Type serviceType)
{
    _container.RemoveService(serviceType);
}

public void RemoveService(Type serviceType, bool promote)
{
    _container.RemoveService(serviceType, promote);
}

This is a very simple Service Locator.   There are lots of very good dependency injection library out there but this will work if you only need a simple container.


Enumerating processes without enumerating flaws of the API This arose out of the incident where it was realized that WMI could not be expected to be reliable across all various computers. The next thing I looked at was API which was positively awful to work with. Based on a suggestion, I decided to try and use tasklist. Note that efforts was made to ensure there are no disk I/Os, which avoids getting into the messy realms of file management. The only annoyance is that the WshExec will pop open a window but that can be managed and is beyond the scope of the question.

The question is - can we make the process more reliable and failsafe? The idea is that it must be consistent across several computer systems, Windows versions, and so on. This makes uses of Windows Host Scripting Model and ADODB recordset. The application already requires ADODB anyway and the code can be updated to be late-bound as well. For testing/development, I left this in early-bound state.

Public Function EnumProcesses() As ADODB.Recordset
    Dim WshShell As IWshRuntimeLibrary.WshShell
    Dim WshExec As IWshRuntimeLibrary.WshExec
    Dim StdOut As IWshRuntimeLibrary.TextStream
    Dim Data As ADODB.Recordset
    Dim Output As String
    Dim ColumnLengths() As Long

    Set Data = New ADODB.Recordset
    Data.Fields.Append "ImageName", adVarChar, 255
    Data.Fields.Append "PID", adInteger, , adFldKeyColumn
    Data.Open

    Set WshShell = CreateObject("WScript.Shell")
    Set WshExec = WshShell.Exec("tasklist")
    Set StdOut = WshExec.StdOut

    Do While WshExec.Status = WshRunning
        If Not StdOut.AtEndOfStream Then
            Output = StdOut.ReadLine
            Select Case True
                Case Len(Output) = 0, _
                     Output Like "Image Name*"
                    'Skip
                Case Output Like "====*"
                    Dim SplitColumns As Variant
                    SplitColumns = Split(Output, " ")
                    ReDim ColumnLengths(UBound(SplitColumns))

                    Dim i As Long
                    For i = 0 To UBound(SplitColumns)
                        ColumnLengths(i) = Len(SplitColumns(i))
                    Next
                Case Else
                    Data.AddNew
                    Data.Fields("ImageName").Value = Mid$(Output, 1, ColumnLengths(0))
                    Data.Fields("PID").Value = Trim$(Mid$(Output, ColumnLengths(0) + 2, ColumnLengths(1)))
                    Data.Update
            End Select
        End If
    Loop

    Set EnumProcesses = Data
End Function

Apart from a performance question regarding ADODB recordsets, I only made one real change to your code. Since there are several fields that are output by the tasklist utility, I would want to capture all of that data just in case I need to expand my database at a later time. So I created a class called OSTask which accepts a single line from the tasklist output and parses it into its component parameters. (This means I could also skip the case you have to calculate column widths.)

Class OSTask

Option Explicit

Private Type InternalData
    ImageName As String
    PID As Long
    SessionName As String  'could also be an Enum: Console, Services
    SessionNumber As Long
    MemUsage As Long
End Type
Private this As InternalData

Public Property Get ImageName() As String
    ImageName = this.ImageName
End Property

Public Property Get PID() As Long
    PID = this.PID
End Property

Public Property Get SessionName() As String
    SessionName = this.SessionName
End Property

Public Property Get SessionNumber() As Long
    SessionNumber = this.SessionNumber
End Property

Public Property Get MemUsage() As Long
    MemUsage = this.MemUsage
End Property

Public Sub Init(ByVal taskData As String)
    '--- converts a single line output from the Windows command
    '    shell utility 'tasklist' and parses the data into the
    '    class properties
    Dim pos1 As Long
    Dim pos2 As Long

    '--- find the end of the task name, looking for double-space
    pos1 = InStr(1, taskData, "  ", vbTextCompare)
    this.ImageName = Trim$(Left$(taskData, pos1))

    '--- the next value is a number followed by a single space
    Dim i As Long
    For i = pos1 To Len(taskData)
        If Not Mid$(taskData, i, 1) = " " Then
            pos2 = InStr(i, taskData, " ", vbTextCompare)
            this.PID = CLng(Mid$(taskData, i, pos2 - i))
            Exit For
        End If
    Next i

    '--- next value is the session name
    pos1 = pos2 + 1
    pos2 = InStr(pos1, taskData, " ", vbTextCompare)
    this.SessionName = Trim$(Mid$(taskData, pos1, pos2 - pos1))

    '--- the next value is a number followed by a single space
    For i = pos2 To Len(taskData)
        If Not Mid$(taskData, i, 1) = " " Then
            pos2 = InStr(i, taskData, " ", vbTextCompare)
            this.SessionNumber = CLng(Mid$(taskData, i, pos2 - i))
            Exit For
        End If
    Next i

    '--- next value is the memory usage, a large number in thousands
    pos1 = pos2
    pos2 = InStr(pos1, taskData, "K", vbTextCompare)
    Dim memUsageText As String
    memUsageText = Mid$(taskData, pos1, pos2 - pos1)
    memUsageText = Replace$(memUsageText, ",", vbNullString)
    this.MemUsage = CLng(memUsageText) * 1000
End Sub

All of the properties are read-only in this case by design.

For my example, I converted your function to return a Collection rather than an ADODB.Recordset just to make my own testing simpler. So the only real change is in the Else case of the Select statement.

For my own learning purposes, I reviewed this answer's detailed review of the command shell interactions. Since you specifically stated that you are avoiding disk I/O, the option to pipe the output to a windows temp file is no good. To really prevent the command shell pop-up, you'd have to go with running a cscript under a wscript shell as the poster there indicates. Additionally, I couldn't find any historical information that the tasklist output has changed over time, so I believe your approach should remain viable across different Windows versions.

Here is my main module with my minor edits for testing:

Option Explicit

Sub test()
    Dim taskList As Collection
    Set taskList = EnumProcesses

    Dim task As Variant
    For Each task In taskList
        Debug.Print task.ImageName & ", " & task.MemUsage
    Next task
End Sub

Public Function EnumProcesses() As Collection
    Dim WshShell As IWshRuntimeLibrary.WshShell
    Dim WshExec As IWshRuntimeLibrary.WshExec
    Dim StdOut As IWshRuntimeLibrary.TextStream
    Dim Data As Collection
    Dim Output As String
    Dim ColumnLengths() As Long

    Set WshShell = CreateObject("WScript.Shell")
    Set WshExec = WshShell.Exec("tasklist")
    Set StdOut = WshExec.StdOut

    Set Data = New Collection

    Do While WshExec.Status = WshRunning
        If Not StdOut.AtEndOfStream Then
            Output = StdOut.ReadLine
            Select Case True
                Case Len(Output) = 0, _
                     Output Like "Image Name*"
                    'Skip
                Case Output Like "====*"
                    'Skip
                Case Else
                    Dim thisTask As OSTask
                    Set thisTask = New OSTask
                    thisTask.Init Output
                    Data.Add thisTask
            End Select
        End If
    Loop

    Set EnumProcesses = Data
End Function


Ticket sale income calculator I am looking for some useful feedback on my programming. I'm only three days in and trying to improve.

Here is the code:

#include <iostream>
using namespace std;
const double SECT_A_TICKPRICE = 20;
const double SECT_B_TICKPRICE = 15;
const double SECT_C_TICKPRICE = 10;

double getSeatsA();
double getSeatsB();
double getSeatsC();
double calcIncomeA(double);
double calcIncomeB(double);
double calcIncomeC(double);
void showIncome(double, double, double);

int main()
{
    double aSeats;
    double bSeats;
    double cSeats;
    double incomeSectA;
    double incomeSectB;
    double incomeSectC;

    aSeats = getSeatsA();

    bSeats = getSeatsB();

    cSeats = getSeatsC();

    incomeSectA = calcIncomeA(aSeats);

    incomeSectB = calcIncomeB(bSeats);

    incomeSectC = calcIncomeC(cSeats);

    showIncome(incomeSectA, incomeSectB, incomeSectC);

    return 0;

}

double geatSeatsA()
{
    double totalSeatsA;

    cout << "How many tickets were sold in Section A?" << endl;

    cin >> totalSeatsA;

        while (totalSeatsA < 0 && totalSeatsA > 300)
        {
            cout << "Enter a value between 0 and 300" << endl;

            cin >> totalSeatsA;
        }

    return totalSeatsA;
}

double geatSeatsB()
{
    double totalSeatsB;

    cout << "How many tickets were sold in Section B?" << endl;

    cin >> totalSeatsB;

        while (totalSeatsB < 0 && totalSeatsB > 500)
        {
            cout << "Enter a value between 0 and 500" << endl;

            cin >> totalSeatsB;
        }

    return totalSeatsB;
}

double geatSeatsC()
{
    double totalSeatsC;

    cout << "How many tickets were sold in Section C?" << endl;

    cin >> totalSeatsC;

        while (totalSeatsC < 0 && totalSeatsC > 200)
        {
            cout << "Enter a value between 0 and 200" << endl;

            cin >> totalSeatsC;
        }

    return totalSeatsC;
}

double calcIncomeA(double aSeats)
{
    double incomeA;

    incomeA = aSeats * SECT_A_TICKPRICE;

    return incomeA;
}

double calcIncomeB(double bSeats)
{
    double incomeB;

    incomeB = bSeats * SECT_B_TICKPRICE;

    return incomeB;
}

double calcIncomeC(double cSeats)
{
    double incomeC;

    incomeC = cSeats * SECT_C_TICKPRICE;

    return incomeC;
}

void showIncome(double incomeSectA, double incomeSectB, double incomeSectC)
{
    double allincome;

    allincome = incomeSectA + incomeSectB + incomeSectC;

    cout << "The income for Section A is ";

    cout << incomeSectA << endl;

    cout << "The income for Section B is ";

    cout << incomeSectB << endl;

    cout << "The income for Section C is ";

    cout << incomeSectC << endl;

    cout << "The total amount made from ticket sales is ";

    cout << allincome;

}

Don't use floating point numbers for representing money.
Don't using namespace std, it's a bad habit that will bite you down the line.
You've "double-spaced" pretty much all your code. That's too much vertical whitespace.
Try to declare your variables as late as possible, ideally only when you can initialize them. This avoids bugs linked to using uninitialized variables, and keeps their scope as small as possible which makes the code simpler to follow - less state to keep in mind. Don't declare variables  then initialize them one line after, just do both at once.

Example for this last point:

double incomeB;

incomeB = bSeats * SECT_B_TICKPRICE;

return incomeB;

Should be:

double incomeB = bSeats * SECT_B_TICKPRICE;
return incomeB;

Or better simply:

return bSeats * SECT_B_TICKPRICE;

Also:

double aSeats;
double bSeats;
double cSeats;

aSeats = getSeatsA();

bSeats = getSeatsB();

cSeats = getSeatsC();

Should be:

double aSeats = getSeatsA();
double bSeats = getSeatsB();
double cSeats = getSeatsC();

For the first point about money, simplest way out is to use int and compute all prices in cents. You can split out dollars and cents (or euros, whatever) on display. I'll use that for the rest of this review.
Also I doubt that buying 0.42 seats makes much sense, so the number of seats purchased/booked should use integers too.

Then look at your repeated functions, and look at what differences they have. Then try to make a more generic function, often by passing additional parameters.

Your getSeatsX functions only depend on a section name, and a number of available seats for that section. So put those in parameters.

int getSeatsForSection(std::string const& name, int maxSeats)
{
    std::cout << "How many tickets were sold in Section "
              << name << "?" << std::endl;

    int seats;
    std::cin >> seats;

    while (seats < 0 && seats > maxSeats)
    {
        std::cout << "Enter a value between 0 and "
                  << maxSeats << std::endl;
        std::cin >> seats;
    }

    return seats;
}

Your calcIncomeX functions only depend on the price of the seats in that section, so move that to a parameter too.

int calcIncome(int seats, int centsPerSeat)
{
    return seats * centsPerSeat;
}

So your main becomes:

const int SECT_A_TICKPRICE = 2000; // price in cents
// ...
int main()
{
  int seatsA = getSeatsForSection("A", 300);
  // ...
  int incomeA = calcIncome(seatsA, SECT_A_TICKPRICE);
  // ...
}

With a little helper to display dollar amounts, you can update your showIncome function easily:

void displayIncome(int cents)
{
  std::cout << (cents / 100) << " dollars and "
            << (cents % 100) << " cents" << std::endl;
}

(Or use a string stream and return the formatted number, either like above or in xxx.yy format.)

But that's still not very good. If you need to add a new section, you need to add a bunch of code in several different places.

I'd suggest the following:

Create a struct that represents sections. It should contain the section name, total number of seats and seat price.
Build a std::vector of all your sections.
Use that to loop over all sections for input and display.

Rough sketch of what main would look like with that:

int main()
{
  std::vector<section> sections = { /* ... */ };
  int totalIncome = 0;

  for (auto const& sect : sections) {
    int seats = getSeatsForSection(sect.name, sect.maxSeats);
    int income = calcIncome(seats, sect.seatPrice);
    displayIncomeForSection(sect.name, income);
    totalIncome += income;
  }
  displayTotalIncome(totalIncome);
}

There are lots of other options possible, depending on exactly what you need to do. This is just a suggestion.


JavaScript date validation I have a date control which has a dropdown for each date, month and year.

The issue is, I can select 31st Nov. 2015 as well, and when I create date object, I get 1st Dec. 2015.

Example

var _date = 31; // value fetched from dropdown
var _month = 10;
var _year = 2015;

var date = new Date(_year, _month, _date);
document.write(date)

I want to validate this condition, and my current approach is to match date.getDate() with _date and show an error message if they are different.

Is there a better way to implement this?

Code

var _monthList = [
    {id:"1",name:"January"} ,
    {id:"2",name:"February"} ,
    {id:"3",name:"March"} ,
    {id:"4",name:"April"} ,
    {id:"5",name:"May"} ,
    {id:"6",name:"June"},
    {id:"7",name:"July"} ,
    {id:"8",name:"August"} ,
    {id:"9",name:"September"} ,
    {id:"10",name:"October"} ,
    {id:"11",name:"November"} ,
    {id:"12",name:"December"}];

function createYears(){
 var _year = [];
    for (var i = 2000; i<=new Date().getFullYear(); i++){
     _year.push({id:i, name:i});
    }
    $("#cbYear").html(createOptions(_year));
}

function createMonths(){
 $("#cbMonth").html(createOptions(_monthList));
}

function createDates(){
 var _dates = [];
    for (var i=1; i<=31; i++){
     _dates.push({id:i, name:i});
    }
    $("#cbDate").html(createOptions(_dates));
}

function createOptions(dataset){
 var optionStr = "<option></option>";
    $.each(dataset, function(key, value){
        var id = value.id? value.id:key;
        var text = value.name;
        var val = value.value?value.value:value.id;
        optionStr += "<option id="+ id + " name='"+ text+ "' value='"+ val + "'>"+ text + "</option>"
    });
    return optionStr;
}

function registerEvents(){
 $("select").on("change", function(e){
     validateDate();
    });
}

function validateDate(){
 var _date = $("#cbDate option:selected").val();
    var _month = $("#cbMonth option:selected").val();
    var _year = $("#cbYear option:selected").val();
    if(_year && _month && _date){
        var dateObj = new Date(_year, _month-1, _date);
        if(_date != dateObj.getDate()){
            $("#lblError").text("Please Select a valid date");
        }
        else{
         $("#lblError").text("");
        }
    }
}

(function(){
 createYears();
    createMonths();
    createDates();
    registerEvents();
})()
select{
    background:#fff;
    color:blue;
    width:100px;
    padding:5px;
    border-radius:4px;
    border: 1px solid gray;
}

.error{
    color:red;
}

<select id="cbYear"></select>
<select id="cbMonth"></select>
<select id="cbDate"></select>

Use Case

Select: Invalid Date (e.g. 31st Nov.)

I have also created a JSFiddle for the same.

The logic you're using for validating a date looks fine to me.

The code is not bad, but here are a few suggestions for improvement:

IIFE

All of the defined variables and functions leak into global scope. Put them all inside the IIFE to avoid the scope leak: (Note the trailing semicolon)

(function(){
    var _monthList = ...
    function createYears() {
        ...
    }

    ...

    createYears();
    createMonths();
    createDates();
    registerEvents();
})();

And it's better to execute this function only when the document is ready, so it works regardless of when the script is loaded:

$(function() {
    ...
});

Naming

Don't prefix your variable names with an underscore. This convention is only used for private properties of objects, which you don't have in your code.
Use day instead of date when you mean the day of the month (e.g. createDays, days, #cbDay, day), Javascript's Date#getDate method is poorly named. Then you can rename dateObj to date.
Don't prefix your elements' IDs with cb (they're not callbacks). Simply use year, month and day (element IDs don't clash with variable/function names, so they don't need to be prefixed).
lblError -> errorLabel. No need to abbreviate.
dataset -> options, to be more descriptive.

Make each function do one thing

createYears both generates the year array and manipulates the DOM. Generate the array outside the function instead: (like you did for createMonths)

var yearList = (function() {
    var years = [];
    for (var i = 2000; i<=new Date().getFullYear(); i++){
        years.push({id:i, name:i});
    }
    return years;
})();

Do the same for createDays.

validateDate both checks if the date is valid and updates #errorLabel accordingly. Use a seperate function for testing if a date is valid:

function isDateValid(day, month, year) {
    var date = new Date(year, month-1, day);
    return day == date.getDate();
}

createOptions

Every time createOptions is called, the return value is set as the HTML value of an element. It's better to pass the element to createOptions as an argument and let it manipulate the element to eliminate this repetition.

Each option passed to this function is known to have id and name properties, so you don't need to check which properties are present.

You don't need to set the IDs for the options, it's enough to set the values. This means the id property of the options in the array argument should be renamed to value (Change id to value when generating the arrays).

Finally, you can use jQuery for creating the elements:

function createOptions(select, options) {
    options.forEach(function (option) {
        select.append($('<option>', { text: option.name, value: option.value }));
    });
}

Now createYears looks like this:

function createYears() {
    createOptions($('#year'), yearList);
}

Which is so short that there is no need to make createYears, createMonths and createDays multiple functions.

Seperating logic from DOM

Put all the functions that are responsible for the logic first, and after them put code that accesses the page's DOM: (i.e. code that has strings with specific element IDs)

var dayList = ...;
var monthList = ...;
var yearList = ...;

function createOptions(select, options){
    ...
}

function isDateValid(day, month, year) {
    ...
}

createOptions($('#year'), yearList);
createOptions($('#month'), monthList);
createOptions($('#day'), dayList);

$("select").on("change", function validateDate() {
    var day = $("#day").val(); /* NOTE: No need to use additional selectors when you're already selecting by ID */
    var month = $("#month").val();
    var year = $("#year").val();
    if(year && month && day) {
        /* NOTE: I used a ternary expression to shorten the code */
        $("#errorLabel").text(isDateValid(day, month, year) ? "" : "Please select a valid date");
    }
});

This makes the code easier to understand and more reusable (e.g. it will make having multiple date forms on the same page easier).

Good luck!


Remove list of files from another list of files by their name In groovy I have a list of files A and also another list of files B. I want to remove all the files from A that have a filename that also occurs in the list of files B. I wrote the following code:

List<File> removeByName(final List<File> files, final List<File> removalFiles) {
    final Set<String> removalNames = removalFiles.collect { it.name }.toSet()
    files.findAll { ! removalNames.contains(it.name) }
}

Unfortunately I cannot use the remove method in the Collection class because two files are not equal even if they have the same file name. Are there any methods that I am missing which could be used to make that code cleaner?

Groovy is weakly typed, so you could save a some cruft using def for local variables. Typing can give nice hints (great on an API), but it can also be ignored, leading to code lies and confusion. Final is also a pretty pointless keyword in Groovy.

There's also a shortcut to collecting properties: the spread operator *, which allows you to do removalFiles*.name. It's also more groovy to cast to a set with as Set.

If you really need to do an in-place change, what you want to do is collect all the file objects from the files collection that you want to remove in order to remove them. This way, you are removing using the exact same object:

void removeByNameInPlace(def files, def removalFiles) {
    def removalNames = removalFiles*.name as Set
    files.removeAll( files.findAll { removalNames.contains(it.name) } )
}

However this isn't very groovy and it's typical to return modified copies of Collections instead. This makes your code pretty close to ideal, functionally speaking. With some stylistic changes to your original code:

List<File> removeByName(List<File> files, List<File> removalFiles) {
    def removalNames = removalFiles*.name as Set
    files.findAll { ! removalNames.contains(it.name) }
}


Building and getting a form of objects that have multiple properties I'm building a form dynamically in a web app where I'm using product_id to keep track which product I'm reading and then product_<id>_property to grab the value of it. Then in my view I end up with this:

for product_id in request.POST.getlist('product_id'):
    ProductPlacement.objects.create(
        product = Product.objects.get(id=product_id),
        board = board,
        x = request.POST.get('product_{0}_x'.format(product_id)),
        y = request.POST.get('product_{0}_y'.format(product_id)),
        width = request.POST.get('product_{0}_width'.format(product_id)),
        height = request.POST.get('product_{0}_height'.format(product_id)),
        rotation = request.POST.get('product_{0}_rotation'.format(product_id)),
        z = 0
    )

It's not very pretty, is there a nicer more readable way of doing it?

Your code is quite ok and I have no idea how to improve this particular piece.

You have used several identical forms and processed it manually. Django have formsets for this purpose. Why don't you use them?

Then I don't see validation in your code. What happens when product_0_width contains not-numeric or empty value? Formsets would solve this problem.


File handling with try and except I am learning programming with Python. I am not sure if I can write a try/except block like this.

import os
import shutil
try:
    shutil.rmtree('c:/guna/newCAT1')
    print("dir newCAT1 deleted")

except:
    os.mkdir('c:/guna/newCAT1')
    print("dir newCAT1 created")
    fileIN=open('c:/guna/newCAT1/secretMSG.txt','w' )
    fileIN.writelines("the dog is green")
    fileIN.close()
else:
    print("exception code did not execute")
    os.mkdir('c:/guna/newCAT1')
    print("dir newCAT1 created")
    fileIN = open('c:/guna/newCAT1/secretMSG.txt', 'w')
    fileIN.writelines("the dog is green")
    fileIN.close()

You might want to ask a more specific question on Stack Overflow if you don't know if it works.

In the meantime, you should catch the most specific exception you can, so except OSError instead of a bare except.   Also, you should use exception code to mitigate a problem and continue, so more like:

try:
    shutil.rmtree('c:/guna/newCAT1')
    print("dir newCAT1 deleted")
except OSError:
    pass  # do nothing on this exception.
# continue with making the directory.  The exception statement is complete

or, if you read the documentation for shutil.rmtree:

shutil.rmtree('c:/guna/newCAT1', ignore_errors=True)
print("dir newCAT1 deleted")
os.mkdir('c:/guna/newCAT1')
print("dir newCAT1 created")
fileIN=open('c:/guna/newCAT1/secretMSG.txt','w' )
fileIN.writelines("the dog is green")
fileIN.close()


Loop through group and difference lags I have time-series data (t) that I want to loop through each t and take the difference of the previous five lags t-1 ... t-5.

This code works, but is inefficient. I'm not sure how to increase the performance and efficiency of the code.

Sample data

import pandas as pd
import numpy.random as nr

dat = pd.DataFrame({'t': [1, 2, 3, 4, 5, 6, 7, 8, 9, 10], 'rnum': nr.rand(10)})

Function

def diff_n(idat):
    # Get t from group
    i = idat['t'].iat[0]

    # If one, nothing to difference
    if i == 1:
        return None

    # Get value to loop through and diff
    h1 = idat.groupby('t')['rnum'].mean().values

    # Subset dat (outside fn)
    h2 = dat[(dat.t < i ) & (dat.t >= i - 5)]

    # Combine data for return
    retdat = pd.DataFrame()

    # Loop through t-1 to last obs
    for j in range(max(h2.t), min(h2.t), -1):

        # Get value to difference from h1
        h3 = h2[h2.t == j].groupby('t')['rnum'].mean().values

        # Difference
        ndiff = (h3 - h1)

        # Build df and concat
        indat = pd.DataFrame({'t': [i], 'lag': i - j, 'diff': ndiff})
        retdat = pd.concat([retdat, indat])
    return retdat

Groupby and apply the function to each t

dat.groupby('t').apply(lambda x: diff_n(x))

Output

>>> dat.groupby('t').apply(lambda x: diff_n(x))
      t  lag      diff
t
2  0   2    1  0.440644
3  0   3    1 -0.284075
   0   3    2  0.156569
4  0   4    1  0.439154
   0   4    2  0.155079
   0   4    3  0.595723
5  0   5    1 -0.095552
   0   5    2  0.343602
   0   5    3  0.059527
   0   5    4  0.500171
6  0   6    1  0.296337
   0   6    2  0.200784
   0   6    3  0.639939
   0   6    4  0.355864
   0   6    5  0.796507
7  0   7    1 -0.749913
   0   7    2 -0.453576
   0   7    3 -0.549128
   0   7    4 -0.109974
   0   7    5 -0.394049
8  0   8    1  0.506379
   0   8    2 -0.243534
   0   8    3  0.052803
   0   8    4 -0.042749
   0   8    5  0.396405
9  0   9    1 -0.021761
   0   9    2  0.484618
   0   9    3 -0.265295
   0   9    4  0.031042
   0   9    5 -0.064510
10 0  10    1  0.409118
   0  10    2  0.387357
   0  10    3  0.893736
   0  10    4  0.143823
   0  10    5  0.440160

First of all, nice code, very readable and good comments.

There are several improvements you can do:

Reindex your dataframe with t. You are writing a lot of times .t, if you will use index some of pandas index optimization will kick in and your code will be faster.
Do not do concat on only 2 items. Concatenate function will work much faster if you will just do all of the concatenation of all 5 elements at once.
Why do you need h2? Try to avoid unnecessary coping of the data frame. You can work on the original h1 or even idat since your function does not change anything implicitly.
Use type annotations. Using type annotations can make the code run faster since the interpreter does not need to figure out what is the variable type. Also, probably even more important, it helps readability.


Laravel token middleware I want to make my API unavailable to every client who doesn't have the token to access. This means the Android app will send a client as Android and token as token string in the header with keys client and token.

Now in middleware, I am checking it with my table fields to pass through authorization. If both match, then I will authorize and if don't then it will send a 403 response.

I am aware of Passport but it is not what I am looking for. In fact, consider it as a first layer of security and then use Passport as a second layer of security to authorize the API.

Is this code correct?

As I am not so familiar with Laravel - Middleware I just want to get some feedback from experts whether the code I have written is accurate and up to the standard. If not, I would appreciate your suggestion and help to make it better.

Middleware

namespace App\Http\Middleware;

use App\ApiToken;
use Closure;
use function response;

class ApiAccess
{
    /**
     * Handle an incoming request.
     *
     * @param  \Illuminate\Http\Request $request
     * @param  \Closure                 $next
     *
     * @return mixed
     */
    public function handle( $request, Closure $next ) {

        if ( $this->checkToken( $request ) ) {
            return $next( $request );
        }

        return response()->json( [ 'error' => 'Unauthorized' ], 403 );

    }

    public function checkToken( $request ) {

        $client = $request->header( 'client' );
        $token  = $request->header( 'token' );

        $checkToken = ApiToken::where( 'client', $client )
                              ->where( 'token', $token )->first();

        return $checkToken;
    }
}

API Route

I am fetching results from the ApiToken table just to check:

Route::get('/', function(Request $request) {
    return ApiToken::all();
})->middleware('apiAccess');

Optimized with Muhammad Nauman's answer here

public function checkToken( $request ) {

    $client = $request->header( 'client' );
    $token  = $request->header( 'token' );

    return ApiToken::where( 'client', $client )
                          ->where( 'token', $token )->exists();
// Nicer, and it will return true and false based on the existence of the token and client.
}

My take on this is the following:

rename the middleware to something meaningful (I took example from VerifyCsrfToken middleware)
throw exception if token is mismatched
rename checkToken to verify, reader of the code knows that code checks/verifies token because thats the point of the middleware
simplify even more verify function and add docblock
optimize the query to the following

select exists(select `id` from `tokens` where (`client` = 'Android' and `token` = 'OuK0ELzYkN3Ss9Zf')) as `exists`

So ApiAccess becomes following:

<?php

namespace App\Http\Middleware;

use App\Exceptions\TokenMismatchException;
use App\ApiToken;
use Closure;

class VerifyApiToken
{

    /**
     * Handle an incoming request.
     *
     * @param  \Illuminate\Http\Request $request
     * @param  \Closure                 $next
     *
     * @return mixed
     * @throws \App\Exceptions\TokenMismatchException
     */
    public function handle(Request $request, Closure $next)
    {
        if ($this->verify($request)) {
            return $next($request);
        }

        throw new TokenMismatchException;
    }

    /**
     * Verify token by querying database for existence of the client:token pair specified in headers.
     *
     * @param \Illuminate\Http\Request $request
     *
     * @return bool
     */
    public function verify($request): bool //optional return types
    {
        return ApiToken::select('id')->where([    // add select so Eloquent does not query for all fields
            'client' => $request->header('client'), // remove variable that is used only once
            'token'  => $request->header('token'),  // remove variable that is used only once
        ])->exists();
    }
}

Create new Exception, php artisan make:exception TokenMismatchException - yes kind of same as Laravel's stock one used when CSRF token is mismatched.

With body:

<?php

namespace App\Exceptions;

use Exception;

class TokenMismatchException extends Exception
{

    /**
     * Report the exception.
     *
     * @return void
     */
    public function report()
    {
        //
    }

    /**
     * Render the exception into an HTTP response.
     *
     * @param  \Illuminate\Http\Request $request
     *
     * @return \Illuminate\Http\Response|null
     */
    public function render($request)
    {
        if ($request->wantsJson()) { // if request has `Accept: application/json` header present
            return response()->json(['error' => 'Unauthorized'], 403);
        }

        return abort(403);
    }
}

  Note: remove use function response; statements because helpers are auto-loaded from Laravel's helpers


Makefile for multiple executables Any suggestions on how to improve the Makefile, e.g. how to best replace the multiple uses of example1/example2, ex1_src/ex2_src, ex1_obj/ex2_obj with static pattern rules?
build := ./build
targets := $(build)/example1 $(build)/example2
src := ./src

srcfiles := $(shell find $(src) \( -name "*.cpp" -or -name "*.c" \) ! -path '*/examples/*')
objects  := $(srcfiles:%=$(build)/%.o)

ex1_src := $(shell find $(src) -name example1.cpp)
ex1_obj := $(ex1_src:%=$(build)/%.o)

ex2_src := $(shell find $(src) -name example2.cpp)
ex2_obj := $(ex2_src:%=$(build)/%.o)

all_obj := $(objects) $(ex1_obj) $(ex2_obj)
depends := $(all_obj:%.o=%.d)
incdirs := $(shell find $(src) -type d ! -path '*/examples')
includes := $(addprefix -I,$(incdirs))

CXX := g++
CXXFLAGS := -std=c++17 $(includes) -Wall -MMD -MP
LDFLAGS := -lpthread

all: $(targets)

$(build)/example1: $(objects) $(ex1_obj)
$(build)/example2: $(objects) $(ex2_obj)

$(targets):
    $(CXX) $(LDFLAGS) $^ -o $@

$(build)/%.cpp.o: %.cpp
    @mkdir -p $(dir $@)
    $(CXX) $(CXXFLAGS) -c $< -o $@

.PHONY: clean
clean:
    rm -rf $(build)

-include $(depends)

It's usually best to have the Makefile in the same directory as the build products, or at least have that as the working directory.  Then there's no need to re-write the built-in rules for compiling and linking.  Use VPATH to ensure the source files can be found.

LDFLAGS := -lpthread

That should go into LIBS (which gets expanded later in the command line), so that it gets used as needed.  LDFLAGS is for flags such as -L which need expanding further to the left.
I'm not convinced that example1 and example2 are great names for your programs - surely you can think of something more descriptive and memorable for your users?

ex1_src := $(shell find $(src) -name example1.cpp)

Do you really need to invoke a find there?  I would expect there to be few enough matches that you could simply list them, and update the list when you add a new one.  I'd do the same for srcfiles too - or create a library, so that the resultant programs only include the objects they need.
I think generally there's too much use of shells here, despite them all being in := assignments.  You want makefile parsing to be fast, so you can see all your unit test results as quickly as possible.
So I'd write
example1: example1.o object_a.o object_b.o object_c.o
example1: LINK.o = $(LINK.cc)

The target-specific LINK.o is necessary so that the C++ linker is used.  It's cleaner and more portable than adding the C++ runtime library to LIBS.
You're missing some very important CXXFLAGS:
CXXFLAGS += -Wall -Wextra

I'd likely add a few more:
CXXFLAGS += -Wpedantic -Warray-bounds -Weffc++
CXXFLAGS += -Werror

Good use of .PHONY - that's often overlooked.
You should also have .DELETE_ON_ERROR: so that interrupted builds don't cause problems.


Shortest path around a set of points Problem
I'm trying to get into Python and get a bit familiar with it, so I found a problem online to train. The problem was to find the shortest path around some points, given a set of nodes which constitute the path and a set of points it must go around
I took the problem from here, but it's all in Czech so I don't know to what extent this will be helpful: https://kasiopea.matfyz.cz/soutez/F/
Algorithm
Since the providen set of edges isn't too large (<300) I iterate through all edges and check if all points are on just one side of the edge (since the shape must obviously be convex). Once I found all the edges I would run them against the Floyd Warshall algorithm to find out the shortest path length from one start node to itself.
Code
from itertools import combinations
from math import sqrt

INF = float("inf")

def floyd_warshall(graph):
    N = len(graph)
    dist = [[col for col in row] for row in graph]
    for k in range(N):
        for i in range(N):
            for j in range(N):
                dist[i][j] = min(dist[i][k] + dist[k][j], dist[i][j])
    return dist

def vector(u, w):
    return (w[0] - u[0], w[1] - u[1])

with open("input.txt") as f:
    T = int(f.readline())
    for _ in range(T):
        N = int(f.readline())
        M = int(f.readline())

        hedges = []
        bombs = []

        for _ in range(N):
            p = f.readline().split()
            x, y = int(p[0]), int(p[1])
            hedges.append((x, y))

        for _ in range(M):
            p = f.readline().split()
            x, y = int(p[0]), int(p[1])
            bombs.append((x, y))

        graph = [[INF] * len(hedges) for i in range(len(hedges))]
        for (ui, u), (wi, w) in combinations(enumerate(hedges), 2):
            edge = vector(u, w)
            side = None
            for bomb in bombs:
                point = vector(u, bomb)
                cross = edge[0]*point[1] - edge[1]*point[0]
                if cross == 0:
                    break
                if side == None:
                    side = cross > 0
                elif side != (cross > 0):
                    break
            else:
                if side != None:
                    dist = sqrt(edge[0]**2 + edge[1]**2)
                    if side:
                        graph[ui][wi] = dist
                    else:
                        graph[wi][ui] = dist

        solved = floyd_warshall(graph)
        cir = INF
        for i in range(len(hedges)):
            cir = min(solved[i][i], cir)

        if cir == INF:
            print(-1)
        else:
            print(cir)

Please don't mind the naming, the problem was presented a bit differently so I used the names for that instead.
Example input
1
4
1
1 1
4 1
4 4
1 4
2 2

Example output
10.24264068711928

I was wondering if there could be a faster solution for this problem and given the fact I'm coming from Java, if there are any things that could be more "pythonic", things I should avoid or stuff that could generally be better. Also I want to point out my solution is working, I'm just wondering if there's a better way to do it.

I assume your algorithm is correct and will comment only coding style

I would change INF = float("inf") to INF = math.inf (also you have to import it), since you use python 3.x (I'll asume you can use newest version) inf is now in math package.

Do not put any algorithms directly to the file (I am thinking about with open("input.txt") as f: and so on...) create a function - this is a good pracice because if someone would ever wanted to load your package he wouldnt like to start algorithm right away and also it helps if it comes to testing (you cannot test file but you can test a function).

Regarding a tuple construct

def vector(u, w):
    return (w[0] - u[0], w[1] - u[1])

I would love to see namedtuple here, it is kind of named tuple but it helps other programmers to see what the object exactly is and improve readibility (you would use edge.x instead of edge[0]). It also helps IDE to intelisence it.

Regarding T = int(f.readline()):

If (and only if) you assume that input file is correct such lines are fine but if not a try-except(python's try-catch) closure would be great here. The same thing might be applied to all the parsing in your script.

Note about performance:
I cannot see any real code improvements if it comes to pure pythonic way, in my opinion if you want a faster solution you should try to come up with faster algorithm (maybe some assumptions for your set that would reduce edges to check?).


Dataset and frequency list generation by looping over files I have been told that it would be wise to split up my code into semantically useful blocks. I tried the following but I need feedback.

What I already did:

As you can see I create two .csv files, so it seemed logical to have two functions. One for the main dataset, the other for a frequency table
I only import modules where I need them, though I did need some globally.

I probably should give fn, c and s better variable names. However, I'm not sure how to name them because they are identical to the column names they will be assigned to (e.g. node = node, sentence = sentence).

So what else can I do to better format my code into useful functions? Do note that I want to put an emphasis on performance, so any edits that decrease performance are discouraged.

import os, pandas as pd, numpy as np

from datetime import datetime

start_time = datetime.now()

# Create empty dataframe with correct column names
column_names = ["fileName", "component", "precedingWord", "node", "leftContext", "sentence" ]
df = pd.DataFrame(data=np.zeros((0, len(column_names))), columns=column_names)

# Create correct path where to fetch files
subdir = "rawdata"
path = os.path.abspath(os.path.join(os.getcwd(), os.pardir, subdir))

def main_dataset():
    import regex as re
    from html import unescape
    # Loop files in folder
    filenames = [name for name in os.listdir(path) if re.match(".*?[.]lst", name)]

    # "Cache" regex
    # See http://stackoverflow.com/q/452104/1150683
    p_filename = re.compile(r"[./\\]")

    p_sentence = re.compile(r"<sentence>(.*?)</sentence>")
    p_typography = re.compile(r" (?:(?=[.,:;?!) ])|(?<=\( ))")
    p_non_graph = re.compile(r"[^\x21-\x7E\s]")
    p_quote = re.compile(r"\"")
    p_ellipsis = re.compile(r"\.{3}(?=[^ ])")

    p_last_word = re.compile(r"^.*\b(?<!-)(\w+(?:-\w+)*)[^\w]*$", re.U)

    fn_list = []
    c_list = []
    pw_list = []
    n_list = []
    lc_list = []
    s_list = []

    for filename in filenames:
        with open(path + '/' + filename, 'r+', encoding="utf-8") as f:
            [n, c] = p_filename.split(filename.lower())[-3:-1]
            fn = ".".join([n, c])
            for line in f:
                s = p_sentence.search(unescape(line)).group(1)
                s = s.lower()
                s = p_typography.sub("", s)
                s = p_non_graph.sub("", s)
                s = p_quote.sub("'", s)
                s = p_ellipsis.sub("... ", s)

                if n in re.split(r"[ :?.,]", s):
                    lc = re.split(r"(^| )" + n + "( |[!\",.:;?})\]])", s)[0]

                    pw = p_last_word.sub("\\1", lc)

                    fn_list.append(fn)
                    c_list.append(c)
                    pw_list.append(pw)
                    n_list.append(n)
                    lc_list.append(lc)
                    s_list.append(s)
                continue

    # Assign data frame
    df['fileName'] = fn_list
    df['component'] = c_list
    df['precedingWord'] = pw_list
    df['node'] = n_list
    df['leftContext'] = lc_list
    df['sentence'] = s_list

    # Reset indices
    df.reset_index(drop=True, inplace=True)

    # Export dataset
    df.to_csv("dataset/py-dataset.csv", sep="\t", encoding="utf-8")

def frequency_table():
    # Define neuter and non_neuter
    neuter = ["het"]
    non_neuter = ["de"]

    # Create crosstab
    df.loc[df.precedingWord.isin(neuter), "gender"] = "neuter"
    df.loc[df.precedingWord.isin(non_neuter), "gender"] = "non_neuter"
    df.loc[df.precedingWord.isin(neuter + non_neuter) == 0, "gender"] = "rest"

    freqDf = pd.crosstab(df.node, df.gender)

    freqDf.to_csv("dataset/py-frequencies.csv", sep="\t", encoding="utf-8")

    # How long has the script been running?
    time_difference = datetime.now() - start_time
    print("Time difference of", time_difference)

main_dataset()
frequency_table()

Styling
Some minor edits, but you should import each module on a new line, especially if you're assigning them different names.
import os
import numpy as np
import pandas as pd

It seems redundant here to assign subdir for just a single use. If there's plans to use it later sure, but as it stands this should be fine.
path = os.path.abspath(os.path.join(os.getcwd(), os.pardir, "rawdata"))

I know you said that you wanted to save on performance, but when you instantly call this function anyway you're always going to run these imports, so they should be at the top with the rest.
import regex as re
from html import unescape

The p_quote regex string is unnecessary when you can just use '"'. Python sees no real difference between strings bound in single or double quotes, meaning you can easily reference them like this.
p_quote = re.compile('"')

Don't use path + '/' + filename to set a file path. Use the os module. It performs certain operations based on the OS Python is being run on. os.path.join(path, filename) will work cross platform.
Also there's no real point to opening the file in r+ mode, as that allows you to read and write but all you ever do is read the file anyway.
As for your naming conundrum, I'd just use the actual column names. They're not going to actually clash since you call the columns with strings so there's a clear delineation from what is a list and what's a string. And what can be clearer than using the same name. Also generally you can leave list out of a variable name except if you actually need to distinguish it from others.
The exception to both in your case of course being that you have a separate list of filenames, so I've given that a different name but you might find a different name more suitable.
filenameDataList.append(filenameData)
components.append(component)
precedingWords.append(precedingWord)
nodes.append(node)
leftContexts.append(leftContext)
sentences.append(sentence)

...

df['sentence'] = sentences

Minor Performance
These are two notes that wont significantly reduce your speed from file to file because they're not affecting the regex but they will save you a fraction of a second per file which can build up if you're running it on many files.
You don't need to set n and c as part of a list, you can just comma separate them and python will understand. And it's faster to not create an unneeded list.
[n, c] = p_filename.split(filename.lower())[-3:-1]

There's also no reason to use join here. It's slower than just string concatenation since it's just two items and you're creating the list for the sake of the join.
fn = n + "." + c

Refactoring
I would put all the imports together as I said above. Also I would put all the regex compilations near the top. They're effectively a constant so it's best to always keep constants near the top of the file rather than in a function where they'll be recreated the exact same (not that this affects your performance). This would mean that you have this all before your data frame is created:
import numpy as np
import os
import pandas as pd
import regex as re

from html import unescape
from datetime import datetime

start_time = datetime.now()

p_filename = re.compile(r"[./\\]")
p_last_word = re.compile(r"^.*\b(?<!-)(\w+(?:-\w+)*)[^\w]*$", re.U)
p_sentence = re.compile(r"<sentence>(.*?)</sentence>")
p_typography = re.compile(r" (?:(?=[.,:;?!) ])|(?<=\( ))")
p_non_graph = re.compile(r"[^\x21-\x7E\s]")
p_quote = re.compile(r"\"")
p_ellipsis = re.compile(r"\.{3}(?=[^ ])")

As for splitting up functions, I don't personally think that's necessary as your functions aren't overly long and it's a relatively specific process without a lot of repeating code, so I would keep to the two separate ones you have here.


Largest palindrome made from the product of two 3-digit numbers I am working on an interview question in which I need to find the largest palindrome made from the product of two 3-digit numbers. Here is the question.

public class PalindromeThreeDigits {

    public static void main(String[] args) {
        int value = 0;
        for(int i = 100;i <=999;i++) {
            for(int j = i;j <=999;j++) {
                int value1 = i * j;
                StringBuilder sb1 = new StringBuilder(""+value1);
                String sb2 = ""+value1;
                sb1.reverse();
                if(sb2.equals(sb1.toString()) && value<value1) {
                    value = value1;
                }
            }
        }

        System.out.println(value);
    }
}

Are there any optimizations/simplifications that we can do in this program?

When checking to see if a number is a palindrome, you effectively do:

StringBuilder sb1 = new StringBuilder(""+value1);
String sb2 = ""+value1;
sb1.reverse();
if(sb2.equals(sb1.toString()) && ....) {

That should be extracted to a function:

private static boolean isPalindromeSB(final int value1) {
    StringBuilder sb1 = new StringBuilder(""+value1);
    String sb2 = ""+value1;
    sb1.reverse();
    return sb1.equals(sb2);
}

so that you can use it as follows:

if (isPalindromeSB(value1) && ....) {

That neatens up your code, and does what is called "Functional Extraction".

Additionally, if you look at that function it does a lot of work... it does 2 string concatenations, creating three Strings, a StringBuilder, and a bunch more.

It would be more efficient to keep everything as numbers, and have a function like:

private static boolean isPalindrome(final int product) {
    int p = product;
    int reverse = 0;
    while (p != 0) {
        reverse *= 10;
        reverse += p % 10;
        p /= 10;
    }
    return reverse == product;
}

Note that this creates only two int values.... and it loops once per digit.

I tested it, and it is about 4 times faster than the String version.


Code to implement the Jaro similarity for fuzzy matching strings Problem
This code is meant to solve the problem in this software engineering question. To summarize the issue: given a set of strings that are sort of similar, but not similar enough to use regex upon, how would one get out a set of those "similar strings" amongst the "nonsimilar" strings? This is apparently called fuzzy matching strings, which I personally think is a fabulous name, but moving on. I looked around and hit upon the Jaro similarity as what seemed like a decently simple way to solve the problem. I'm particularly interested in seeing how the functions can be improved. It is written in Python 3.
Approach
First, in the match function, I convert each string into sets and find the intersection of the sets. Then, in the technical_match() function, I calculate the maximum distance to satisfy the requirements of the Jaro similarity, and then find the index of each item in the set of matches in each string, and subtract the two indices to find the distance. If this distance is less than the maximum distance, then it is a "true match" and is appended to a list of these.
Then, I calculate the number of transpositions necessary, i.e., given hello and jello, there are none necessary, because the strings produced when non-matches are stripped out are 'ello' and 'ello'. However, if I have 'cello' and 'ecllo', then it requires a transposition to switch c and e. I calculate this by producing a string from the match letters ordered as per their position in the actual string and then compare each for differences using diff_letters().
With these calculations in hand, I can then calculate the actual Jaro similarity, which is a pretty simple formula.
Finally, I created a fake file, foobar.txt, with some words, and open it, read it, split it by line breaks, and go through each and match to a pattern. If the Jaro similarity is greater than a certain constant, in this case 0.5 (the max, if both strings are the same, being 1, I believe), than that value is added to a list, and the list is printed at the end.
Code
import math

def match(s1, s2):
    set_of_matches = set.intersection(set(s1), set(s2))
    return set_of_matches

def technical_match(s1, s2):
    matches = match(s1, s2)
    max_distance = math.floor(max(len(s1), len(s2)/2)) - 1
    true_list = []
    for i in matches:
        distance = abs(s1.index(i) - s2.index(i))
        if distance <= max_distance:
            true_list.append(i)
    return true_list

def diff_letters(seq1, seq2):
    return sum(1 for a, b in zip(seq1, seq2) if a != b)

def transpositions(s1, s2):
    t = list(technical_match(s1, s2))
    s1_list = []
    s2_list = []
    for i in s1:
        if i in t:
            s1_list.append(i)
    for i in s2:
        if i in t:
            s2_list.append(i)
    s1 = ''.join(s1_list)
    s2 = ''.join(s2_list)
    return diff_letters(s1, s2)

def jaro_similarity(s1, s2):
    matches = len(technical_match(s1, s2))
    if matches == 0:
        return 0
    else:
        return 1/3*(matches/len(s1) + matches/len(s2) + (matches + transpositions(s1, s2))/matches)

match_text = open('foobar.txt', 'r').read().splitlines()
pattern = 'hat'
constant = .5

results = []
for i in match_text:
    if jaro_similarity(i, pattern) > constant:
        results.append(i)

print(results)

Fake text file and output
I used a file called foobar.text which contained
cheat
chat
choose
hat
hot

and ran the code on it (see here if you wish to run it) and received the output ['cheat', 'chat', 'hat', 'hot'].
Review
I'd be glad to accept all comments, as I need all the improvement I can get in my coding style. I am fairly sure this works, as I've tested it on a variety of inputs, but if there's a bug that's slipped through, I apologize; I did my best implementing the Jaro similarity, but the wikipedia article was a tad bit confusing in places.

Style

PEP8 advocates two blank lines before function definition at top-level code. You should also try to reduce line length such as your long return in jaro_similarity, taking advantage of implicit line continuation and splitting before operators you could rewrite it as:

def jaro_similarity(s1, s2):
    matches = len(technical_match(s1, s2))
    if matches == 0:
        return 0
    else:
        return (
                matches / len(s1)
                + matches / len(s2)
                + (matches + transpositions(s1, s2)) / matches
        ) / 3

You should also avoid leaving code at top-level and, instead, put it in a main function. Also, get yourself familiar with the if __name__ == '__main__': guard:

def main(filename='foobar.txt', pattern='hat', constant=.5):
    match_text = open(filename).read().splitlines()
    results = []
    for i in match_text:
        if jaro_similarity(i, pattern) > constant:
            results.append(i)
    return results

if __name__ == '__main__':
    print(main())

Code

you open a file but never close it, prefer using a with statement when working with file;
your match function should be more understandable if shortened to return set(s1) & set(s2) which uses the mathematical notation for set intersection; in fact, since it is only used once, it would be even clearer if the function was not defined at all;
technical_match can be simplified a lot using either a list-comprehension or the filter builtin;
instead of re-computing the matched letters in transpositions, you could pass them as parameter; besides, using a set instead of a list let you perform containing checks in \$\mathcal{O}(1)\$ time vs \$\mathcal{O}(n)\$ for  lists; and again, filter is your friend;
you don't need math.floor as you can perform integral division using the // operator.

Proposed rewrite:

def technical_match(s1, s2):
    max_distance = max(len(s1), len(s2)//2) - 1

    def is_close(letter):
        return abs(s1.index(letter) - s2.index(letter)) <= max_distance

    return filter(is_close, set(s1) & set(s2))

def transpositions(s1, s2, matches):
    s1 = ''.join(filter(matches.__contains__, s1))
    s2 = ''.join(filter(matches.__contains__, s2))
    return sum(a != b for a, b in zip(s1, s2))

def jaro_similarity(s1, s2):
    matches = set(technical_match(s1, s2))
    if not matches:
        return 0
    else:
        match_count = len(matches)
        return (
                match_count / len(s1)
                + match_count / len(s2)
                + (match_count + transpositions(s1, s2, matches)) / matches
        ) / 3

def main(filename='foobar.txt', pattern='hat', threshold=.5):
    with open(filename) as f:
        for line in f:
            line = line.strip()
            if jaro_similarity(line, pattern) > threshold:
                yield line

if __name__ == '__main__':
    print(list(main()))

Algorithm

Your code don't implement Jaro similarity it may give close results but it is not the right algorithm. For starter, you don't substract half the transpositions in the third term of the sum, instead you add them fully; you also failed to correctly compute the maximum allowed distance which is \$\frac{\max(\text{len(s1)},\text{len(s2)})}{2}\$ instead of \$\max(\text{len(s1)},\frac{\text{len(s2)}}{2})\$. Second, using sets in technical_match (well, match actually) lead you to completely ignore repeated letters: think things like matlab vs mathematica where the second a in both words is a match. (Your version return 3 matches, the algorithm consider there are 4 of them.)

The french version of the wikipedia page gives useful examples of application showing that the algorithm instead uses a sliding window to check for matches. I wondered how duplicate letters should be handled (think of goose vs hot, is there 1 or 2 matches?) until I found a C# implementation on SO: a letter that already matched should not match again (this is the first check in the first for loop).

Converting the code to Python is almost straightforward:

import itertools

def jaro_similarity(string1, string2):
    if not string1:
        return float(not string2)

    window_size = max(len(string1), len(string2)) // 2 - 1
    matched1 = [False] * len(string1)
    matched2 = [False] * len(string2)

    matches = 0
    for i, char in enumerate(string1):
        start = max(0, i - window_size)
        end = i + window_size + 1
        for j, c in enumerate(string2[start:end], start):
            if matched2[j]:
                # Account for repeated characters matching a single one
                # in the other string. Like `goose` vs `hot`, only a
                # single `o` of `goose` should match the only `o` in `hot`
                continue
            if c == char:
                matched1[i] = True
                matched2[j] = True
                matches += 1
                break

    if not matches:
        return 0.0

    matches1 = itertools.compress(string1, matched1)
    matches2 = itertools.compress(string2, matched2)
    transpositions = sum(a != b for a, b in zip(matches1, matches2))

    return (
            matches / len(string1)
            + matches / len(string2)
            + (matches - transpositions/2) / matches
    ) / 3

def main(filename='foobar.txt', pattern='hat'):
    print('Comparing to pattern:', pattern)
    with open(filename) as match_text:
        for line in match_text:
            line = line.strip()
            print(line, jaro_similarity(pattern, line))

if __name__ == '__main__':
    main(pattern='hat')
    main(pattern='goose')


Handle HTTP and BLE requests Im implementing and designing a workflow that performs both HTTP and BLE requests.

Im having trouble figuring out the best approach to design a service to handle both types of requests, and manage state.

I thought to make a base class Request with an execute method that all types of requests will conform to.

BLE and HTTP are quite different and dont share any attributes, so I made an enum to handle their associated values. It includes a provider which is a client that exposes api to perform actual BLE or HTTP request.

enum RequestCode: UInt {
    case getStatus             = 0x01
    case download              = 0x02
    case confirm               = 0x03
    case cancel                = 0x04
    case getAuthToken          = 0xF5
}

enum RequestType {
    case BLE(code:RequestCode, provider:BLEProvider, characteristic: CBCharacteristic, service: CBService)
    case HTTP(code:RequestCode, provider:NetworkProvider, data: Data, accessToken:String, method:HTTPMethod, params:[String:Any]?)
}

class Request {

    var type: RequestType

    init(type: RequestType) {
        self.type = type
    }

    func execute() {

        switch type {

        case .BLE(let code, let provider, let characteristic, let service):
            provider.writeValue(data: code.rawValue, characteristic: characteristic, service:service)

        case .HTTP(let code, let provider, let data, let accessToken, let method, let params):
            provider.performRequest(data: data, token: accessToken, method: method, params:params)

        }

    }
}

Questions

What is a better way to structure my requests, to handle both BLE and HTTP?
How can I handle transitions from one state/event to the next? Does it make sense to try to use promises, where I can call execute on each request, and chain them all together? If not, should I think of adding each request to a queue, if and how could a state machine be designed around RequestCode which are the events in the sequence?

You should separate concerns whenever possible. Since BLE & HTTP requests are quite different, you could write the following protocols:

protocol Request {
    associatedtype Provider
    var provider: Provider { get }

    func execute()
}

protocol BLERequest: Request {
    typealias Provider = BLEProvider

    var code: UInt { get }
    var characteristic: CBCharacteristic { get }
    var service: CBService { get }
}

protocol HTTPRequest: Request {
    typealias Provider = NetworkProvider

    var data: Data { get }
    var accessToken: String { get }
    var method: HTTPMethod { get }
    var params: [String:Any]? { get }
}

Then comes the magic; you can write generic extensions on your Request protocol, for each specific Request type.

extension Request where Self: BLERequest {
    func execute() {
        // provider is available since it is a get-able requirement,
        // and we know it is a BLEProvider instance
        // same goes for all the other get-able vars
        provider.writeValue(data: code, characteristic: characteristic, service: service)
    }
}

extension Request where Self: HTTPRequest {
    func execute() {
        // Again, same thing, we have all vars we need
        provider.performRequest(data: data, token: accessToken, method: method, params: params)
    }
}

Then, when you are implementing a new Request, all you need to do is implement the requirements for your chosen protocol, and everything else will be handled for you :)

Example:

enum MyBLERequest: BLERequest {
    case getStatus
    case download

    // MARK: BLE Request conformance
    var provider: Provider {
        return BLEProvider(... whatever init is needed)
    }

    var code: UInt {
        switch self {
        case .getStatus: return 0x01
        case .download: return 0x02
        // Any further new case will require you to implement the code here, so you have compile-time safety :)
        }
    }

    var characteristic: CBCharacteristic {
        return CBCharacteristic(.. init)
    }

    var service: CBService {
        return CBService(... more init)
    }
}

Then you can simply do MyBLERequest.getStatus.execute(), and it's all done through the protocol extension code.

And since Swift Enums allow associated values, you can always pass in additional parameters, whenever needed (you could have another case case getAuthToken(byUserID: String), which are then available to you in your implementation)

Now, since you have this all neatly separated, I would go further and pass your Request instances to each specific Provider directly. In effect, you are not the one executing a request, a provider is. That means your requests are agnostic to whatever Provider executes them, which will allow you to set up unit testing easier (because then you can write Mock Providers, and still pass them your requests like usual).

So you should probably extend each provider like such:

extend BLEProvider {
    func execute(request: BLERequest) {
        writeValue(code: request.code,
                   characteristic: request.characteristic,
                   service: request.service)
    }
}


Knapsack problem with duplicate elements I solved the knapsack problem where duplicates elements are allowed:

  Given weights and values related to n items and the maximum capacity allowed for these items. What is the maximum value we can achieve if we can pick any weights any number of times for a total allowed weight of W?

  input:

  W = 10

  weight = [2,3,6,4]

  cost = [3,4,6,10]

  output:

  23

I have written this code. Is there any way to improve it?

def knapsack(W, weight, cost):
    #import pdb; pdb.set_trace()
    table = [0] * (W+1)
    for w in xrange(W+1):
        max_so_far = 0
        for i, wt in enumerate(weight):
            if wt <= w:
                cur = cost[i] + table[w-weight[i]]
                if cur > max_so_far:
                    max_so_far = cur
        table[w] = max_so_far
    print table
    return table[W]

weight = [2,3,6,4]
cost = [3,4,6,10]
print knapsack(10,weight,cost)

table is a meaningless name for an important data structure. It should be explained by a comment at least.
W and w look similar enough to cause confusion.
Instead of using enumerate, you could iterate over zip(weight, cost) and avoid indexing.
The max builtin function is convenient for computing a maximum. The inner for loop can be converted to a generator expression for max to process. However, max raises an error if there are no items to process. In recent versions of Python a default value can be given to max to avoid that.

My rewrite for Python 3.4 or later (I see you are using Python 2.x):

def knapsack(W, weights, costs):
    '''Given weights and costs of items, compute maximum cost for total
    allowed weight W when any item can be picked any number of times.
    '''
    best = [0] # best[x] is the max cost for allowed weight x
    for capacity in range(1, W + 1):
        best.append(max((cost + best[capacity - weight]
                         for weight, cost in zip(weights, costs)
                         if weight <= capacity),
                        default=0))
    return best[W]


Is there any loss in performance converting a 'where' clause to a lambda? Is there any different in performance for the following 2 queries.
I'm just wondering what is the better of the two:

var res1 = (from a in _ctx.DataContext.Actions
            join e in _ctx.DataContext.Events on a.EventId equals e.EventId
            select a).Single(a => a.ActionId == actionId);

or

var res2 = (from a in _ctx.DataContext.Actions
            join e in _ctx.DataContext.Events on a.EventId equals e.EventId
            where a.ActionId == actionId
            select a).Single();

There should not be any performance difference that is a result of the syntax used. The query syntax is just eye candy that gets converted to the same underlying code.  The difference between the two is really just

_ctx.DataContext.Actions
.Join(_ctx.DataContext.Events, blah, blah, blah)
.Single(a=>a.ActionId == actionId);

vs.

_ctx.DataContext.Actions
.Join(_ctx.DataContext.Events, blah, blah, blah)
.Where(a=>a.ActionId == actionId)
.Single();

If there's a performance difference, I'd be very surprised.  The only way to really tell is to run some tests.  I personally prefer your second method or my first; as I do not like mixing query syntax with imperative syntax.


Frequency distribution of words and phrases I need to find a way how to create a frequency distribution out of multilple text files. In fact I am asked to check the number of times a word or a phrase occurs in a txt file. The code should check from a predefinded list (my list is called l1) how often this word or phrase can be found in the doc. My output should look like the following:

UNIQA VERSICHERUNGEN:31.12.2008
acceptance  2
acceptance credit   0
acceptance sampling 0
accounting principles   10
accounting principles board 0
additional  30
corporate   36
corporate bond  0
corporate finance   0
corporate governance    15

My code looks as follows. It does what it should do, but it is very slow (takes about a minute per file to process).

    from collections import Counter
    from itertools import chain
    import re
    import os
    import glob
    from nltk.tokenize import *
    import nltk
    from os import listdir

    def removeNonAscii(s): return "".join(i for i in s if ord(i)<128)

    def read_textfile(filename):
        # Reads the entire content of FILENAME and returns a non Ascii letters cleaned string
        infile = open(filename)
        contents = removeNonAscii(infile.read())
        infile.close()
        return contents

    def list_textfiles(directory, min_file_size):
        # Creates a list of all files stored in DIRECTORY ending on '.txt'
        textfiles = []
    for root, dirs, files in os.walk(directory):
        for name in files:
            filename = os.path.join(root, name)
            if os.stat(filename).st_size > min_file_size:
                textfiles.append(filename)
    return textfiles

def remove_punctuation1(text):
    # Removes all punctuation and conotation from the string and returns a 'plain' string
    punctuation = '®©™€â´‚³©¥ã¼•ž®è±äüöž!@#Â“§$%^*()î_+€$=¿{”}[]:«;"»\â¢|<>,.?/~`0123456789'
    for sign in punctuation:
        text = text.replace(sign, "")
    return text

def remove_punctuation2(text):
    # Removes all punctuation and conotation from the string and returns a 'plain' string
    punctuation2 = '-&'
    for sign in punctuation2:
        text = text.replace(sign, " ")
    return text

filepath_dict = "H:/MA Daske/Wordlists/IFRS.txt" # input filepath for the used wordlist (here external accounting dictionary)
directory = "H:/Converted Text/EU0_OM0_FY2001" # directory of the text files to be processed
min_file_size = 90000

l1 = remove_punctuation2(removeNonAscii(read_textfile(filepath_dict))).lower().split('\n') # externally created word/expression list

vocabulary_dict  = {k:0 for k in l1}

for filename in list_textfiles(directory, min_file_size):
    # inread each report as textfile, match tokenized text with predefined wordlist and count number of occurences of each element of that wordlist
    sample_text = remove_punctuation2(remove_punctuation1(read_textfile(filename).lower())).replace('\n', " ")
    #sample_text = remove_punctuation2(remove_punctuation1(sample_text)).replace('\n', " ")
    sample_text = ' '.join(sample_text.split())
    splitted = sample_text.split()
    c = Counter()
    c.update(splitted)
    #print(c)
    outfile = open(filename[:-4] + '_output' + '.txt', mode = 'w')
    string = str(filename)
    string_print = string[string.rfind('/')+1:string.find('-')] + ':' + string[-6:-4] + '.' + string[-8:-6] + '.' + string[-12:-8]
    samples = set(sample_text.split())
    for k in vocabulary_dict:
        spl = k.split()
        ln = len(spl)
        if ln > 1:
            check = re.findall(r'\b{0}\b'.format(k),sample_text)
            if check:
                vocabulary_dict[k] += len(check)
        elif k in samples:
            vocabulary_dict[k] += c[k]
    outfile.write(string_print + '\n')
    # line wise write each entry of the dictionary to the corresponding outputfile including comapany name, fiscal year end and tabulated frequency distribution
    for key, value in sorted( vocabulary_dict.items() ):
        outfile.write( str(key) + '\t' + str(value) + '\n' )
    outfile.close()

Use consistent naming

removeNonAscii(s) -> remove_non_ascii(n)

Use with

def read_textfile(filename):
    # Reads the entire content of FILENAME and returns a non Ascii letters cleaned string
    with open(filename) as f:
        return removeNonAscii(f.read())

Merge similar functions

def remove_punctuation1(text):
    # Removes all punctuation and conotation from the string and returns a 'plain' string
    punctuation = '®©™€â´‚³©¥ã¼•ž®è±äüöž!@#Â“§$%^*()î_+€$=¿{”}[]:«;"»\â¢|<>,.?/~`0123456789'
    for sign in punctuation:
        text = text.replace(sign, "")
    return text

    def remove_punctuation(text):
    # Removes all punctuation and conotation from the string and returns a 'plain' string
    punctuation2 = '-&'
    for sign in punctuation2:
        text = text.replace(sign, " ")
    return text

Should become

def remove_punctuation(text):
    # Removes all punctuation and conotation from the string and returns a 'plain' string
    punctuation2 = '-&'+'®©™€â´‚³©¥ã¼•ž®è±äüöž!@#Â“§$%^*()î_+€$=¿{”}[]:«;"»\â¢|<>,.?/~`0123456789'
    for sign in punctuation2:
        text = text.replace(sign, " ")
    return text

Use docstrings

def read_textfile(filename):
    """
    Reads the entire content of FILENAME and
    returns a non Ascii letters cleaned string.
    """
    with open(filename) as f:
        return removeNonAscii(f.read())

Avoid one-two letter variable names, prefer longer ones

l1 -> words_to_check


SQL query with GROUP BY to show badges and points by user and course I have the following database schema. My goal is to obtain a result set that lists the total badge points earned by each user. The badges might be earned in different courses. I want to include the courseid and the average score per course.

Users table:

| id | username |
|----|----------|
|  1 |    user1 |
|  2 |    user2 |
|  3 |    user3 |

Badges table:

| id | badgename | points | courseid |
|----|-----------|--------|----------|
|  1 |   badge a |     15 |        1 |
|  2 |   badge b |     10 |        1 |
|  3 |   badge c |     20 |        1 |
|  4 |   badge d |     15 |        2 |
|  5 |   badge e |     10 |        2 |
|  6 |   badge f |     25 |        2 |

BadgeAssignments table:

| userid | badgeid |
|--------|---------|
|      1 |       1 |
|      2 |       2 |
|      3 |       3 |
|      1 |       4 |
|      2 |       5 |
|      3 |       6 |
|      1 |       5 |
|      2 |       4 |
|      3 |       3 |

Courses table:

| courseid | coursename |
|----------|------------|
|        1 |   course 1 |
|        2 |   course 2 |

I have ended up with the following code, and I think it works fine:

SELECT userid, SUM(b.points), courseid,
   (SELECT AVG(points) FROM Badges WHERE courseid = b.courseid) as courseAVG
FROM
  Badges b INNER JOIN BadgeAssignments ba ON b.id = ba.badgeid
  INNER JOIN Users u ON ba.userid = u.id
GROUP BY userid, b.courseid

which brings me this result:

| userid | SUM(b.points) | courseid | courseAVG |
|--------|---------------|----------|-----------|
|      1 |            15 |        1 |        15 |
|      1 |            25 |        2 |   16.6667 |
|      2 |            10 |        1 |        15 |
|      2 |            25 |        2 |   16.6667 |
|      3 |            40 |        1 |        15 |
|      3 |            25 |        2 |   16.6667 |

The numbers seem to be correct. I wonder if my query makes sense? and if it needs to be revised?

Here is the SQL Fiddle.

Directly based on the current tables definition, I agree that it doesn't seem possible to have a better way of querying the needed results.

But it appears that performance might be improved using a slightly different approach, assuming that likely:

the number of courses and badges is reduced, compared to the number of users
courses and badges have relatively stable content (don't change daily!)

In the other hand we can notice that, in the current approach, courseAVG is computed again when the query is launched (more over, I'm not sure the optimizer takes care of computing it only once, rather than for each user/course pair!).

So an alternative approach might be:

add a pointsavg field to the Courses table
compute it in a separate UPDATE query (could be launched through a trigger applying AFTER INSERT and AFTER UPDATE on Badges and Courses

Here is it:

UPDATE Courses c
SET pointsavg = (
  SELECT AVG(points)
  FROM Badges b
  WHERE b.courseid = c.courseid
  GROUP BY courseid
);

Then we can simply use this pointsavg in the main query:

SELECT
  u.username,
  SUM(b.points) AS userCoursePoints,
  c.coursename,
  c.pointsavg AS courseAVG
FROM
  Users u,
  BadgeAssignments ba,
  Badges b,
  Courses c
WHERE ba.userid = u.id
  AND b.id = ba.badgeid
  AND c.courseid = b.courseid
GROUP BY u.username, b.courseid

Note that I took the opportunity to output username and coursename instead of their simple id, supposing it's closer to what is generally needed.

Here is the SQL fiddle:


Duplicate method call or add logic to set local variables as parameters? This is a fairly trivial example of a question I come up to often.  In the example below I tend to think that allowing duplication sometimes results in clearer code.

Does anyone agree/disagree on this point?

First way: an additional local variable is used so method.Invoke is called just once on the last line. I think this has slightly more logic to decode than the second way.

private static void Invoke<T>(string[] args, MethodInfo method) where T : new()
{
    T target = new T();
    target.InvokeMethod("Initialize", errorIfMethodDoesNotExist: false, accessNonPublic: true);
    string[] methodArgs = null;
    if (args.Length > 1)
    {
        methodArgs = args.Skip(1).ToArray();
    }
    method.Invoke(target, methodArgs);
}

Second way: No additional local variable but the call to method.Invoke is duplcated in both branches of the if statement.  I think this is clearer even though logic was duplicated.

private static void Invoke<T>(string[] args, MethodInfo method) where T : new()
    {
        T target = new T();
        target.InvokeMethod("Initialize", errorIfMethodDoesNotExist: false, accessNonPublic: true);
        if (args.Length > 1)
        {
            method.Invoke(target, args.Skip(1).ToArray());
        }
        else
        {
            method.Invoke(target, null);
        }
    }

I think this would be even better, in this case.

private static void Invoke<T>(string[] args, MethodInfo method) where T : new()
{
    T target = new T();
    target.InvokeMethod(
        "Initialize", errorIfMethodDoesNotExist: false, accessNonPublic: true);
    string[] methodArgs = args.Skip(1).ToArray();
    method.Invoke(target, methodArgs);
}

Skip() on an empty collection returns an empty collection.


O.O. Style Palindrome Checker I am beginning to learn about object oriented programming in my programming class, and to make sure I had all the ideas down, I made a small palindrome checker program. It works by checking if the letter on the opposite side is the same.

from time import sleep  # To add readability delays

class PalindromeFinder:

    # Makes the variables
    def __init__(self):
        self.letter_count = {}
        self.loop = 0

    def check_for_palindrome(self, base):
        self.loop = 0
        for letter in base:
            # Checks if the letter in x position is the same as the letter in the -x position
            if base[self.loop] != base[-(self.loop + 1)]:
                return (f"{base} is not a palindrome")
            self.loop += 1
        return (f"{base} is a palindrome.")

def introduce_palindromes():
    print("Palindromes are really cool!")
    sleep(2)
    print("It when a word or phrase is the same backwards and forwards!")
    sleep(2)
    print("For example", palindrome_checker.check_for_palindrome("racecar"))
    sleep(2)
    print("While", palindrome_checker.check_for_palindrome("alarm"))
    sleep(2)
    print("Now you enter some palindromes!")

palindrome_checker = PalindromeFinder()
introduce_palindromes()
while True:
    sleep(1)
    print(palindrome_checker.check_for_palindrome(input("Enter your palindrome: ").lower()))

Since I have no idea what I am doing, I just generally want to know if what I am doing is not violating any conventions. More specifically, I want to know if having both functions and classes are ok, as well as if putting the variables in __init__ like I did is fine. Also, when initializing several instances at once, is there any documentation on were you do it? Do you do it between the class and functions or between the functions and the code?

Any other advice is welcome.

As stated in the comments, this is not a good example for OO programming, because it can be solved with just a function. Sometimes you can still use classes to separate different functionality, but in Python this is not really required nor generally practiced (compared to e.g. Java where all code must be in a class - if we discount default methods in interfaces, static code blocks and such).

Now you've put designated two variables as fields, while they should be local to the function. There is no need at all to use them as fields. This won't directly show up as a problem, but if you'd e.g. call the method check_for_palindrome in a concurrent fashion, you'd be in trouble, because the loop field will now be used by two different threads at once. letter_count on the other hand is secure, but only because it is simply not used.

Only create fields for variable values that are intrinsic to the class (or are references that tie the class to another of which it is dependent). For instance, you might want to configure the checker to check for upper or lowercase, or maybe even mixed case. That kind of field would be logical for a palindrome checker class. All in all, you should try and minimize the variables that make up the state and thus number & complexity of the fields for a particular class.

As you've already found out yourself, PalindromeChecker would be a better name for the class, as it doesn't search for palindromes, at least not at the moment. If you've got a method for a class, you don't need to repeat everything, just PalindromeChecker.check would be sufficient. I don't know why the input is called base, just input seems sufficient (edit: word would probably even be a better name).

When it comes to functionality of a class, it is important to distinguish human interaction with actual output. I'd never create a checker that would return a string as response. In this case I'd expect a bool output instead. Then the program that uses the checker can create a user response if the result is True or False. At least didn't print out the result, which would have been the action of most starting developers.

Finally, in the call to the class instance, you're trying to do too much at once:

print(palindrome_checker.check_for_palindrome(input("Enter your palindrome: ").lower()))

First assign the result to a variable and then print it out. Separate the input. You'll thank yourself if you have to startup a debugger and step through your program.


Encrypting messages to be sent over a socket I recognize the importance of properly implement cryptographic functions. Since this is my first time using them I really wanted a second opinion on my method. Therefore, I was wondering if my method of sign-then-encrypt as implemented would be a valid method of safely achieving those results.

def gen_keys(size):
    print('Generating new keys...')
    pubkey, privkey = rsa.newkeys(size, poolsize=8)
    print('Keys generated!')
    return pubkey, privkey

def encrypt_full_message(message, pubkey, privkey):
    parts = textwrap.wrap(message, 245)
    message_to_send = b''
    for part in parts:
        part = part.encode('utf-8')
        signature = rsa.sign(part, privkey, 'SHA-256')
        encrypted_message = rsa.encrypt(part, pubkey)
        message_to_send += encrypted_message
        message_to_send += signature
    return message_to_send

def decrypt_full_message(message, privkey, pubkey):
    parts = [message[i:i+256] for i in range(0, len(message), 256)]
    messages = parts[0::2]
    signatures = parts[1::2]
    fmessage = b''
    for signature, message in zip(signatures, messages):
        message = rsa.decrypt(message, privkey)
        if rsa.verify(message, signature, pubkey):
            fmessage += message
        else:
            print('Message has been tampered with.')
    return fmessage

gen_keys()

It's generally a bad idea to have both calculation logic and output in the same function. What if you have to move your system to somewhere you can't use print() such as a batch process or web server? Use the logging module instead, that way it can easily be redirected.
encrypt_full_message()

The problem with this one is that you are constructing many temporary string objects. Python strings don't support the append operation natively; what you're doing is throwing away the old string and constructing a new one. A better way would be to write an encrypt_and_sign_single_part() function and then this whole function would just be

def encrypt_full_message(message, _pubkey, privkey):
    return ''.join([encrypt_and_sign_single_part(part,_pubkey,privkey) for part in textwrap.wrap(message, max)])

Also why did you put the underscore before pubkey? That doesn't match the naming conventions you're using.

Similar warning about using print(). Really, check out the logging module.
decrypt_full_message()

AAAAAAAAGH! Did you even test this with a multi-part message? You're going to get n^2-n warnings for every n part-pairs!

The biggest problem is you separate the parts in two and then use the two streams separately. Python has tuples, use them. That way you only can use one loop, rather than the incorrect nested loops you have now.