Creating objects and putting them into std::list I am writing small roguelike game where I spawn some items and put them into the std::list. Is this the right way to do it?

The Item class is as followed (I know it has public id property):

class Item {

public:
   Item();
   ~Item();

   unsigned int id;
};

Somewhere in the code I also defined:

list<Item*> listOfItems;
list<Item*>::iterator it;

Now I am using this code to create an item:

Item *p = new Item();
p->id = itemCrowbar;
listOfItems.push_back(p);

At the end of the program I use small loop to delete all items:

for (it = listOfItems.begin(); it!=listOfItems.end(); it++) {
    delete *it;
}

Is this approach for creating objects, putting them in std::list and
at the end deleting them correct, or am I missing something important here?

If Item has subclasses, then make the ~Item() destructor virtual.

If Item has a copy constructor, and is not a superclass of a subclass, then you can store a copy of the item (not pointer to the item) in the list:

list<Item> listOfItems;

Item item;
item.id = itemCrowbar;
listOfItems.push_back(item);

In that case, you won't need to delete Item pointers before the list is destroyed.

However, if Item has subclasses, then this might be legal but a bug (called 'object slicing'):

list<Item> listOfItems;

SubclassOfItem item;
item.id = itemCrowbar;
item.extra = "hello";
listOfItems.push_back(item);

Or you can create a list of 'smart pointers' (std::unique_ptr) to items: which behave like pointers to Items except that you don't need to delete the Item (the Item is deleted when the smart pointer which contains it is destroyed).


Performance on countifs and sumifs I'm trying to calculate with SUMIFS and COUNTIFS some data.

The output sheet looks like this:

I've hidden some irrelevant or sensitive data to make it easy to go to the point.

My code runs an array on this worksheet and calculates per each day and user some KPIs I need from this worksheet:

The calculation is pretty simple, but it impacts heavily on the performance:

Option Explicit
Sub Test()

    With Hoja4
        Dim arrTotal As Variant: arrTotal = .UsedRange.Value
        Dim Fecha As Double
        Dim i As Long
        For i = 2 To UBound(arrTotal)
            Fecha = arrTotal(i, 1)
            arrTotal(i, 13) = CalculaLlamadas(CStr(arrTotal(i, 2)), Fecha, 1, 4)
            arrTotal(i, 14) = CalculaFCR(CStr(arrTotal(i, 2)), Fecha, 1, 4)
            arrTotal(i, 15) = CalculaLlamadas(CStr(arrTotal(i, 2)), Fecha, 3, 5)
            arrTotal(i, 16) = CalculaFCR(CStr(arrTotal(i, 2)), Fecha, 3, 5)
            arrTotal(i, 17) = CalculaLlamadas(CStr(arrTotal(i, 2)), Fecha, 30, 6)
            arrTotal(i, 18) = CalculaFCR(CStr(arrTotal(i, 2)), Fecha, 30, 6)
        Next i
        .UsedRange.Value = arrTotal
    End With

End Sub
Private Function CalculaFCR(Agente As String, Fecha As Double, Dias As Byte, Columna As Byte) As Long

    Dim FechaFin As Double: FechaFin = Fecha + Dias
    With Hoja37
        CalculaFCR = Application.SumIfs(.Columns(Columna), .Columns(1), Agente, _
            .Columns(2), ">=" & Fecha, .Columns(2), "<=" & FechaFin)
    End With

End Function
Private Function CalculaLlamadas(Agente As String, Fecha As Double, Dias As Byte, Columna As Byte) As Long

    Dim FechaFin As Double: FechaFin = Fecha + Dias
    With Hoja37
        CalculaLlamadas = Application.CountIfs(.Range("A:A"), Agente, _
            .Range("B:B"), ">=" & Fecha, .Columns(2), "<=" & FechaFin)
    End With
    CalculaLlamadas = LLamadas

End Function

As you can see on the code, the logic is pretty easy calculate how many times an user shows between 2 dates, and get the sum of a column for these dates.

I can't figure out how to make this quicker, I've also tried using an array to loop through it instead using the countif and the sumif, but it takes even longer because the input sheet has 40k (1 and half month of data) rows and its likely to be increased with days to come.

Any suggestions on how to improve the performance for this task?

You are generally correct in using Application functions (SumIfs and CountIfs) over rolling your own functions, as the built-in functions in Excel should be optimized in the DLL than anything you could write in VBA. The difference in performance is due to the organization of the data.

The key to my example below is to pre-process the input data into a form that reduces the "lookup" time of all the data. For comparison sake, using SumIfs and CountIfs will always be working with your entire data range. Your data set may be large (40k+ rows), so these Excel functions will always take a fixed amount of time. I timed my example to process 45,500 rows in 44.8 seconds. (I'll share a GenerateData sub that I created to set up the inputs for you or anyone else to double-check my work.)

The simple reccommendation for processing large amounts of data is to use a memory-based array. You're already doing this with your arrTotal array. There are two main issues in performance with this: 1) the Excel functions are still accessing the Worksheet object to process SumIfs and CountIfs, and 2) there is no optimization in the arrTotal data to help speed things up. This is where my own example starts.

First, I want to create an array from the input data that is "ordered". In this case, it means a memory-based array with the data sorted first on Agente and then on Fecha Completa. Please note that my code calculates the used area of the spreadsheet in a manner that should always be accurate (see @MathieuGuindon's comment). This function returns an ordered array:

Private Function OrderedInputs() As Variant
    '--- first sort the input data by Agente, then by date
    With Hoja37
        Dim inputArea As Range
        Dim lastRow As Long
        lastRow = .Cells(.Rows.count, 1).End(xlUp).Row
        Set inputArea = .Range("A2").Resize(lastRow - 1, 6)
        With .Sort
            .SortFields.Clear
            .SortFields.Add Key:=inputArea.Columns(1), SortOn:=xlSortOnValues, _
                            Order:=xlAscending, DataOption:=xlSortNormal
            .SortFields.Add Key:=inputArea.Columns(2), SortOn:=xlSortOnValues, _
                            Order:=xlAscending, DataOption:=xlSortNormal
            .SetRange inputArea
            .Header = xlNo
            .MatchCase = False
            .Orientation = xlTopToBottom
            .SortMethod = xlPinYin
            .Apply

            '--- now grab the data into a memory-based array
            Dim inputData As Variant
            inputData = inputArea.Value

            '--- assuming the original data was sorted by date, then by Agente
            '    we'll put it back
            .SortFields.Clear
            .SortFields.Add Key:=inputArea.Columns(2), SortOn:=xlSortOnValues, _
                            Order:=xlAscending, DataOption:=xlSortNormal
            .SortFields.Add Key:=inputArea.Columns(1), SortOn:=xlSortOnValues, _
                            Order:=xlAscending, DataOption:=xlSortNormal
            .SetRange inputArea
            .Header = xlNo
            .MatchCase = False
            .Orientation = xlTopToBottom
            .SortMethod = xlPinYin
            .Apply
        End With
    End With
    OrderedInputs = inputData
End Function

Having a sorted, ordered set of data is nice, but it's not nearly optimized enough yet. When you're processing this much data, you want to be able to immediately access any given agent's data. The way to do this is a Dictionary, using the agent's name as the key. Associated with each agent, is the date plus FCR1, FCR3, and FCR30 data values -- and there are multiple sets of this data for each agent. So for each agent, we can build a Collection of this data. The best way to do this is to create a simple object class called AgentData.

'--- Class: AgentData
Option Explicit

Public day As Date
Public fcr1 As Long
Public fcr3 As Long
Public fcr30 As Long

All the fields are public because you really don't need to manipulate the data going in or coming out. Building up a Collection of AgentData objects starts with looping through all the (ordered) input data by name, and creating a Collection entry for each date:

Private Function BuildAgentData(ByRef inputArray As Variant) As Dictionary
    '--- we're creating a Dictionary of Agentes for quick lookup
    '    each Item in the Dictionary is an ordered Collection of AgentData
    '    objects, i.e. the objects are in date ascending order
    Dim agentes As Dictionary
    Set agentes = New Dictionary

    Dim i As Long
    Dim thisAgente As String
    Dim aData As AgentData
    Dim allAgentData As Collection
    For i = LBound(inputArray, 1) To UBound(inputArray, 1)
        If thisAgente <> inputArray(i, 1) Then
            thisAgente = inputArray(i, 1)
            '--- create the first entry
            Set aData = New AgentData
            aData.day = inputArray(i, 2)
            aData.fcr1 = inputArray(i, 4)
            aData.fcr3 = inputArray(i, 5)
            aData.fcr30 = inputArray(i, 6)
            Set allAgentData = New Collection
            allAgentData.Add aData
            agentes.Add thisAgente, allAgentData
        Else
            '--- increase the size of the array by one and add the date
            Set allAgentData = agentes(thisAgente)
            Set aData = New AgentData
            aData.day = inputArray(i, 2)
            aData.fcr1 = inputArray(i, 4)
            aData.fcr3 = inputArray(i, 5)
            aData.fcr30 = inputArray(i, 6)
            allAgentData.Add aData
        End If
    Next i
    Set BuildAgentData = agentes
End Function

This reduces the beginning of your test sub to:

Dim sortedInputData As Variant
Dim agentes As Dictionary
sortedInputData = OrderedInputs()
Set agentes = BuildAgentData(sortedInputData)
Debug.Print "There are " & UBound(sortedInputData, 1) & " rows of input data."

There are changes to your Calcula functions, but the main test sub changes very little:

With Hoja4
    Dim arrTotal As Variant: arrTotal = .UsedRange.Value
    Dim fecha As Double
    Dim i As Long
    For i = 2 To UBound(arrTotal)
        fecha = arrTotal(i, 1)
        arrTotal(i, 13) = CalculaLlamadas2(sortedInputData, agentes, CStr(arrTotal(i, 2)), fecha, 1, 4)
        arrTotal(i, 14) = CalculaFCR2(sortedInputData, agentes, CStr(arrTotal(i, 2)), fecha, 1, 4)
        arrTotal(i, 15) = CalculaLlamadas2(sortedInputData, agentes, CStr(arrTotal(i, 2)), fecha, 3, 5)
        arrTotal(i, 16) = CalculaFCR2(sortedInputData, agentes, CStr(arrTotal(i, 2)), fecha, 3, 5)
        arrTotal(i, 17) = CalculaLlamadas2(sortedInputData, agentes, CStr(arrTotal(i, 2)), fecha, 30, 6)
        arrTotal(i, 18) = CalculaFCR2(sortedInputData, agentes, CStr(arrTotal(i, 2)), fecha, 30, 6)
    Next i
    .UsedRange.Value = arrTotal
End With

Each of the Calcula functions will now use both the sortedInputData and the agentes dictionary as inputs. Instead of being forced to loop through all 40,000+ rows to count and sum for every single agent, the function immediately accesses the collection of data for each agent (via the Dictionary), and then can loop through the Collection (which may have as many entries as needed, but still is far, far less than the original input set).

Private Function CalculaFCR2(ByRef inputData As Variant, ByRef agentDates As Dictionary, _
                             ByVal agente As String, ByVal fecha As Date, _
                             ByVal dias As Long, ByVal columna As Long) As Long
    Dim fechaFin As Date
    fechaFin = fecha + dias
    Dim sum As Long
    If agentDates.Exists(agente) Then
        Dim aData As Variant
        Set aData = agentDates(agente)
        Dim item As Variant
        For Each item In aData
            If (item.day >= fecha) And (item.day <= fechaFin) Then
                Select Case columna
                    Case 4
                        sum = sum + item.fcr1
                    Case 5
                        sum = sum + item.fcr3
                    Case 6
                        sum = sum + item.fcr30
                End Select
            ElseIf (item.day > fechaFin) Then
                Exit For
            End If
        Next item
        CalculaFCR2 = sum
    Else
        '--- error processing if the agente doesn't exist
    End If
End Function

Private Function CalculaLlamadas2(ByRef inputData As Variant, ByRef agentDates As Dictionary, _
                                  ByVal agente As String, ByVal fecha As Date, _
                                  ByVal dias As Long, ByVal columna As Long) As Long
    Dim fechaFin As Date
    fechaFin = fecha + dias
    Dim count As Long
    If agentDates.Exists(agente) Then
        Dim aData As Variant
        Set aData = agentDates(agente)
        Dim item As Variant
        For Each item In aData
            If (item.day >= fecha) And (item.day <= fechaFin) Then
                count = count + 1
            ElseIf (item.day > fechaFin) Then
                Exit For
            End If
        Next item
        CalculaLlamadas2 = count
    Else
        '--- error processing if the agente doesn't exist
    End If
End Function

I added a software timer class to measure execution time on my laptop (Intel Core i5-6300, 2.40GHz) and received the following results:

There are 4550 rows of input data.
Time to build ordered inputs: 0.078 seconds
Total Run Time: 4.407 seconds

There are 45500 rows of input data.
Time to build ordered inputs: 0.484 seconds
Total Run Time: 44.797 seconds

For reference, here are the three code modules you need to add:

Class: AgentData

'--- Class: AgentData
Option Explicit

Public day As Date
Public fcr1 As Long
Public fcr3 As Long
Public fcr30 As Long

Class: StopWatch

Option Explicit
'--- from https://stackoverflow.com/a/939260/4717755

Private mlngStart As Long
Private Declare Function GetTickCount Lib "kernel32" () As Long

Public Sub StartTimer()
    mlngStart = GetTickCount
End Sub

Public Function EndTimer() As Long
    EndTimer = (GetTickCount - mlngStart)
End Function

Code Module: Module1

Option Explicit

Sub Test()
    Dim sw As StopWatch
    Set sw = New StopWatch
    sw.StartTimer

    Dim sortedInputData As Variant
    Dim agentes As Dictionary
    sortedInputData = OrderedInputs()
    Set agentes = BuildAgentData(sortedInputData)
    Debug.Print "There are " & UBound(sortedInputData, 1) & " rows of input data."
    Debug.Print "Time to build ordered inputs: " & CDbl((sw.EndTimer / 1000#)) & " seconds"

    With Hoja4
        Dim arrTotal As Variant: arrTotal = .UsedRange.Value
        Dim fecha As Double
        Dim i As Long
        For i = 2 To UBound(arrTotal)
            fecha = arrTotal(i, 1)
            arrTotal(i, 13) = CalculaLlamadas2(sortedInputData, agentes, CStr(arrTotal(i, 2)), fecha, 1, 4)
            arrTotal(i, 14) = CalculaFCR2(sortedInputData, agentes, CStr(arrTotal(i, 2)), fecha, 1, 4)
            arrTotal(i, 15) = CalculaLlamadas2(sortedInputData, agentes, CStr(arrTotal(i, 2)), fecha, 3, 5)
            arrTotal(i, 16) = CalculaFCR2(sortedInputData, agentes, CStr(arrTotal(i, 2)), fecha, 3, 5)
            arrTotal(i, 17) = CalculaLlamadas2(sortedInputData, agentes, CStr(arrTotal(i, 2)), fecha, 30, 6)
            arrTotal(i, 18) = CalculaFCR2(sortedInputData, agentes, CStr(arrTotal(i, 2)), fecha, 30, 6)
        Next i
        .UsedRange.Value = arrTotal
    End With
    Debug.Print "Total Run Time: " & CDbl((sw.EndTimer / 1000#)) & " seconds"
End Sub

Private Function CalculaFCR2(ByRef inputData As Variant, ByRef agentDates As Dictionary, _
                             ByVal agente As String, ByVal fecha As Date, _
                             ByVal dias As Long, ByVal columna As Long) As Long
    Dim fechaFin As Date
    fechaFin = fecha + dias
    Dim sum As Long
    If agentDates.Exists(agente) Then
        Dim aData As Variant
        Set aData = agentDates(agente)
        Dim item As Variant
        For Each item In aData
            If (item.day >= fecha) And (item.day <= fechaFin) Then
                Select Case columna
                    Case 4
                        sum = sum + item.fcr1
                    Case 5
                        sum = sum + item.fcr3
                    Case 6
                        sum = sum + item.fcr30
                End Select
            ElseIf (item.day > fechaFin) Then
                Exit For
            End If
        Next item
        CalculaFCR2 = sum
    Else
        '--- error processing if the agente doesn't exist
    End If
End Function

Private Function CalculaLlamadas2(ByRef inputData As Variant, ByRef agentDates As Dictionary, _
                                  ByVal agente As String, ByVal fecha As Date, _
                                  ByVal dias As Long, ByVal columna As Long) As Long
    Dim fechaFin As Date
    fechaFin = fecha + dias
    Dim count As Long
    If agentDates.Exists(agente) Then
        Dim aData As Variant
        Set aData = agentDates(agente)
        Dim item As Variant
        For Each item In aData
            If (item.day >= fecha) And (item.day <= fechaFin) Then
                count = count + 1
            ElseIf (item.day > fechaFin) Then
                Exit For
            End If
        Next item
        CalculaLlamadas2 = count
    Else
        '--- error processing if the agente doesn't exist
    End If
End Function

Private Function OrderedInputs() As Variant
    '--- first sort the input data by Agente, then by date
    With Hoja37
        Dim inputArea As Range
        Dim lastRow As Long
        lastRow = .Cells(.Rows.count, 1).End(xlUp).Row
        Set inputArea = .Range("A2").Resize(lastRow - 1, 6)
        With .Sort
            .SortFields.Clear
            .SortFields.Add Key:=inputArea.Columns(1), SortOn:=xlSortOnValues, _
                            Order:=xlAscending, DataOption:=xlSortNormal
            .SortFields.Add Key:=inputArea.Columns(2), SortOn:=xlSortOnValues, _
                            Order:=xlAscending, DataOption:=xlSortNormal
            .SetRange inputArea
            .Header = xlNo
            .MatchCase = False
            .Orientation = xlTopToBottom
            .SortMethod = xlPinYin
            .Apply

            '--- now grab the data into a memory-based array
            Dim inputData As Variant
            inputData = inputArea.Value

            '--- assuming the original data was sorted by date, then by Agente
            '    we'll put it back
            .SortFields.Clear
            .SortFields.Add Key:=inputArea.Columns(2), SortOn:=xlSortOnValues, _
                            Order:=xlAscending, DataOption:=xlSortNormal
            .SortFields.Add Key:=inputArea.Columns(1), SortOn:=xlSortOnValues, _
                            Order:=xlAscending, DataOption:=xlSortNormal
            .SetRange inputArea
            .Header = xlNo
            .MatchCase = False
            .Orientation = xlTopToBottom
            .SortMethod = xlPinYin
            .Apply
        End With
    End With
    OrderedInputs = inputData
End Function

Private Function BuildAgentData(ByRef inputArray As Variant) As Dictionary
    '--- we're creating a Dictionary of Agentes for quick lookup
    '    each Item in the Dictionary is an ordered Collection of AgentData
    '    objects, i.e. the objects are in date ascending order
    Dim agentes As Dictionary
    Set agentes = New Dictionary

    Dim i As Long
    Dim thisAgente As String
    Dim aData As AgentData
    Dim allAgentData As Collection
    For i = LBound(inputArray, 1) To UBound(inputArray, 1)
        If thisAgente <> inputArray(i, 1) Then
            thisAgente = inputArray(i, 1)
            '--- create the first entry
            Set aData = New AgentData
            aData.day = inputArray(i, 2)
            aData.fcr1 = inputArray(i, 4)
            aData.fcr3 = inputArray(i, 5)
            aData.fcr30 = inputArray(i, 6)
            Set allAgentData = New Collection
            allAgentData.Add aData
            agentes.Add thisAgente, allAgentData
        Else
            '--- increase the size of the array by one and add the date
            Set allAgentData = agentes(thisAgente)
            Set aData = New AgentData
            aData.day = inputArray(i, 2)
            aData.fcr1 = inputArray(i, 4)
            aData.fcr3 = inputArray(i, 5)
            aData.fcr30 = inputArray(i, 6)
            allAgentData.Add aData
        End If
    Next i
    Set BuildAgentData = agentes
End Function

And at the bottom of Module1 is the bonus sub to create the input data:

Sub GenerateData()
    Dim names() As String
    '--- from http://listofrandomnames.com/
    names = Split("Lori,Norah,Valorie,Evita,Alden,Tressa,Carrol," & _
                  "Leeanna,Alexia,Marlyn,Wilhemina,Lanell,Kari," & _
                  "Rose,Arianne,Lesa,Royal,Maura,Sherie,Lahoma", ",")
    Const START As Date = #1/1/2020#
    Const FINISH As Date = #3/31/2020#
    Const LOGINS_PER_DAY As Long = 500

    '--- output setup
    Dim entry As Range
    Dim day As Date
    Dim i As Long
    Dim index As Long
    With Hoja4
        .Cells.Clear
        .Range("A1").Value = "Fecha"
        .Range("B1").Value = "Login"
        .Range("F1").Value = "Centro"
        .Range("F1").Value = "Centro"
        .Range("M1").Value = "Encuestas 1"
        .Range("N1").Value = "FCR 1"
        .Range("O1").Value = "Encuestas 3"
        .Range("P1").Value = "FCR 3"
        .Range("Q1").Value = "Encuestas 30"
        .Range("R1").Value = "FCR 30"
        Set entry = .Range("A2")
        For day = START To FINISH
            For i = LBound(names) To UBound(names)
                entry.Cells(1, 1).Value = day
                entry.Cells(1, 2).Value = names(i)
                Set entry = entry.Offset(1, 0)
            Next i
        Next day
    End With

    '--- input setup
    With Hoja37
        .Cells.Clear
        .Range("A1").Value = "Agente"
        .Range("B1").Value = "Fecha Completa"
        .Range("D1").Value = "FCR 1"
        .Range("E1").Value = "FCR 3"
        .Range("F1").Value = "FCR 30"
        Set entry = .Range("A2")
        For day = START To FINISH
            For i = 1 To LOGINS_PER_DAY
                index = CLng((UBound(names) - LBound(names)) * Rnd() + LBound(names))
                entry.Cells(1, 1).Value = names(index)
                entry.Cells(1, 2).Value = day
                entry.Cells(1, 4).Value = IIf(Rnd() > 0.5, 1, 0)
                entry.Cells(1, 5).Value = IIf(Rnd() > 0.5, 1, 0)
                entry.Cells(1, 6).Value = IIf(Rnd() > 0.5, 1, 0)
                Set entry = entry.Offset(1, 0)
            Next i
        Next day
    End With
End Sub


Android Custom WebView I have made a custom webview which is used to open a very heavy site in terms of database. The webview uses custom code for uploading files using an upload handler class. I need suggestions on how I can optimize my code for better performance.

Features necessary (current custom webview written below does it all):

The basic functionality is just to load a responsive WordPress website.
Webview should support image uploading features.
mailto: links should open in the default email client.

I am adding the complete main activity page for your reference.

package com.Example.application;

import java.io.File;
import java.lang.reflect.Method;
import java.net.URL;

import android.annotation.SuppressLint;

public class MainActivity extends Activity  implements OnClickListener {

      final Context context = this;
    private WebView webView;
     private ImageButton btnrefresh;
     private TextView txtrefresh;
     private Menu optionsMenu;
     private WebChromeClient.CustomViewCallback customViewCallback;
        private View mCustomView;

    @SuppressWarnings("deprecation")
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        //define button
        btnrefresh = (ImageButton) findViewById(R.id.imageButton1);

        btnrefresh.setOnClickListener(this);
        btnrefresh.setVisibility(View.GONE);

        //define textView
        txtrefresh = (TextView)findViewById((R.id.textView1));
        txtrefresh.setVisibility(View.GONE);

        if(isConnected())
        {

            webView = (WebView) findViewById(R.id.webView1);
            webView.getSettings().setJavaScriptEnabled(true);
               webView.getSettings().setAppCacheEnabled(true);

                webView.getSettings().setRenderPriority(RenderPriority.HIGH);
                webView.getSettings().setCacheMode(WebSettings.LOAD_NO_CACHE);
                webView.getSettings().setSaveFormData(true);
        //  webView.getSettings().setPluginState(PluginState.ON);
          webView.setWebViewClient(new WebViewClient()
          {
          @Override
          public boolean shouldOverrideUrlLoading(WebView view, String url) {

              if (url.startsWith("mailto:")) {
                  sendEmail(url.substring(7));
                  return true;
              }

              return false;
          }

        });

            initWebView(webView);
            webView.loadUrl("http://Example.com/");

            }

        else
        {

Your code formatting needs a lot of work, I couldn't even read through the code because I kept getting lost.

Be consistent with your bracing style

pick one and stick to it

Get rid of unnecessary whitespace

too many extra lines

Don't comment out code and leave it there

if you aren't using it, delete it

Then you end up with something like this this

package com.Example.application;

import java.io.File;
import java.lang.reflect.Method;
import java.net.URL;
import android.annotation.SuppressLint;
import android.annotation.TargetApi;
import android.app.Activity;
import android.app.AlertDialog;
import android.content.ActivityNotFoundException;
import android.content.Context;
import android.content.DialogInterface;
import android.content.Intent;
import android.content.res.Configuration;
import android.graphics.Bitmap;
import android.net.ConnectivityManager;
import android.net.MailTo;
import android.net.NetworkInfo;
import android.net.Uri;
import android.net.wifi.WifiManager;
import android.os.Build;
import android.os.Bundle;
import android.os.Environment;
import android.provider.MediaStore;
import android.view.KeyEvent;
import android.view.Menu;
import android.view.MenuInflater;
import android.view.MenuItem;
import android.view.View;
import android.view.View.OnClickListener;
import android.webkit.JsResult;
import android.webkit.ValueCallback;
import android.webkit.WebChromeClient;
import android.webkit.WebSettings;
import android.webkit.WebSettings.PluginState;
import android.webkit.WebSettings.RenderPriority;
import android.webkit.WebView;
import android.webkit.WebViewClient;
import android.widget.ImageButton;
import android.widget.RelativeLayout;
import android.widget.TextView;
import android.widget.Toast;

public class MainActivity extends Activity  implements OnClickListener {

    final Context context = this;
    private WebView webView;
    private ImageButton btnrefresh;
    private TextView txtrefresh;
    private Menu optionsMenu;
    private WebChromeClient.CustomViewCallback customViewCallback;
    private View mCustomView;

    @SuppressWarnings("deprecation")
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        //define button
        btnrefresh = (ImageButton) findViewById(R.id.imageButton1);

        btnrefresh.setOnClickListener(this);
        btnrefresh.setVisibility(View.GONE);

        //define textView
        txtrefresh = (TextView)findViewById((R.id.textView1));
        txtrefresh.setVisibility(View.GONE);

        if(isConnected())
        {

            webView = (WebView) findViewById(R.id.webView1);
            webView.getSettings().setJavaScriptEnabled(true);
            webView.getSettings().setAppCacheEnabled(true);

            webView.getSettings().setRenderPriority(RenderPriority.HIGH);
            webView.getSettings().setCacheMode(WebSettings.LOAD_NO_CACHE);
            webView.getSettings().setSaveFormData(true);
            webView.setWebViewClient(new WebViewClient()
            {
                @Override
                public boolean shouldOverrideUrlLoading(WebView view, String url) {
                    if (url.startsWith("mailto:")) {
                        sendEmail(url.substring(7));
                        return true;
                    }
                    return false;
                }
            });

            initWebView(webView);
            webView.loadUrl("http://Example.com/");
        }
        else
        {
            RelativeLayout rel = (RelativeLayout)findViewById(R.id.relativelayout1);
            rel.setOnClickListener(new View.OnClickListener(){
                @Override
                public void onClick(View v){
                    refresh();
                }
            });
            btnrefresh.setVisibility(View.VISIBLE);
            txtrefresh.setVisibility(View.VISIBLE);
            Toast.makeText(getBaseContext(), "No Internet Connection !!", Toast.LENGTH_SHORT).show();
        }
    }

    private void sendEmail(String add) {
        Intent i = new Intent(Intent.ACTION_SEND);
        i.setType("text/plain");
        i.putExtra(Intent.EXTRA_EMAIL, new String[] { add });
        try {
            startActivity(Intent.createChooser(i, "Send mail..."));
        } catch (android.content.ActivityNotFoundException ex) {
            Toast.makeText(MainActivity.this,
                "There are no email clients installed.", Toast.LENGTH_SHORT)
                .show();
        }

    }

    public boolean onCreateOptionsMenu(Menu menu) {
        this.optionsMenu = menu;
        MenuInflater inflater = getMenuInflater();
        inflater.inflate(R.menu.main_activity_actions, menu);
        return super.onCreateOptionsMenu(menu);
    }

    public boolean onOptionsItemSelected(MenuItem item) {
        switch (item.getItemId()) {
            case R.id.airport_menuRefresh:
                webView.reload();
                Toast.makeText(getBaseContext(), "Refeshing...", Toast.LENGTH_SHORT).show();
                // Complete with your code
                return true;
            case R.id.goBack:
                if(webView.canGoBack()) {
                    webView.goBack();
                }
                return true;
            case R.id.goForward:
                if(webView.canGoForward()) {
                    webView.goForward();
                }
                return true;
            default:
                return super.onOptionsItemSelected(item);
        }
    }

    public void refresh()
    {
        if (isConnected()) {
            btnrefresh.setVisibility(View.GONE);
            txtrefresh.setVisibility(View.GONE);
            webView = (WebView) findViewById(R.id.webView1);

            webView.setWebViewClient(new WebViewClient()
            {
                @Override
                public boolean shouldOverrideUrlLoading(WebView view, String url) {
                    if (url.startsWith("mailto:")) {
                        sendEmail(url.substring(7));
                        return true;
                    }
                    return false;
                }

            });
            initWebView(webView);
            webView.loadUrl("http://Example.com/");

            Toast.makeText(getBaseContext(), "Connected !!", Toast.LENGTH_LONG).show();
            Toast.makeText(getBaseContext(), "Loading....", Toast.LENGTH_SHORT).show();
            Toast.makeText(getBaseContext(), "Please wait....", Toast.LENGTH_SHORT).show();
        }
        else
        {
            Toast.makeText(getBaseContext(), "No Internet Connection !!", Toast.LENGTH_SHORT).show();
        }
    }

    public boolean isConnected(){
        ConnectivityManager connMgr = (ConnectivityManager) getSystemService(Activity.CONNECTIVITY_SERVICE);
        NetworkInfo networkInfo = connMgr.getActiveNetworkInfo();
        if (networkInfo != null && networkInfo.isConnected())
        {
            return true;
        }
        else
            return false;
        }
        @Override
        public boolean onKeyDown(int keyCode, KeyEvent event) {
            if ((keyCode == KeyEvent.KEYCODE_BACK)) {
                AlertDialog.Builder alertbox = new AlertDialog.Builder(MainActivity.this);

                alertbox.setTitle("Do You Want To Exit ?");
                alertbox.setPositiveButton("Yes", new DialogInterface.OnClickListener() {
                    public void onClick(DialogInterface arg0, int arg1) {
                        // finish used for destroyed activity
                        exit();
                    }
                });

                alertbox.setNegativeButton("No", new DialogInterface.OnClickListener() {
                    public void onClick(DialogInterface arg0, int arg1) {
                        // Nothing will be happened when clicked on no button
                        // of Dialog
                    }
                });

                alertbox.show();
            }
            return super.onKeyDown(keyCode, event);
        }
        public void exit()
        {
            Intent intent = new Intent(Intent.ACTION_MAIN);
            intent.addCategory(Intent.CATEGORY_HOME);
            intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
            startActivity(intent);
        }
        public void onClick(View v)
        {
            //refresh ImageButton function
            if (v == btnrefresh)
            {
                refresh();
            }
        }

        private final static Object methodInvoke(Object obj, String method, Class<?>[] parameterTypes, Object[] args) {
            try {
                Method m = obj.getClass().getMethod(method, new Class[] { boolean.class });
                m.invoke(obj, args);
            } catch (Exception e) {
                e.printStackTrace();
            }
            return null;
        }

        private void initWebView(WebView webView) {
            WebSettings settings = webView.getSettings();

            settings.setJavaScriptEnabled(true);
            settings.setAllowFileAccess(true);
            settings.setDomStorageEnabled(true);
            settings.setCacheMode(WebSettings.LOAD_NO_CACHE);
            settings.setLoadWithOverviewMode(true);
            settings.setUseWideViewPort(true);
            settings.setRenderPriority(RenderPriority.HIGH);

            methodInvoke(settings, "setPluginsEnabled", new Class[] { boolean.class }, new Object[] { true });
            methodInvoke(settings, "setPluginState", new Class[] { PluginState.class }, new Object[] { PluginState.ON });
            methodInvoke(settings, "setPluginsEnabled", new Class[] { boolean.class }, new Object[] { true });
            methodInvoke(settings, "setAllowUniversalAccessFromFileURLs", new Class[] { boolean.class }, new Object[] { true });
            methodInvoke(settings, "setAllowFileAccessFromFileURLs", new Class[] { boolean.class }, new Object[] { true });

            webView.setScrollBarStyle(View.SCROLLBARS_INSIDE_OVERLAY);
            webView.clearHistory();
            webView.clearFormData();
            webView.clearCache(true);
            webView.setWebChromeClient(new MyWebChromeClient());
        }

        UploadHandler mUploadHandler;
        @Override
        protected void onActivityResult(int requestCode, int resultCode, Intent intent) {
            if (requestCode == Controller.FILE_SELECTED) {
                // Chose a file from the file picker.
                if (mUploadHandler != null) {
                    mUploadHandler.onResult(resultCode, intent);
                }
            }
            super.onActivityResult(requestCode, resultCode, intent);
        }

        @SuppressLint("NewApi")
        class MyWebChromeClient extends WebChromeClient {
            public MyWebChromeClient() {

            }
            @SuppressLint("NewApi")
            private String getTitleFromUrl(String url) {
                String title = url;
                try {
                    URL urlObj = new URL(url);
                    String host = urlObj.getHost();
                    if (host != null && !host.isEmpty()) {
                        return urlObj.getProtocol() + "://" + host;
                    }
                    if (url.startsWith("file:")) {
                        String fileName = urlObj.getFile();
                        if (fileName != null && !fileName.isEmpty()) {
                            return fileName;
                        }
                    }
                } catch (Exception e) {
                    // ignore
                }

                return title;
            }

            @Override
            public boolean onJsAlert(WebView view, String url, String message, final JsResult result) {
                String newTitle = getTitleFromUrl(url);

                new AlertDialog.Builder(MainActivity.this).setTitle(newTitle).setMessage(message).setPositiveButton(android.R.string.ok, new DialogInterface.OnClickListener() {
                    @Override
                    public void onClick(DialogInterface dialog, int which) {
                        result.confirm();
                    }
                }).setCancelable(false).create().show();
                return true;
                // return super.onJsAlert(view, url, message, result);
            }

            @Override
            public boolean onJsConfirm(WebView view, String url, String message, final JsResult result) {

                String newTitle = getTitleFromUrl(url);

                new AlertDialog.Builder(MainActivity.this).setTitle(newTitle).setMessage(message).setPositiveButton(android.R.string.ok, new DialogInterface.OnClickListener() {
                    @Override
                    public void onClick(DialogInterface dialog, int which) {
                        result.confirm();
                    }
                }).setNegativeButton(android.R.string.cancel, new DialogInterface.OnClickListener() {
                    public void onClick(DialogInterface dialog, int which) {
                        result.cancel();
                    }
                }).setCancelable(false).create().show();
                return true;

                // return super.onJsConfirm(view, url, message, result);
            }

            // Android 2.x
            public void openFileChooser(ValueCallback<Uri> uploadMsg) {
                openFileChooser(uploadMsg, "");
            }

            // Android 3.0
            public void openFileChooser(ValueCallback<Uri> uploadMsg, String acceptType) {
                openFileChooser(uploadMsg, "", "filesystem");
            }

            // Android 4.1
            public void openFileChooser(ValueCallback<Uri> uploadMsg, String acceptType, String capture) {
                mUploadHandler = new UploadHandler(new Controller());
                mUploadHandler.openFileChooser(uploadMsg, acceptType, capture);
            }
        };

        class Controller {
            final static int FILE_SELECTED = 4;

            Activity getActivity() {
                return MainActivity.this;
            }
        }

        class UploadHandler {
            /*
            * The Object used to inform the WebView of the file to upload.
            */
            private ValueCallback<Uri> mUploadMessage;
            private String mCameraFilePath;
            private boolean mHandled;
            private boolean mCaughtActivityNotFoundException;
            private Controller mController;
            public UploadHandler(Controller controller) {
                mController = controller;
            }
            String getFilePath() {
                return mCameraFilePath;
            }
            boolean handled() {
                return mHandled;
            }
            void onResult(int resultCode, Intent intent) {
                if (resultCode == Activity.RESULT_CANCELED && mCaughtActivityNotFoundException) {
                    // Couldn't resolve an activity, we are going to try again so skip
                    // this result.
                    mCaughtActivityNotFoundException = false;
                    return;
                }
                Uri result = intent == null || resultCode != Activity.RESULT_OK ? null
                        : intent.getData();
                // As we ask the camera to save the result of the user taking
                // a picture, the camera application does not return anything other
                // than RESULT_OK. So we need to check whether the file we expected
                // was written to disk in the in the case that we
                // did not get an intent returned but did get a RESULT_OK. If it was,
                // we assume that this result has came back from the camera.
                if (result == null && intent == null && resultCode == Activity.RESULT_OK) {
                    File cameraFile = new File(mCameraFilePath);
                    if (cameraFile.exists()) {
                        result = Uri.fromFile(cameraFile);
                        // Broadcast to the media scanner that we have a new photo
                        // so it will be added into the gallery for the user.
                        mController.getActivity().sendBroadcast(
                                new Intent(Intent.ACTION_MEDIA_SCANNER_SCAN_FILE, result));
                    }
                }
                mUploadMessage.onReceiveValue(result);
                mHandled = true;
                mCaughtActivityNotFoundException = false;
            }
            void openFileChooser(ValueCallback<Uri> uploadMsg, String acceptType, String capture) {
                final String imageMimeType = "image/*";
                final String videoMimeType = "video/*";
                final String audioMimeType = "audio/*";
                final String mediaSourceKey = "capture";
                final String mediaSourceValueCamera = "camera";
                final String mediaSourceValueFileSystem = "filesystem";
                final String mediaSourceValueCamcorder = "camcorder";
                final String mediaSourceValueMicrophone = "microphone";
                // According to the spec, media source can be 'filesystem' or 'camera' or 'camcorder'
                // or 'microphone' and the default value should be 'filesystem'.
                String mediaSource = mediaSourceValueFileSystem;
                if (mUploadMessage != null) {
                    // Already a file picker operation in progress.
                    return;
                }
                mUploadMessage = uploadMsg;
                // Parse the accept type.
                String params[] = acceptType.split(";");
                String mimeType = params[0];
                if (capture.length() > 0) {
                    mediaSource = capture;
                }
                if (capture.equals(mediaSourceValueFileSystem)) {
                    // To maintain backwards compatibility with the previous implementation
                    // of the media capture API, if the value of the 'capture' attribute is
                    // "filesystem", we should examine the accept-type for a MIME type that
                    // may specify a different capture value.
                    for (String p : params) {
                        String[] keyValue = p.split("=");
                        if (keyValue.length == 2) {
                            // Process key=value parameters.
                            if (mediaSourceKey.equals(keyValue[0])) {
                                mediaSource = keyValue[1];
                            }
                        }
                    }
                }
                //Ensure it is not still set from a previous upload.
                mCameraFilePath = null;
                if (mimeType.equals(imageMimeType)) {
                    if (mediaSource.equals(mediaSourceValueCamera)) {
                        // Specified 'image/*' and requested the camera, so go ahead and launch the
                        // camera directly.
                        startActivity(createCameraIntent());
                        return;
                    } else {
                        // Specified just 'image/*', capture=filesystem, or an invalid capture parameter.
                        // In all these cases we show a traditional picker filetered on accept type
                        // so launch an intent for both the Camera and image/* OPENABLE.
                        Intent chooser = createChooserIntent(createCameraIntent());
                        chooser.putExtra(Intent.EXTRA_INTENT, createOpenableIntent(imageMimeType));
                        startActivity(chooser);
                        return;
                    }
                } else if (mimeType.equals(videoMimeType)) {
                    if (mediaSource.equals(mediaSourceValueCamcorder)) {
                        // Specified 'video/*' and requested the camcorder, so go ahead and launch the
                        // camcorder directly.
                        startActivity(createCamcorderIntent());
                        return;
                } else {
                        // Specified just 'video/*', capture=filesystem or an invalid capture parameter.
                        // In all these cases we show an intent for the traditional file picker, filtered
                        // on accept type so launch an intent for both camcorder and video/* OPENABLE.
                        Intent chooser = createChooserIntent(createCamcorderIntent());
                        chooser.putExtra(Intent.EXTRA_INTENT, createOpenableIntent(videoMimeType));
                        startActivity(chooser);
                        return;
                    }
                } else if (mimeType.equals(audioMimeType)) {
                    if (mediaSource.equals(mediaSourceValueMicrophone)) {
                        // Specified 'audio/*' and requested microphone, so go ahead and launch the sound
                        // recorder.
                        startActivity(createSoundRecorderIntent());
                        return;
                    } else {
                        // Specified just 'audio/*',  capture=filesystem of an invalid capture parameter.
                        // In all these cases so go ahead and launch an intent for both the sound
                        // recorder and audio/* OPENABLE.
                        Intent chooser = createChooserIntent(createSoundRecorderIntent());
                        chooser.putExtra(Intent.EXTRA_INTENT, createOpenableIntent(audioMimeType));
                        startActivity(chooser);
                        return;
                    }
                }
                // No special handling based on the accept type was necessary, so trigger the default
                // file upload chooser.
                startActivity(createDefaultOpenableIntent());
            }
            private void startActivity(Intent intent) {
                try {
                    mController.getActivity().startActivityForResult(intent, Controller.FILE_SELECTED);
                } catch (ActivityNotFoundException e) {
                    // No installed app was able to handle the intent that
                    // we sent, so fallback to the default file upload control.
                    try {
                        mCaughtActivityNotFoundException = true;
                        mController.getActivity().startActivityForResult(createDefaultOpenableIntent(),
                                Controller.FILE_SELECTED);
                    } catch (ActivityNotFoundException e2) {
                        // Nothing can return us a file, so file upload is effectively disabled.
                        Toast.makeText(mController.getActivity(), R.string.uploads_disabled,
                                Toast.LENGTH_LONG).show();
                    }
                }
            }
            private Intent createDefaultOpenableIntent() {
                // Create and return a chooser with the default OPENABLE
                // actions including the camera, camcorder and sound
                // recorder where available.
                Intent i = new Intent(Intent.ACTION_GET_CONTENT);
                i.addCategory(Intent.CATEGORY_OPENABLE);
                i.setType("*/*");
                Intent chooser = createChooserIntent(createCameraIntent(), createCamcorderIntent(),
                        createSoundRecorderIntent());
                chooser.putExtra(Intent.EXTRA_INTENT, i);
                return chooser;
            }
            private Intent createChooserIntent(Intent... intents) {
                Intent chooser = new Intent(Intent.ACTION_CHOOSER);
                chooser.putExtra(Intent.EXTRA_INITIAL_INTENTS, intents);
                chooser.putExtra(Intent.EXTRA_TITLE,
                        mController.getActivity().getResources()
                                .getString(R.string.choose_upload));
                return chooser;
            }
            private Intent createOpenableIntent(String type) {
                Intent i = new Intent(Intent.ACTION_GET_CONTENT);
                i.addCategory(Intent.CATEGORY_OPENABLE);
                i.setType(type);
                return i;
            }
            private Intent createCameraIntent() {
                Intent cameraIntent = new Intent(MediaStore.ACTION_IMAGE_CAPTURE);
                File externalDataDir = Environment.getExternalStoragePublicDirectory(
                        Environment.DIRECTORY_DCIM);
                File cameraDataDir = new File(externalDataDir.getAbsolutePath() +
                        File.separator + "browser-photos");
                cameraDataDir.mkdirs();
                mCameraFilePath = cameraDataDir.getAbsolutePath() + File.separator +
                        System.currentTimeMillis() + ".jpg";
                cameraIntent.putExtra(MediaStore.EXTRA_OUTPUT, Uri.fromFile(new File(mCameraFilePath)));
                return cameraIntent;
            }
            private Intent createCamcorderIntent() {
                return new Intent(MediaStore.ACTION_VIDEO_CAPTURE);
            }
            private Intent createSoundRecorderIntent() {
                return new Intent(MediaStore.Audio.Media.RECORD_SOUND_ACTION);
            }
        }
    }

But when I had done all this I saw that you had things like this.

webView.loadUrl("http://Example.com/");

// Complete with your code

webView.loadUrl("http://Example.com/");

These tell me that this isn't code that you have written yourself.


Better code for error handling in NodeJs and Express I have this code for a post request, when the user wants to change his password. Because of all the cases and the following page renders the code came out really ugly. Is there a better way to structure this? (It works fine and does what I want.)

// Check if old password is correct
    SQL.getUserFromDB(request.session.username).then(function (results) {
        // Hash and compare with stored hash
        bcrypt.compare(request.body.oldPw, results[0].password, function (error, result) {
            // Log possible error
            if (error) console.log(error);
            if (result === true) {
                // Check if new passwords are both the same
                if (request.body.newPw === request.body.newPw2) {
                    // Call mysql function
                    SQL.changeUserPassword(request.session.username, request.body.newPw).then(function () {
                        response.render('pages/changePassword', {
                            user: request.session.username,
                            text: 'Passwort erfolgreich geändert.'
                        });
                    }).catch(function (error) {
                        console.log(error);
                        if (error == 'pw') {
                            response.render('pages/changePassword', {
                                user: request.session.username,
                                text: 'Neues Passwort zu unsicher.'
                            });
                        } else {
                            // Render error page
                            response.render('pages/changePassword', {
                                user: request.session.username,
                                text: 'Fehler beim Ändern des Passworts.'
                            });
                        }
                    });
                } else {
                    // Render error page
                    response.render('pages/changePassword', {
                        user: request.session.username,
                        text: 'Neue Passwörter stimmen nicht überein!'
                    });
                }
            } else {
                // Render error page
                response.render('pages/changePassword', {
                    user: request.session.username,
                    text: 'Altes Passwort stimmt nicht überein!'
                });
            }
        });
        // Catch sql errorsFehler beim Ändern des Passworts
    }).catch(function (error) {
        if (error) console.log(error);
        response.render('pages/errors/loginFailed');
    });

I tried just setting the text in the different cases and rendering one page with the text at the bottom, this didn't work however.

What you first need to do is convert all non-promise/callback-based operations into promises, or wrap them in one. This way, you can write seamless promise-based code. For instance, convert bcrypt.compare() into:

const bcryptCompare = (pw1, pw2) => {
  return new Promise((resolve, reject) => {
    bcrypt.compare(pw1, pw2, (error, result) => {
      if(error) reject(error)
      else resolve(result)
    })
  })
}

There are libraries that do this for you, and newer versions of libraries usually support promise-based versions of their APIs. But if you ever need to craft this yourself, this is how it's generally done.

Next, I would separate express from this logic. This way, you're not constrained with Express. Instead of accessing request, pass only the needed information to your function. Instead of immediately sending a response, consider throwing error objects instead. This way, you can defer your response logic to your routing logic.

Next, you could turn to async/await for cleaner code. It's just syntax sugar on top of Promises, allowing asynchronous code to look synchronous.

// Custom error class to house additional info. Also used for determining the
// correct response later.
class PasswordMismatch extends Error {
  constructor(message, user){
    this.message = 'Neue Passwörter stimmen nicht überein!'
    this.user = user
  }
}

class UserDoesNotExist extends Error {...}
class PasswordIncorrect extends Error {...}

// More custom error classes here

// Our logic, which knows nothing about express and only requires a few
// pieces of information for our operation.
const changePassword = async (username, oldPw, newPw, newPw2) => {

  // Throwing an error inside an async function rejects the promise it returns
  if (newPw !== newPw2) {
    throw new PasswordMismatch('Neue Passwörter stimmen nicht überein!', user)
  }

  const results = await SQL.getUserFromDB(username)

  if (!results) {
    throw new UserDoesNotExist('...', user)
  }

  const result = bcryptCompare(oldPw, results[0].password)

  if(result !== true) {
    throw new PasswordIncorrect('...', user)
  }

  try {
    SQL.changeUserPassword(username, newPw)
  } catch(e) {
    throw new PasswordChangeFailed('...', user)
  }
}

// In your express router
app.post('route', async (request, response) => {
  // Gather the necessary data from the request
  const username = request.session.username
  const oldPw = request.body.oldPw
  const newPw = request.body.newPw
  const newPw2 = request.body.newPw2

  try {
    await changePassword(username, oldPw, newPw, newPw2)

    // If the operation didn't throw, then we're good. Happy path.
    response.render('pages/changePassword', {
      user: username,
      text: 'Passwort erfolgreich geändert.'
    });
  } catch (e) {
    // If any of the operations failed, then we determine the right response

    // In this case, we used a custom error. We can get the data from it.
    // You can have more of these for the other custom classes.
    if(error instanceof PasswordConfirmError){
      response.render('pages/changePassword', {
        user: error.user,
        text: error.message
      });

    } else {
      // Generic 500 error, if all else fails.
    }
  }
})


Rearrange data without using Cut and Insert I'm fairly new to VBA, and this was basically a brute-force solution to a problem I was encountering. I wanted to take data that appeared in two columns, and pull it together into one.

The current code works, but is very slow with large datasets. I've been told to avoid using the clipboard if possible, but I'm not quite sure where to begin with this. I've made a few attempts to use an array, but I'm not quite sure where to start. Any other suggestions would be very welcome.

Private Sub Arra()

    Dim Library As Worksheet
    Set Library = Sheets("Library")
    Dim Rng As Range
    Dim i As Long
    Dim lastRow As Long
    i = 1
    lastRow = Library.Range("A1").SpecialCells(xlCellTypeLastCell).row

    While i <= lastRow
    Set Rng = Library.Range("A" & i)
         If Application.WorksheetFunction.CountA(Rng.Offset(0, 1)) = 1 Then
             Rng.Offset(0, 1).Cut
             Rng.Offset(1, 0).Insert Shift:=xlDown
             Rng.Offset(0, 1).Insert Shift:=xlDown
        ElseIf Application.WorksheetFunction.CountA(Rng.Offset(0, 1)) = 0 Then
            i = i + 1
        End If
    Wend

End Sub

To speed it up, I would read it into arrays. One array for column A and one array for column B and then combine them into another array and print that to sheet

Option Explicit
Sub Rearrange()
    Dim lastRow As Long
    lastRow = Library.Cells(Rows.Count, 1).End(xlUp).Row
    Dim firstColumn As Variant
    firstColumn = Library.Range("A1:A" & lastRow)
    Dim secondColumn As Variant
    secondColumn = Library.Range("B1:B" & lastRow)
    Dim totalCount As Long
    totalCount = Application.CountA(firstColumn) + Application.CountA(secondColumn)

    Dim combinedArray As Variant
    ReDim combinedArray(1 To totalCount)
    Dim i As Long
    Dim index As Long
    index = 1

    For i = 1 To lastRow
        combinedArray(index) = firstColumn(i, 1)
        index = index + 1
        If Not IsEmpty(secondColumn(i, 1)) Then
            combinedArray(index) = secondColumn(i, 1)
            index = index + 1
        End If
    Next

    Library.Range("A1:A" & totalCount) = Application.Transpose(combinedArray)

End Sub

Arrays are fast!

Also, as you can see worksheets have a CodeName property - View Properties window (F4) and the (Name) field (the one at the top) can be used as the worksheet name. This way you can avoid Sheets("Library") and instead just use Library.

I also switched your

lastRow = Library.Range("A1").SpecialCells(xlCellTypeLastCell).row

To the standard.

I also used a For loop instead of Do While.


Extended exception for network errors in Python I need a custom exception for network errors that holds a related exception and more than one URL (the first one as the used service has its URL but it indirectly calls other URLs), so I wrote this:

class ExtendedNetworkError(Exception):

    def __init__(self, reason, *args, **kwargs):
        kwargs_copy = dict(kwargs)
        main_url, related_exception = self._move_kwargs(kwargs_copy)
        Exception.__init__(self, reason, **kwargs_copy)
        self.reason = reason
        self.main_url = main_url
        self.related_exception = related_exception

    def _move_kwargs(self, kwargs):
        moved = []
        for kwarg in ['main_url', 'related_exception']:
            moved.append(kwargs.get(kwarg))
            if kwarg in kwargs:
                del kwargs[kwarg]
        return tuple(moved)

    def as_dict(self):
        d = OrderedDict()
        for attr in ['reason', 'main_url', 'related_exception']:
            d[attr] = self.__dict__.get(attr)
        return d

Problems? Improvements?

Let's go bottom-to-top:

d[attr] = self.__dict__.get(attr)

Manipulating the __dict__ of an object is rarely necessary as it is often cleaner to use getattr or setattr instead. Here, the exact replacement would be d[attr] = getattr(self, attr, None) because dict.get default to None itself. However, here you know in advance that all 3 attributes are set so you could d[attr] = getattr(self, attr) instead.

def as_dict(self):

You can turn that initialization loop into a generator expression and feed that to the OrderedDict constructor. Doing so you’re not modifying d after building it so you could directly return it:

def as_dict(self):
    return OrderedDict((attr, getattr(self, attr)) for attr in ['reason', 'main_url', 'related_exception'])

def _move_kwargs(self, kwargs):

So… you need to get some values out of a dictonary and remove the corresponding entry afterwards: you should look into the dict.pop method:

def _move_kwargs(self, kwargs):
     return kwargs.pop('main_url', None), kwargs.pop('related_exception', None)

In fact, it would feel cleaner to do these operations directly into __init__.

kwargs_copy = dict(kwargs)

You do not need to copy that dictionary to work with parameter value. Deleting or poping items from this dictionary is fine.

Exception.__init__(self, reason, **kwargs_copy)

You should ask super to call the super class __init__ method for you. This is not so much of a difference with this code but it's a good habit to have, especially if you happen to need multiple inheritance or complex inheritance scheme.

super(ExtendedNetworkError, self).__init__(reason, **kwargs)

or, using Python 3, you can simplify the call to:

super().__init__(reason, **kwargs)

So far, the code has been reduced to:

class ExtendedNetworkError(Exception):
    def __init__(self, reason, *args, **kwargs):
        self.reason = reason
        self.main_url = kwargs.pop('main_url', None)
        self.related_exception = kwargs.pop('related_exception', None)
        super().__init__(reason, **kwargs)

    def as_dict(self):
        return = OrderedDict(
            (attr, getattr(self, attr))
            for attr in ['reason', 'main_url', 'related_exception']
        )

Now there is this *args parameter in __init__. Since you’re not using it, you should remove it. This will force the need to use named parameter when specifying more than the reason.

Last thing you can do to improve the code is to add some docstrings explaining what this class is about.


Count frequency of characters in a file Some patterns are emerging ("fmap (b->a) . fmap (c->b) . .. . (IO z)"). Also infix zip is kind-of a hack.

What is:

Best practice in point-free style?
Best practice?

Elegance > Performance;  Functional > Imperative

import qualified Data.Map.Lazy as M
import qualified Data.ByteString.Char8 as BS

fromFile :: FilePath -> IO (M.Map Char Int)
fromFile = fmap (M.fromListWith (+)) . fmap (`zip` [1,1..]) . readFile
    where readFile = fmap BS.unpack . BS.readFile

Best practice would be to separate the pure operations of your program from those that actually require IO. Counting the frequency of elements in a list doesn't require any IO, so you should tease that fromFile function apart into its constituent components for reusability, testing, comprehensibility, or whatever other purpose you'd like.

frequencies :: Ord k => [k] -> Map k Int
frequencies = fromListWith (+) . (`zip` [1,1..])

fromFile :: FilePath -> IO (Map Char Int)
fromFile = fmap frequencies . fmap unpack . readFile

I'd tweak this just a bit further, using repeat from the Prelude to build the infinite list instead of abusing list ranges, and to take advantage of the Functor laws and drop a few characters. I flip back and forth on writing functions in pointfree style when it requires infix sectioning too, in this case I'd probably keep the points but I don't know that one choice is clearly better than the other.

frequencies ks = fromListWith (+) $ zip ks (repeat 1)

fromFile = fmap (frequencies . unpack) . readFile


phpMailer script for a contact form I've just downloaded a script and would like to use it as a contact form on my site. The problem is I'm a front-end dev with little knowledge of securing PHP code. So, could you please have a look and let me know if there are any glaring problems with security with this script that uses phpMailer to send?

As you can see, I haven't done much with the SMTP setting at the bottom, but it still seems to work ok on my server.

<?php

$name = trim($_POST['name']);
$email = $_POST['email'];
$comments = $_POST['comments'];

$site_owners_email = 'email@someemail.com'; // Replace this with your own email address
$site_owners_name = 'Name'; // replace with your name

if (strlen($name) < 2) {
    $error['name'] = "Please enter your name";
}

if (!preg_match('/^[a-z0-9&\'\.\-_\+]+@[a-z0-9\-]+\.([a-z0-9\-]+\.)*+[a-z]{2}/is', $email)) {
    $error['email'] = "Please enter a valid email address";
}

if (strlen($comments) < 1) {
    $error['comments'] = "Please leave a comment.";
}

if (!$error) {

    require_once('class.phpmailer.php');
    $mail = new PHPMailer();

    $mail->From = $email;
    $mail->FromName = $name;
    $mail->Subject = "Website Contact Form";
    $mail->AddAddress($site_owners_email, $site_owners_name);
    $mail->AddAddress('email@someemail.com', 'Name');
    $mail->Body="
    Name: $name
    Email: $email
    Comments: $comments";

    $mail->SMTPAuth = true; // turn on SMTP authentication
    $mail->Username = "email@someemail.com"; // SMTP username
    $mail->Password = ""; // SMTP password

    $mail->Send();

    echo "
 Congratulations, " . $name . " We've received your email. We'll be in touch as soon as we possibly can!

close

return to home page
";

} # end if no error
else {

    $response = (isset($error['name'])) ? "
" . $error['name'] . "
 \n" : null;
    $response .= (isset($error['email'])) ? "
" . $error['email'] . "
 \n" : null;
    $response .= (isset($error['comments'])) ? "
" . $error['comments'] . "
" : null;

    echo $response;
} # end if there was an error sending

?>

As @e-sushi remarked in his answer: you're still vulnerable to header injections. Though his approach is rather complex, and (when applied to $email, not required if you validate the email address the way I do (explained further down), but for the subject, and message body, you could use these simpler, and (IMO, therefore) better ways to tackle the header-injection issue:

$mail->Body = str_ireplace(
    array("%0a", "%0d", '0x0A'), '',//PHPMailer replaces \r\n already for you
   'your string here...'
);
//a Core-function alternative:
$body = 'your string here';
filter_var($body, FILTER_SANITIZE_EMAIL);//will alter $body
$mail->Body = $body;

More details on the caveats/benefits of each approach can be found here. Because you're setting the $name along side the email address, too as the from of the email, you should pass the $name value to either one of these filters, too.

Your code is attempting to validate an email address using a regular expression that is, at best, slightly off. There is a built-in function that can validate email addresses, without an inaccurate, too restrictive and slow regular expression, called filter_var.

if (!filter_var($_POST['email'], FILTER_VALIDATE_EMAIL))
{
    exit('Invalid email');
}

The result is that, while your regex doesn't allow for: some.name@site.info to be used as an email address (even though it's valid), filter_Var will accept it.
Unlike your regex, which, because it uses the s modifier allows for line-breaks in your email (for some reason), this won't... not blindly.
If, however, you wish to keep on using a regular expression, here's a PCRE expression that is (almost) fully RFC822 compliant:

/(?:(?:\r\n)?[ \t])*(?:(?:(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t]
)+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))*"(?:(?:
\r\n)?[ \t])*)(?:\.(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(
?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[
\t]))*"(?:(?:\r\n)?[ \t])*))*@(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\0
31]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\
](?:(?:\r\n)?[ \t])*)(?:\.(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+
(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:
(?:\r\n)?[ \t])*))*|(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z
|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))*"(?:(?:\r\n)
?[ \t])*)*\<(?:(?:\r\n)?[ \t])*(?:@(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\
r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[
 \t])*)(?:\.(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)
?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t]
)*))*(?:,@(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[
 \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*
)(?:\.(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t]
)+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*))*)
*:(?:(?:\r\n)?[ \t])*)?(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+
|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))*"(?:(?:\r
\n)?[ \t])*)(?:\.(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:
\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t
]))*"(?:(?:\r\n)?[ \t])*))*@(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031
]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\](
?:(?:\r\n)?[ \t])*)(?:\.(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+(?
:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?
:\r\n)?[ \t])*))*\>(?:(?:\r\n)?[ \t])*)|(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?
:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?
[ \t]))*"(?:(?:\r\n)?[ \t])*)*:(?:(?:\r\n)?[ \t])*(?:(?:(?:[^()<>@,;:\\".\[\]
\000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|
\\.|(?:(?:\r\n)?[ \t]))*"(?:(?:\r\n)?[ \t])*)(?:\.(?:(?:\r\n)?[ \t])*(?:[^()<>
@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"
(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))*"(?:(?:\r\n)?[ \t])*))*@(?:(?:\r\n)?[ \t]
)*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\
".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*)(?:\.(?:(?:\r\n)?[ \t])*(?
:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[
\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*))*|(?:[^()<>@,;:\\".\[\] \000-
\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(
?:(?:\r\n)?[ \t]))*"(?:(?:\r\n)?[ \t])*)*\<(?:(?:\r\n)?[ \t])*(?:@(?:[^()<>@,;
:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([
^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*)(?:\.(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\"
.\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\
]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*))*(?:,@(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\
[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\
r\\]|\\.)*\](?:(?:\r\n)?[ \t])*)(?:\.(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\]
\000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]
|\\.)*\](?:(?:\r\n)?[ \t])*))*)*:(?:(?:\r\n)?[ \t])*)?(?:[^()<>@,;:\\".\[\] \0
00-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\
.|(?:(?:\r\n)?[ \t]))*"(?:(?:\r\n)?[ \t])*)(?:\.(?:(?:\r\n)?[ \t])*(?:[^()<>@,
;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|"(?
:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))*"(?:(?:\r\n)?[ \t])*))*@(?:(?:\r\n)?[ \t])*
(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".
\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*)(?:\.(?:(?:\r\n)?[ \t])*(?:[
^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\]
]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*))*\>(?:(?:\r\n)?[ \t])*)(?:,\s*(
?:(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\
".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))*"(?:(?:\r\n)?[ \t])*)(?:\.(?:(
?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[
\["()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))*"(?:(?:\r\n)?[ \t
])*))*@(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t
])+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*)(?
:\.(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|
\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*))*|(?:
[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".\[\
]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))*"(?:(?:\r\n)?[ \t])*)*\<(?:(?:\r\n)
?[ \t])*(?:@(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["
()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*)(?:\.(?:(?:\r\n)
?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>
@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*))*(?:,@(?:(?:\r\n)?[
 \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,
;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*)(?:\.(?:(?:\r\n)?[ \t]
)*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\
".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*))*)*:(?:(?:\r\n)?[ \t])*)?
(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\["()<>@,;:\\".
\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))*"(?:(?:\r\n)?[ \t])*)(?:\.(?:(?:
\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z|(?=[\[
"()<>@,;:\\".\[\]]))|"(?:[^\"\r\\]|\\.|(?:(?:\r\n)?[ \t]))*"(?:(?:\r\n)?[ \t])
*))*@(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])
+|\Z|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*)(?:\
.(?:(?:\r\n)?[ \t])*(?:[^()<>@,;:\\".\[\] \000-\031]+(?:(?:(?:\r\n)?[ \t])+|\Z
|(?=[\["()<>@,;:\\".\[\]]))|\[([^\[\]\r\\]|\\.)*\](?:(?:\r\n)?[ \t])*))*\>(?:(
?:\r\n)?[ \t])*))*)?;\s*)/

Source: ex-parrot

I think it's clear which one of the validations is the more elegant one...

The name ($name) is a trimmed version of the POST var. That's good, because you know that, if the strlen($name) > 2, you're not just counting whitespace. However, you still are counting possible whitespace-only input when validating the $comment variable.

I'd suggest additional processing on both of these values:

$name = trim(html_entity_decode($_POST['name']));
if (strlen($name) < 2 || !preg_match('/^[^\d]+\s*[^\d]*/',$name))
{//nobody is called R2D2 in real-life!
    exit('name is too short ( < 2 chars) or contains numeric chars');
}
$comment = trim(strip_tags(html_entity_decode($_POST['comment'])));
if (!strlen($comment))
{//empty comment
   exit('No comment supplied');
}

As you can see, I've also used html_entity_decode and, for the comments at least strip_tags to remove any markup that might have been passed. This is often overlooked, so if I were to post a comment saying:

&nbsp;<br/>&nbsp;

Even trim would see this as a valid comment, whereas it actually is little more than whitespace


Strategy Design Pattern in Python I'm reading the awesome Head First Design Patterns book. As an exercise I converted first example (or rather a subset of it) to Python. The code I got is rather too simple, i.e. there is no abstract nor interface declarations, it's all just 'classes'. Is that the right way to do it in Python? My code is below, original Java code and problem statement can be found at http://books.google.com/books?id=LjJcCnNf92kC&pg=PA18

class QuackBehavior():
    def __init__(self):
        pass
    def quack(self):
        pass

class Quack(QuackBehavior):
    def quack(self):
        print "Quack!"

class QuackNot(QuackBehavior):
    def quack(self):
        print "..."

class Squeack(QuackBehavior):
    def quack(self):
        print "Squeack!"

class FlyBehavior():
    def fly(self):
        pass

class Fly():
    def fly(self):
        print "I'm flying!"

class FlyNot():
    def fly(self):
        print "Can't fly..."

class Duck():
    def display(self):
        print this.name

    def performQuack(self):
        self.quackBehavior.quack()

    def performFly(self):
        self.flyBehavior.fly()

class MallardDuck(Duck):
    def __init__(self):
        self.quackBehavior = Quack()
        self.flyBehavior = Fly()

if __name__ == "__main__":
    mallard = MallardDuck()
    mallard.performQuack()
    mallard.performFly()
    mallard.flyBehavior = FlyNot()
    mallard.performFly()

class QuackBehavior():

Don't put () after classes, either skip it, or put object in there

    def __init__(self):
        pass

There's no reason to define a constructor if you aren't going to do anything, so just skip  it

    def quack(self):
        pass

You should probably at least raise NotImplementedError(), so that if anyone tries to call this it'll complain. That'll also make it clear what this class is doing.

You don't really need this class at all. The only reason to provide classes like this in python is documentation purposes. Whether or not you think that's useful enough is up to you.

class Quack(QuackBehavior):
    def quack(self):
        print "Quack!"

class QuackNot(QuackBehavior):
    def quack(self):
        print "..."

class Squeack(QuackBehavior):
    def quack(self):
        print "Squeack!"

class FlyBehavior():
    def fly(self):
        pass

class Fly():

If you are going to defined FlyBehavior, you should really inherit from it. It just makes it a litle more clear what you are doing.

    def fly(self):
        print "I'm flying!"

class FlyNot():
    def fly(self):
        print "Can't fly..."

class Duck():

I'd define a constructor taking the quack and fly behavior here.

    def display(self):
        print this.name

    def performQuack(self):
        self.quackBehavior.quack()

    def performFly(self):
        self.flyBehavior.fly()

class MallardDuck(Duck):
    def __init__(self):
        self.quackBehavior = Quack()
        self.flyBehavior = Fly()

There's not really a reason for this to be a class. I'd make a function that sets up the Duck and returns it.

if __name__ == "__main__":
    mallard = MallardDuck()
    mallard.performQuack()
    mallard.performFly()
    mallard.flyBehavior = FlyNot()
    mallard.performFly()


Servlet responses to upload/download requests My task is to write a program to store files in object store. One of the subtasks requires to send json or xml format response to the file uploader/downloader as requested by client.
The following rules must be followed:

                         entry
               +-------------------------+
               v                         v
           upload                      download
       +-----------+              +--------------+
       v           v              v              v
    succ          err            succ           err
  +-----+       +-----+                       +-----+
  v     v       v     v                       v     v
json1  xml1    json2  xml2                  josn2  xml2

if user uploads file with success, return json/xml type 1
if user uploads file with error, return json/xml type 2
if user downloads file with success, don't change return object
if user downloads file with error, return json/xml type 2

I've managed to solve the task but I'm not satisfied with the class dependencies and nested if/else-s I have. I think mixture of abstract factory and strategy are suitable for the issue. Can you give me a hint on how I can deal with the problem?
My code snippet:
public Result upload(HttpServletRequest request) {
        UploadRequestParams uploadParams = UploadRequestParams.getObjectFrom(request);
        ... // create info object
        Result res = handleUploadRequest(uploadParams);

        Result result = jsonXml(info, res);
        return result;
    }

private Result jsonXml(RequestInfo info, Result result) {
        Serializer ser;
        if ("xml".equals(format)) {
            ser = new XmlSerializer();
        } else {
            ser = new JsonSerializer();
        }
        Result res = ser.serialize(info, result);

        return res;
    }

@Override
public Result serialize(RequestInfo info, Result result) { // json implementation
        GsonBuilder builder = new GsonBuilder();
        Gson g = builder.create();

        InputStream stream = null;
        InfoInterface wrap;
        if (info.isUpload()) {
            if (result.isValid()) {
                wrap = new InfoWrapperImpl1(info, result);
            } else {
                wrap = new InfoWrapperImpl2(info, result);
            }
        } else {
            wrap = new InfoWrapperImpl1(info, result);
        }
        String res = g.toJson(wrap);
        stream = new ByteArrayInputStream(res.getBytes(StandardCharsets.UTF_8));
        Result newRes = Result.getBuilder().setInputStream(stream).setResultCode(result.getResultCode()).build();
        return newRes;
    }

private class InfoWrapperImpl1 implements InfoInterface {
        private String fileName;
        private ResultCode code;

        public InfoWrapper1(RequestInfo info, Result res) {
            this.fileName = info.getUniqueName();
            this.code = res.getCode;
        }
    }

UML class diagram:

EDIT
I rewrote serialize method, but looks like to be bound to the parameters' implementation. Am I right? On the other, I got rid of the ugly nested classes.
@Override
    public Result serialize(RequestInfo info, Result result) {

        JsonObject body = new JsonObject();

        JsonObject resultCode = new JsonObject();
        resultCode.addProperty("code", result.getResultCode().getCode());
        resultCode.addProperty("value", result.getResultCode().getDescription());

        if (info.isUploadRequest() && result.isValid()) {
            body.addProperty(fileTag, info.getExtendedName());
            body.addProperty("checksum", result.getMySwiftObject().getEtag());
        }
        body.add(statusTag, resultCode);

        String jsonRes = body.toString();
        InputStream stream = new ByteArrayInputStream(jsonRes.getBytes(StandardCharsets.UTF_8));
        return Result.getBuilder().setInputStream(stream).setResultCode(result.getResultCode()).build();
    }

if (info.isUpload()) {
        if (result.isValid()) {
            wrap = new InfoWrapperImpl1(info, result);
        } else {
            wrap = new InfoWrapperImpl2(info, result);
        }
    } else {
        wrap = new InfoWrapperImpl1(info, result);
    }

Can be written as:

   if (!info.isUpload() || result.isValid()) {
        wrap = new InfoWrapperImpl1(info, result);
    } else {
        wrap = new InfoWrapperImpl2(info, result);
    }

Also InfoWrapperImpl1 and InfoWrapperImpl2 are not very good names, as they are not expressive and look quite similar to each other.


Python Flask website I'm new to programming and in back-end. Right now I'm trying to learn flask, so I wrote a very simple website that currently encrypts and decrypts the message using RSA and DES.

I feel like my app.py file is just bad and should do it much better. I mean, there're a lot of repeating code like this:

@app.route('/rsa_encrypt', methods=['GET', 'POST'])
def RSA_enc():
    global get_form
    if request.method == "GET":
        get_form = True
        return render_template('rsa_encrypt.html', gf=get_form)
    else:
        get_form = False
        rsa = RSA(request.form.get('pt'), p=request.form.get('p'),
                  q=request.form.get('q'), e=request.form.get('e'))
        return render_template('rsa_encrypt.html', gf=get_form, rsa=rsa, gspn=gspn)

I tried to write some decorators for those routes, but they didn't work well.

Can you take a look and tell me what should I do better? It works but I think not as well as it should.

app.py

from flask import Flask, render_template, request, url_for, jsonify
from ciphers import RSA, DES
from prime import generate_semiprime_number as gspn

app = Flask(__name__)
get_form = None

@app.route('/')
def index():
    return render_template('index.html')

@app.route('/rsa_encrypt', methods=['GET', 'POST'])
def RSA_enc():
    global get_form
    if request.method == "GET":
        get_form = True
        return render_template('rsa_encrypt.html', gf=get_form)
    else:
        get_form = False
        rsa = RSA(request.form.get('pt'), p=request.form.get('p'),
                  q=request.form.get('q'), e=request.form.get('e'))
        return render_template('rsa_encrypt.html', gf=get_form, rsa=rsa, gspn=gspn)

@app.route('/rsa_decrypt', methods=['GET', 'POST'])
def RSA_dec():
    global get_form
    if request.method == "GET":
        get_form = True
        return render_template('rsa_decrypt.html', gf=get_form)
    else:
        get_form = False
        ct = [int(x) for x in request.form.get('ct')[1:-1].split(',')]
        rsa = RSA(ciphertext=ct, p=request.form.get('p'),
                  q=request.form.get('q'), e=request.form.get('e'))
        return render_template('rsa_decrypt.html', gf=get_form, rsa=rsa)

@app.route('/des_encrypt', methods=['GET', 'POST'])
def DES_enc():
    global get_form
    if request.method == "GET":
        get_form = True
        return render_template('des_encrypt.html', gf=get_form)
    else:
        get_form = False
        des = DES(request.form.get('pt'), user_key=request.form.get('key'))
        return render_template('des_encrypt.html', gf=get_form, des=des)

@app.route('/des_decrypt', methods=['GET', 'POST'])
def DES_dec():
    global get_form
    if request.method == "GET":
        get_form = True
        return render_template('des_decrypt.html', gf=get_form)
    else:
        get_form = False
        des = DES(ciphertext=request.form.get('ct'), user_key=request.form.get('key'))
        return render_template('des_decrypt.html', gf=get_form, des=des)

@app.route('/gpsn', methods=['POST'])
def _gpsn():
    return jsonify({'result' : str(gspn(request.form['bits']))})

if __name__ == '__main__':
    app.run()

Other files repository. It's also hosted so you can quickly take a look.

Your main issue is (as you've already noticed) one of code repetition. Some simple helper methods here will do the trick; caveat - untested:

from flask import Flask, render_template, request, url_for, jsonify
from ciphers import RSA, DES
from prime import generate_semiprime_number as gspn

app = Flask(__name__)
get_form = None

@app.route('/')
def index():
    return render_template('index.html')

def do_rsa(encrypting):
    global get_form

    direction = 'en' if encrypting else 'de'
    tmpl = f'rsa_{direction}crypt.html'

    if request.method == 'GET':
        return render_template(tmpl, gf=True)

    kwargs = {'p': request.form.get('p'),
              'q': request.form.get('q'),
              'e': request.form.get('e')}
    if encrypting:
        rsa = RSA(request.form.get('pt'), **kwargs)
    else:
        ct = [int(x) for x in request.form.get('ct')[1:-1].split(',')]
        rsa = RSA(ciphertext=ct, **kwargs)

    kwargs = {}
    if encrypting:
        kwargs['gpsn'] = gpsn
    return render_template(tmpl, gf=False, rsa=rsa, **kwargs)

@app.route('/rsa_encrypt', methods=('GET', 'POST'))
def RSA_enc():
    return do_rsa(True)

@app.route('/rsa_decrypt', methods=('GET', 'POST'))
def RSA_dec():
    return do_rsa(False)

def do_des(encrypting):
    global get_form

    direction = 'en' if encrypting else 'de'
    tmpl = f'des_{direction}crypt.html'

    if request.method == 'GET':
        return render_template(tmpl, gf=True)

    kwargs = {'user_key': request.form.get('key')}
    if encrypting:
        des = DES(request.form.get('pt'), **kwargs)
    else:
        des = DES(ciphertext=request.form.get('ct'), **kwargs)

    return render_template(tmpl, gf=False, des=des)

@app.route('/des_encrypt', methods=('GET', 'POST'))
def DES_enc():
    return do_des(True)

@app.route('/des_decrypt', methods=('GET', 'POST'))
def DES_dec():
    return do_des(False)

@app.route('/gpsn', methods=('POST',))
def _gpsn():
    return jsonify({'result' : str(gspn(request.form['bits']))})

if __name__ == '__main__':
    app.run()


DRYing a stateless functional builder This was just an experiment to see if it was possible to build a stateless builder in ES6. I was able to do it, but when I took it a step further I got stuck trying to DRY it out.

In this case, we have a MoveableBuilder and a ParticleBuilder. A Particle is a Moveable, so the ParticleBuilder could reuse a lot of the code from the MoveableBuilder. Unfortunately, I haven't been able to figure out how to combine the two functions in an elegant way.

function ParticleBuilder(coordinates = [], vector = [], lifespan = 255, decay = 1) {
  const self = this;

  this.at = (...coordinates) =>
    new self.constructor(coordinates, vector, lifespan, decay);

  this.facing = (...vector) =>
    new self.constructor(coordinates, vector, lifespan, decay);

  this.lifespan = (lifespan) =>
    new self.constructor(coordinates, vector, lifespan, decay);

  this.decay = (decay) =>
    new self.constructor(coordinates, vector, lifespan, decay);

  this.build = () => Object.freeze({
    coordinates: coordinates,
    vector: vector,
    lifespan: lifespan,
    decay: decay
  });
}

function MoveableBuilder(coordinates = [], vector = []) {
  const self = this;

  this.at = (...coordinates) => new self.constructor(coordinates, vector);

  this.facing = (...vector) => new self.constructor(coordinates, vector);

  this.build = () => Object.freeze({
    coordinates: coordinates,
    vector: vector
  });
}

console.info(new MoveableBuilder().at(1,1).facing(2,1).build());

console.info(new ParticleBuilder().at(1, 1).build());

What I had trouble with is how to reuse the at and facing methods within the ParticleBuilder. Granted, since the signatures of both builders is different, it might not be the correct thing to do. However, I figured I could elicit the community for feedback on this.

Found a solution and I think this is elegant, but I'm unsure if this is something that should be avoided or not since I'm fairly new to ES6.

Here are the following things I did to refactor it:

Unify the Builder signatures to use a single parameter product
Invoke the parent constructor within the child constructor with the same arguments on the instance.
Move the defaults into the build step of the Builder.
Use Object.assign to merge new properties.

There's a bit more that can be done to clean this up even further, but hopefully, this could be useful for someone else. One could repeat step 2 to include multiple Builders as well.

function ParticleBuilder(product) {
  MoveableBuilder.call(this, product);
  this.lifespan = (lifespan) =>
    new this.constructor(Object.assign({}, product, {
      lifespan: lifespan
    }));

  this.decay = (decay) =>
    new this.constructor(Object.assign({}, product, {
      decay: decay
    }));

  this.build = () => Object.assign({
      lifespan: 255,
      decay: 1
    },
    product);
};

function MoveableBuilder(product) {
  this.at = (...coordinates) =>
    new this.constructor(Object.assign({}, product, {
      coordinates: coordinates
    }));

  this.facing = (...vector) =>
    new this.constructor(Object.assign({}, product, {
      vector: vector
    }));

  this.build = () => Object.assign({
    coordinates: [],
    vector: []
  }, product);
}

console.info(new MoveableBuilder().at(1, 1).facing(2, 1).build());

console.info(new ParticleBuilder().at(1, 1).build());


Easy-to-use C++ class for asking current time stamp in milli-, micro- and nanoseconds I have to benchmark my code often, and decided that it is about time to implement an easy API for that:

current_time.h:

#ifndef CURRENT_TIME_H
#define CURRENT_TIME_H

#include <chrono>
#include <cstdint>

class CurrentTime {
    std::chrono::high_resolution_clock m_clock;

public:
    uint64_t milliseconds();
    uint64_t microseconds();
    uint64_t nanoseconds();
};

#endif  /* CURRENT_TIME_H */

current_time.cpp:

#include "current_time.h"

uint64_t CurrentTime::milliseconds()
{
    return std::chrono::duration_cast<std::chrono::milliseconds>
              (m_clock.now().time_since_epoch()).count();
}

uint64_t CurrentTime::microseconds()
{
    return std::chrono::duration_cast<std::chrono::microseconds>
              (m_clock.now().time_since_epoch()).count();
}

uint64_t CurrentTime::nanoseconds()
{
    return std::chrono::duration_cast<std::chrono::nanoseconds>
              (m_clock.now().time_since_epoch()).count();
}

main.cpp:

#include <iostream>
#include "current_time.h"

using namespace std;

int main(int argc, char** argv) {
    CurrentTime current_time;

    uint64_t start1 = current_time.milliseconds();
    uint64_t start2 = current_time.microseconds();
    uint64_t start3 = current_time.nanoseconds();

    for (int i = 0; i < 100000000; ++i) {

    }

    uint64_t end1 = current_time.milliseconds();
    uint64_t end2 = current_time.microseconds();
    uint64_t end3 = current_time.nanoseconds();

    cout << "Milliseconds: "   << (end1 - start1)
         << ", microseconds: " << (end2 - start2)
         << ", nanoseconds: "  << (end3 - start3)
         << ".\n";

    return 0;
}

Any critique is much appreciated, yet I would like to hear your opinions on returning uint64_t: is there some implicit conversions going on?

Since std::chrono::high_resolution_clock::now() is static method you don't need any instance for it.

This is all you need.

#ifndef CURRENT_TIME_H
#define CURRENT_TIME_H

#include <chrono>
#include <cstdint>

inline uint64_t CurrentTime_milliseconds()
{
    return std::chrono::duration_cast<std::chrono::milliseconds>
              (std::chrono::high_resolution_clock::now().time_since_epoch()).count();
}

inline uint64_t CurrentTime_microseconds()
{
    return std::chrono::duration_cast<std::chrono::microseconds>
              (std::chrono::high_resolution_clock::now().time_since_epoch()).count();
}

inline uint64_t CurrentTime_nanoseconds()
{
    return std::chrono::duration_cast<std::chrono::nanoseconds>
              (std::chrono::high_resolution_clock::now().time_since_epoch()).count();
}

#endif  /* CURRENT_TIME_H */


Optimize custom double.parse() In my company's code, they use double.tryParse() which is quite good but sets too much security for our needs. As we sometimes have to parse a few billion strings, I came up with this code which is a little bit faster (10%) but I have the feeling that it's not at its best... Unfortunately, this kind of stuff require more knowledge than I have.

private static double GetDoubleValue(string input)
{
    double n = 0;
    int decimalPosition = input.Length;
    bool separatorFound = false;
    bool negative = input[0] == '-';
    for (int k = (negative ? 1 : 0); k < input.Length; k++)
    {
        char c = input[k];

        if (c == '.' || c == ',')
        {
            if (separatorFound)
                return Double.NaN;
            decimalPosition = k + 1;
            separatorFound = true;
        }
        else
        {
            if (!char.IsDigit(c))
                return Double.NaN;
            n = (n * 10) + (c - '0');
        }
    }
    return ((negative ? -1 : 1) * n) / CustomPow(10, input.Length - decimalPosition);
}

private static double CustomPow(int num, int exp)
{
    double result = 1.0;
    while (exp > 0)
    {
        if (exp%2 == 1)
            result *= num;
        exp >>= 1;
        num *= num;
    }
    return result;
}

Naming

Avoid single-letter or shortened form variable names. Extra characters on a variable's name are free, and work wonders for a maintenance programmer down the line.

e.g.

double n = 0;

would better be:

double output = 0;

Additionally, I'm not sold on the name GetDoubleValue. At no point does this method retrieve the value from anywhere. What this method does is conversion, so it should be appropriately named as such.

Var

Use the var keyword when defining local variables where the right hand side of the definition makes the type obvious. This looks cleaner and saves time when it comes to changing types during refactoring.

e.g.

bool separatorFound = false;

should be

var separatorFound = false;

You should also use var when declaring foreach and for loop iterators.

e.g.

for (int k = (negative ? 1 : 0); k < input.Length; k++)

should be:

for (var k = (negative ? 1 : 0); k < input.Length; k++)

Braces

Always use braces for if statement bodies. It's cleaner, more explicit in intent, and it's quicker should you later decide to add extra lines to the body.

e.g.

if (!char.IsDigit(c))
    return Double.NaN;

should be:

if (!char.IsDigit(c))
{
    return Double.NaN;
}

Design

Why have you created the function CustomPow when all you're really doing is repeatedly multiplying by 10? It seems this is a case of over-abstraction, unless you plan to use the method elsewhere too.

Why make GetDoubleValue private? It sounds like a useful function you may want to use elsewhere or in other projects. I suspect it's private because it is in a class it does not belong in, and you don't want somebody calling StringLoader.GetDoubleValue or something like that.

Put it in a utility class or an extension method somewhere, it doesn't make sense as a method of your string loader class or whatever.