Refactoring two node-accessing methods I cannot edit the QueryPermissions class because it is an external .dll that was given to me to use. Yes, I realize it's a bad practice to use an out variables, but I am pushing for a rewrite of that service.

As you can see below the two methods do the almost the EXACT same thing only one uses a groupkey and the other uses a userkey and then calls the appropriate method. For whatever reason I am drawing a blank on refactoring these methods.

public IEnumerable<Node> GetNodesForUser(int userKey, IEnumerable<OnlineReport> onlineReports)
{
    var nodes = new List<Node>();
    foreach (var onlineReport in onlineReports)
    {
        var parentNode = new Node {Text = onlineReport.Key, Value = onlineReport.Value};
        var webQueries = new List<WebQuery>();
        //Line that is different
        _QueryPermissions.GetQueriesForTaskAndUser(Int32.Parse(onlineReport.Value), userKey, ref webQueries, true);
        foreach (var childNode in webQueries.Select(webQuery => new Node(parentNode){Text = webQuery.QueryTitle, Value = webQuery.QueryKey.ToString()}))
        {
            parentNode.Children.Add(childNode);
        }
        nodes.Add(parentNode);
    }
    return nodes;
}

public IEnumerable<Node> GetNodesForGroup(int groupKey, IEnumerable<OnlineReport> onlineReports)
{
    var nodes = new List<Node>();
    foreach (var onlineReport in onlineReports)
    {
        var parentNode = new Node { Text = onlineReport.Key, Value = onlineReport.Value };
        var webQueries = new List<WebQuery>();
        //Line that is different
         _QueryPermissions.GetQueriesForTaskAndUserGroup(Int32.Parse(onlineReport.Value), groupKey, ref webQueries, true);
        foreach (var childNode in webQueries.Select(webQuery => new Node(parentNode) { Text = webQuery.QueryTitle, Value = webQuery.QueryKey.ToString() }))
        {
            parentNode.Children.Add(childNode);
        }
        nodes.Add(parentNode);
    }
    return nodes;
}

The big difference between the functions is not the userKey vs. groupKey but rather than they have different queries embedded in them

_QueryPermissions.GetQueriesForTaskAndUser(Int32.Parse(onlineReport.Value), userKey, ref webQueries, true);

vs.

_QueryPermissions.GetQueriesForTaskAndUserGroup(Int32.Parse(onlineReport.Value), groupKey, ref webQueries, true);

It looks like these can be abstracted as

 delegate void NodeQueryDelegate(int reportValue, int key, ref List<WebQuery> webQueries, bool someBool);

so it should be possible to refactor thus

public IEnumerable<Node> GetNodesFromQuery(int key, IEnumerable<OnlineReport> onlineReports, NodeQueryDelegate query) {
    var nodes = new List<Node>();
    foreach (var onlineReport in onlineReports)  {
        var parentNode = new Node { Text = onlineReport.Key, Value = onlineReport.Value };
        var webQueries = new List<WebQuery>();
        //Line that is different
        query(Int32.Parse(onlineReport.Value), key, ref webQueries, true);
        foreach (var childNode in webQueries.Select(webQuery => new Node(parentNode) { Text = webQuery.QueryTitle, Value = webQuery.QueryKey.ToString() })) {
            parentNode.Children.Add(childNode);
        }
        nodes.Add(parentNode);
    }
    return nodes;
}

Getting the user nodes becomes

var userNodes = GetNodesFromQuery(userKey, onlineReports, _QueryPermissions.GetQueriesForTaskAndUser);

Getting the group node

var groupNodes = GetNodesFromQuery(groupKey, onlineReports, _QueryPermissions.GetQueriesForTaskAndUserGroup);


Data formatting using string spanning I would like to know how to make my code better. I'm still a newbie in Android, so any tips are appreciated.

The code is an AsyncTask class extender that formats data into a nice looking Spannable String. It works, but it's quite slow, even if ran on a background thread.

//SlideContentType is an enumerable with 4 options: Movie, Action, Game, Music
//SlideContent is a class that stores info about a specific movie/song/game etc...
public class PickCardAsyncTask extends AsyncTask<Pair<SlideContent.SlideContentType, List<SlideContent>>, Integer, Pair<SpannableStringBuilder, String>> {

    @Override
    protected Pair<SpannableStringBuilder, String> doInBackground(Pair<SlideContent.SlideContentType, List<SlideContent>>... pairs){

        //Get a reference to the list of all Slide Contents containing movies/games/etc.
        List<SlideContent> list = (List<SlideContent>) pairs[0].second;

        //Select a random item in the list
        Random random = new Random();
        random.setSeed(System.nanoTime());
        int randomNr = random.nextInt(list.size());
        SlideContent currentContent = list.get(randomNr);

        //The link and the content are released as a pair
        String websiteLink = currentContent.movieWebsite;

        //End result variable
        SpannableStringBuilder coloredString = new SpannableStringBuilder();

        //Do the appropriate actions
        if(pairs[0].first == SlideContent.SlideContentType.Movie){

            //Format the information from the Slide Content into a nice looking Spannable String
            SpannableString text = new SpannableString(currentContent.movieTitle);
            text.setSpan(new StyleSpan(Typeface.BOLD), 0, text.length(), Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
            coloredString.append(text);

            coloredString.append(" (" + currentContent.movieYear + ")\n");

            text = new SpannableString(currentContent.movieGenres);
            text.setSpan(new RelativeSizeSpan(0.7f), 0, text.length(), Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
            text.setSpan(new ForegroundColorSpan(Color.GRAY), 0, text.length(), Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
            coloredString.append(text);
            coloredString.append("\n");

            text = new SpannableString(currentContent.movieDescription);
            text.setSpan(new RelativeSizeSpan(0.8f), 0, text.length(), Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
            coloredString.append(text);
            coloredString.append("\n");

            text = new SpannableString("Movie rated " + currentContent.movieRating + " out of 10.");
            text.setSpan(new RelativeSizeSpan(0.7f), 0, text.length(), Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
            text.setSpan(new ForegroundColorSpan(Color.GRAY), 0, text.length(), Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
            coloredString.append(text);

        }
        //Return a pair of the Spannable String Builder and the link to the related website
        return new Pair<>(coloredString, websiteLink);
    }
}

Random random = new Random();
random.setSeed(System.nanoTime());

Don't put this inside here. I'm assuming that this method will end up being called more than once. If so, you don't want to be seeding and re-seeding the Random every call.

random should be moved to a field of the class. Also, I don't think you need to manually call Random.setSeed. See here for more details.

I don't know my Java 100%, but I think this might be a typo:

Pair<SlideContent.SlideContentType, List<SlideContent>>... pairs

I think it should be:

Pair<SlideContent.SlideContentType>, List<SlideContent>... pairs


Employee Compensations Query I have a pretty straightforward query that I am essentially hard-coding. I have a table with 3 keys and a description field.

The first key has to do with the type of compensation for an employee (Award, Hourly, NonHourly, Staff, Other, and Commission).

The next key has to do with Payment Frequency. O refers to One-Time Payment, M to Multiple Payments.

The third key has to do with the Compensation Rate Code. It's like another frequency. P means Per Project/Assignment, H means Hourly, and M means Monthly.

Certain Employee Compensations types are restricted to different types of compensation. In the below query, I've hard-coded every possibility. Is there a better way to write this query than just hard-coding every combination and unioning them together?

SELECT 'Award'
 , 'O'
 , 'P'
 , 'Per Project or Assignment'
  FROM DUAL
  UNION
 SELECT 'Hourly'
 , 'M'
 , 'H'
 , 'Per Hour'
  FROM DUAL
  UNION
 SELECT 'Hourly'
 , 'M'
 , 'P'
 , 'Per Project or Assignment'
  FROM DUAL
  UNION
 SELECT 'Hourly'
 , 'M'
 , 'M'
 , 'Per Month'
  FROM DUAL
  UNION
 SELECT 'Hourly'
 , 'O'
 , 'H'
 , 'Per Hour'
  FROM DUAL
  UNION
 SELECT 'Hourly'
 , 'O'
 , 'P'
 , 'Per Project/Assignment or Month'
  FROM DUAL
  UNION
 SELECT 'NonHourly'
 , 'M'
 , 'P'
 , 'Per Project or Assignment'
  FROM DUAL
  UNION
 SELECT 'NonHourly'
 , 'M'
 , 'M'
 , 'Per Month'
  FROM DUAL
  UNION
 SELECT 'NonHourly'
 , 'O'
 , 'P'
 , 'Per Project/Assignment or Month'
  FROM DUAL
  UNION
 SELECT 'Staff'
 , 'M'
 , 'H'
 , 'Per Hour'
  FROM DUAL
  UNION
 SELECT 'Staff'
 , 'O'
 , 'H'
 , 'Per Hour'
  FROM DUAL
  UNION
 SELECT 'Other'
 , 'M'
 , 'H'
 , 'Per Hour'
  FROM DUAL
  UNION
 SELECT 'Other'
 , 'M'
 , 'P'
 , 'Per Project or Assignment'
  FROM DUAL
  UNION
 SELECT 'Other'
 , 'M'
 , 'M'
 , 'Per Month'
  FROM DUAL
  UNION
 SELECT 'Other'
 , 'O'
 , 'H'
 , 'Per Hour'
  FROM DUAL
  UNION
 SELECT 'Other'
 , 'O'
 , 'P'
 , 'Per Project or Assignment'
  FROM DUAL
  UNION
 SELECT 'Other'
 , 'O'
 , 'M'
 , 'Per Month'
  FROM DUAL
  UNION
 SELECT 'Commission'
 , 'O'
 , 'P'
 , 'Commission/Tips'
  FROM DUAL

Note that DUAL is just a dummy table. I'm not actually selecting anything from any tables.

EDIT:

This is the SQL behind a View. I wouldn't be opposed to creating new views to "extract" out pieces of the query if that could be used to simplify it. It creates a view that has the following rows in it:

Essentially, my query takes the results of 18 SQL statements and unions them together to get 18 rows in the newly created view. Surely, there's a better way to do it?

The second and third keys are represented in a table that maps their character values to a description (e.g. O -> One-Time Payment). The first key is only used for this view.

This really should be put into a table, if possible.

If such a table already exists, you can take the SELECT for your existing view and simply wrap an INSERT INTO TargetTable statement around it.

INSERT INTO TargetTable
SELECT key1,key2,key3,key4 FROM YourView;
COMMIT;

Otherwise, you can CREATE TABLE TargetTable AS to generate a new table from your return query.

CREATE TABLE TargetTable AS
SELECT key1,key2,key3,key4 FROM YourView;
COMMIT;


VBA - faster way to list out files in a folder & subfolders I have written a VBA code in excel (basically through combining various codes from others -_-") to list out the path, file name, file size & extension of all files under a folder and the sub-folders there-under.

Is it possible to improve the efficiency for collecting the same info?

Sub MainList()
    Set folder = Application.FileDialog(msoFileDialogFolderPicker)
    If folder.Show <> -1 Then Exit Sub
    xDir = folder.SelectedItems(1)
    Cells(3, 1) = Now()
    Call ListFilesInFolder(xDir, True)
    Cells(5, 1) = Now()
End Sub

Sub ListFilesInFolder(ByVal xFolderName As String, ByVal xIsSubfolders As Boolean)
    Dim xFileSystemObject As Object
    Dim xFolder As Object
    Dim xSubFolder As Object
    Dim xFile As Object
    Dim rowIndex As Long
    Dim folder_index As Integer
    Dim file_extension As String
    Dim file_type As String
    Set xFileSystemObject = CreateObject("Scripting.FileSystemObject")
    Set xFolder = xFileSystemObject.GetFolder(xFolderName)
    folder_index = Range("B65536").End(xlUp).row + 1
    rowIndex = Range("F65536").End(xlUp).row + 1
    Cells(folder_index, 2).Select
    ActiveSheet.Hyperlinks.Add Anchor:=Selection, Address:=xFolder.Path, TextToDisplay:=xFolder.Path

    For Each xFile In xFolder.Files
        file_extension = LCase(xFileSystemObject.GetExtensionName(xFile.Path))
        If file_extension = "pdf" Then
            file_type = "PDF"
        ElseIf Left(file_extension, 3) = "doc" Then
            file_type = "DOC"
        ElseIf Left(file_extension, 2) = "xl" Then
            file_type = "XLS"
        ElseIf Left(file_extension, 3) = "msg" Then
            file_type = "MSG"
        ElseIf Left(file_extension, 3) = "zip" Then
            file_type = "ZIP"
        ElseIf Left(file_extension, 3) = "ppt" Then
            file_type = "PPT"
        Else
            file_type = ""
        End If
        Cells(rowIndex, 6).Select
        ActiveSheet.Hyperlinks.Add Anchor:=Selection, Address:=xFolder.Path, TextToDisplay:=xFolder.Path
        Cells(rowIndex, 7).Select
        ActiveSheet.Hyperlinks.Add Anchor:=Selection, Address:=xFile.Path, TextToDisplay:=xFile.Name
        Cells(rowIndex, 8).Formula = file_type
        Cells(rowIndex, 9).Formula = xFile.Size
        Cells(rowIndex, 10).Formula = xFile.DateLastModified
        Cells(rowIndex, 11).Formula = file_extension
        rowIndex = rowIndex + 1
    Next xFile

    If xIsSubfolders Then
        For Each xSubFolder In xFolder.SubFolders
            ListFilesInFolder xSubFolder.Path, True
        Next xSubFolder
    End If

    Set xFile = Nothing
    Set xFolder = Nothing
    Set xFileSystemObject = Nothing

End Sub

Variables
Always turn on Option Explicit. You can have it automatically by going to Tools -> Options in the VBE and checking the Require Variable Declaration option. This way if you have any variables not defined, the compiler will let you know.
When you don't define your variable, VBA will declare it as a Variant, which are objects:
Performance. A variable you declare with the Object type is flexible enough to contain a reference to any object. However, when you invoke a method or property on such a variable, you always incur late binding (at run time). To force early binding (at compile time) and better performance, declare the variable with a specific class name, or cast it to the specific data type.
By not declaring variables, you could possibly be paying a penalty.
You didn't define folder or xDir in MainList.
Your variables in ListFilesInFolder are not really adhering to Standard VBA naming conventions. Why are these prefaced by x? There doesn't seem to be a reason. Also, using the underscore in a variable should be avoided unless it's a constant.

Dim folder_index As Integer

Integers - integers are obsolete. According to msdn VBA silently converts all integers to long.
Referencing
Every time you use Cells or Range without an explicit reference, it is implicitly referencing the active sheet. You want Sheet1.Range etc
Be sure to avoid things like .Select - it just slows the code down by needing to fiddle with the spreadsheet while doing everything else behind the scenes. There's a good question on StackOverflow addressing this.
Take this for instance -

Cells(rowIndex, 6).Select
    ActiveSheet.Hyperlinks.Add Anchor:=Selection, Address:=xFolder.Path,
TextToDisplay:=xFolder.Path

The same thing could just be written directly -
 ActiveSheet.Cells(rowIndex, 6).Hyperlinks.Add Address:=xFolder.Path, TextToDisplay:=xFolder.Path

    Cells(rowIndex, 8).Formula = file_type
    Cells(rowIndex, 9).Formula = xFile.Size
    Cells(rowIndex, 10).Formula = xFile.DateLastModified
    Cells(rowIndex, 11).Formula = file_extension

Why are you setting these values via a formula? You want to use the Cells().Value property when assigning a value.
What are you doing here -

rowIndex = Range("F65536").End(xlUp).Row + 1

Are you just getting the last row? There is a standard way to find lastRow and lastColumn. That post explains why.

Your If structure here -

    If file_extension = "pdf" Then
        file_type = "PDF"
    ElseIf Left(file_extension, 3) = "doc" Then
        file_type = "DOC"
    ElseIf Left(file_extension, 2) = "xl" Then
        file_type = "XLS"
    ElseIf Left(file_extension, 3) = "msg" Then
        file_type = "MSG"
    ElseIf Left(file_extension, 3) = "zip" Then
        file_type = "ZIP"
    ElseIf Left(file_extension, 3) = "ppt" Then
        file_type = "PPT"
    Else
        file_type = ""
    End If

Why are you doing this? First, if you need to only know those files, then you would use your Else value to filter out files that aren't what you need and then not print them. In which case a Select Case would work. But, in general, this seems unnecessary when you could just use something like
Dim fileName As String
fileName = Dir("C:\Temp" & "\*")
Do While Len(fileName) > 0
    'here fileName = filename.filetype
    fileName = Dir
Loop

You can avoid that entire if structure by just parsing your file name
    baseName = Left(fileName, Len(fileName)-4)
    extention = Right(fileName, 3)

Though in general you'd be better off parsing it like
    Dim delimiterPosition As Long
    delimiterPosition = InStr(1, fileName, ".")

But your call.

Another thing that's slow is writing to the sheet so many times. Instead, gather your data into an array, populate the array, and then write it all to the sheet. Since you're creating hyperlinks, you'd store that data in your array as well.


Most Efficient Solution for USACO: Triangles - Python I'm trying to solve this problem with Python 3.8. In my code, I used 3 nested for loops to check every single point and storing the largest area with each set of points. This program works fine, but it's \$ O(n^3) \$ time complexity, and I'm wondering if there are any more elegant/efficient solutions. Is there a more efficient algorithm that doesn't loop through every single point, or is that necessary?
My code:
with open("triangles.in", "r") as file_in:
  lines = [x.strip() for x in file_in.readlines()]
  n, points = lines[0], lines[1:]

def main(points):
  largest = 0

  for corner in points:
    cx, cy = corner.split()
    for leg in points:
      lx, ly = leg.split()
      for width in points:
        wx, wy = width.split()
        if lx == cx and wy == cy:
          area = abs(int(ly)-int(cy)) * abs(int(wx)-int(cx))

          if area > largest:
            largest = area

  return str(largest)

with open("triangles.out", "w+") as file_out:
  file_out.write(main(points))
  file_out.close()

The input file triangles.in:
4
0 0
0 1
1 0
1 2

Problem synopsis:
Given a set of \$ n \$ distinct points \$ (X_1, Y_1) \$ to \$ (X_n, Y_n) \$, find the largest triangle's area multiplied by 2, given that the triangle is a right triangle (one of the lines of the triangle in parallel to the x-axis, and one other parallel to the y-axis).

An obvious improvement is to not split the strings and convert the parts to int over and over again. Do it once, at the start:
def main(points):
  points = [tuple(map(int, point.split())) for point in points]
  largest = 0

  for cx, cy in points:
    for lx, ly in points:
      for wx, wy in points:
        if lx == cx and wy == cy:
          area = abs(ly-cy) * abs(wx-cx)
          if area > largest:
            largest = area

  return str(largest)

And it can be solved in O(n). For every "corner" as you call it, you go through all point pairs. Instead, just look up the point furthest away on the same y-coordinate and the point furthest away on the same x-coordinate. That can be precomputed in O(n):
with open('triangles.in') as f:
    next(f)
    points = [tuple(map(int, line.split())) for line in f]

xmin, xmax, ymin, ymax = {}, {}, {}, {}
for x, y in points:
    xmin[y] = min(xmin.get(y, x), x)
    xmax[y] = max(xmax.get(y, x), x)
    ymin[x] = min(ymin.get(x, y), y)
    ymax[x] = max(ymax.get(x, y), y)

result = max(max(x - xmin[y], xmax[y] - x) * max(y - ymin[x], ymax[x] - y)
             for x, y in points)

with open('triangles.out', 'w') as f:
    print(result, file=f)

Note that I also did the output a bit differently. No need to close yourself. Getting the file closed for you kinda is the reason you used with in the first place, remember? And I prefer print over write, as I don't have to convert to string then and trust line endings to be done as appropriate for the platform (maybe not an issue here, as the output is just one line and apparently they don't care how it ends).
P.S. Those darn... they kept saying my solution failed due to "Runtime error or memory limit exceeded" and it took me a while to figure out: Instead of tuple(map(...)) I had used my preferred [*map(...)]. But they're inexplicably using Python 3.4 and it didn't exist back then. But that should be a syntax error. Grrrr....


Sudoku solution without the use of classes A Sudoku puzzle is a 9 × 9 grid with some numbers between 1 and 9 in
  it, that has to be filled in so that

  every row,
  every column, and
  each of the nine 3 × 3 blocks

  contain all the numbers from 1 to 9.

  For example the following matrix is not a valid solution of a Sudoku
  puzzle: the fourth column contains two 7's, block 4 contains also two
  7's, and block 5 contains two 9's (there are more reasons why this
  matrix is not a valid solution but we will not list all them):

1 8 2 | 3 5 9 | 4 7 6
7 5 3 | 2 6 4 | 9 8 1
6 4 9 | 7 1 8 | 2 5 3
------+-------+------
2 6 4 | 9 7 3 | 8 1 5
9 1 7 | 4 8 5 | 9 3 2
8 3 5 | 7 2 1 | 7 4 9
------+-------+------
5 9 6 | 1 4 7 | 3 2 8
4 2 8 | 5 3 6 | 1 9 7
3 7 1 | 8 9 2 | 5 6 4

  Write a program to determine if a given 9 × 9 matrix is the solution
  of a Sudoku puzzle.

  In the example above we have marked the limits of the 9 blocks to make
  the example easier to follow, but the delimiter characters do not
  appear in the input.

Important: the only structures I can use are structs and the defined in the vector library. I cannot use classes (as I have not studied them yet).

#include <iostream>
#include <vector>
//I know I should not include std
//but I am restricted to do so.
using namespace std;

const int sdk_size = 9;
const int sdk_blocks = 3;

typedef vector<int> permutation;
typedef vector<permutation> sdk;

sdk read_sdk()
{
  sdk v(sdk_size, permutation(sdk_size));
  for (int y = 0; y < sdk_size; ++y) {
      for (int x = 0; x < sdk_size; ++x) {
        cin >> v[y][x];
      }
  }
  return v;
}

//Checks if a permutation *is* a permutation, i.e.
//it contains every number in 1..sdk_size once and only once.
bool is_permutation(const permutation& per)
{
  vector<bool> used(sdk_size + 1, false);
  for (int i = 0; i < sdk_size; ++i) {
    if (per[i] < 1 or per[i] > sdk_size or used[per[i]]) {
      return false;
    }
    used[per[i]] = true;
  }
  return true;
}

bool has_valid_rows(const sdk& v)
{
  for (int y = 0; y < sdk_size; ++y) {
    if (not is_permutation(v[y])) return false;
  }
  return true;
}

bool has_valid_cols(const sdk& v)
{
  for (int x = 0; x < sdk_size; ++x) {
    permutation col(sdk_size);
    for (int y = 0; y < sdk_size; ++y) {
      col[y] = v[y][x];
    }
    if (not is_permutation(col)) return false;
  }
  return true;
}

bool has_valid_blocks(const sdk& v)
{
  //A block is a submatrice of v with start point (y0,x0)
  //and height and width of sdk_size.
  for (int y_0 = 0; y_0 < sdk_size; y_0 += sdk_blocks) {
    for (int x_0 = 0; x_0 < sdk_size; x_0 += sdk_blocks) {
      permutation block;
      for (int dy = 0; dy < sdk_blocks; ++dy) {
        for (int dx = 0; dx < sdk_blocks; ++dx) {
          block.push_back(v[y_0+dy][x_0+dx]);
        }
      }
      if (not is_permutation(block)) return false;
    }
  }
  return true;
}

bool is_valid_sdk(const sdk& v)
{
  if (not has_valid_rows(v))   return false;
  if (not has_valid_cols(v))   return false;
  if (not has_valid_blocks(v)) return false;
  return true;
}

int main()
{
  sdk v = read_sdk();
  if (is_valid_sdk(v)) {
    cout << "The sudoku is well-formed" << endl;
  } else {
    cout << "The sudoku is not well-formed" << endl;
  }
}

I see some things that may help you improve your code.

Don't abuse using namespace std

Putting using namespace std within your program is generally a bad habit that you'd do well to avoid.

The comment in the code seems to indicate you know this and can't change it for some reason, but others who may read this question and answer may benefit

Simplify your code

The current code has this function:

bool is_valid_sdk(const sdk& v)
{
  if (not has_valid_rows(v))   return false;
  if (not has_valid_cols(v))   return false;
  if (not has_valid_blocks(v)) return false;
  return true;
}

However, this could be considerably shorter and easier to read and understand if written like this:

bool is_valid_sdk(const sdk& v)
{
  return has_valid_rows(v) and has_valid_cols(v) and has_valid_blocks(v);
}

Reconsider the design

Although the current code works, it's not very efficient in that it makes a copy of each vector in order to check it.  An alternative approach would be to allow checking in place without making a copy.  One way to do that would be to separate the iteration through the rows/columns/blocks into a separate function that could be passed to the is_permutation function.  For example:

bool is_permutation(const sdk& v, int x, int y,
                    int(*next)(const sdk& v, int& x, int& y))
{
  vector<bool> used(sdk_size, false);
  for (int i = 0; i < sdk_size; ++i) {
    int val = next(v, x, y);
    if (val < 1 or val > sdk_size or used[val-1]) {
      return false;
    }
    used[val-1] = true;
  }
  return true;
}

int next_in_blk(const sdk& v, int &x, int &y)
{
    int val = v[x][y];
    if (++y % sdk_blocks == 0) {
        y -= sdk_blocks;
        ++x;
    }
    return val;
}

bool has_valid_blocks(const sdk& v)
{
  for (int i = 0; i < sdk_size; ++i) {
    int x = sdk_blocks * (i / sdk_blocks);
    int y = sdk_blocks * (i % sdk_blocks);
    if (not is_permutation(v, x, y, next_in_blk)) return false;
  }
  return true;
}

The corresponding functions for row and col are even simpler.


Multithreaded search for solutions to an inequality This question is related to my previous question on the brute-force search
for a solution to an unsolved mathematical inequality:

$$3^{k}-2^{k}\left\lfloor \left({\tfrac {3}{2}}\right)^{k}\right\rfloor >2^{k}-\left\lfloor \left({\tfrac {3}{2}}\right)^{k}\right\rfloor -2$$

The following code, tested on Ubuntu 16.04, compiles with no warnings or errors using the flags -Wall -std=c++14 -pthread -Ofast using the g++ compiler. I'm using -Ofast because I am trying to squeeze every ounce of performance out of this code (more info here).

I am not 100% sure if this is the proper, fastest, implementation of multithreading. Since I'm new to multithreading, I used this SO question, this page, and this page for learning how to multithread.

For questions about other parts of the code, I explain it way more on my previous question (like why I have a completely separate if-else check).

Let me know how I can improve my use of multithreading in this program.

#include <boost/multiprecision/cpp_dec_float.hpp>
//#include <cmath> (already included from cpp_dec_float.hpp)
#include <iostream>
#include <future>

typedef boost::multiprecision::number<boost::multiprecision::cpp_dec_float<2000>> arbFloat;

enum returnID {success = 0, precisionExceeded = 1};

arbFloat calcThreeK(const arbFloat & k){
    return pow(3, k);
}

arbFloat calcTwoK(const arbFloat & k){
    return pow(2, k);
}

int main(){
    arbFloat k, threeK, twoK;
    bool isSolution = false;

    for(k = 6; !isSolution; ++k){
        std::future<arbFloat> futureThreeK = std::async(std::launch::async, calcThreeK, k);
        std::future<arbFloat> futureTwoK = std::async(std::launch::async, calcTwoK, k);

        threeK = futureThreeK.get();
        twoK = futureTwoK.get();

        isSolution = threeK - twoK * floor(threeK / twoK) > twoK - floor(threeK / twoK) - 2;
    }

    if(pow(3, k) - (pow(2, k) * floor(pow(1.5, k))) <= pow(2, k) - floor(pow(1.5, k)) - 2){
        std::cout << "Solution at k = " << k << ".\n";
        return returnID::success;
    } else {
        std::cout << "Error: Precision exceeded at k = " << k << ".\n";
        return returnID::precisionExceeded;
    }
}

You are asynchronously calculating intermediate results that are not worth parallelizing. Instead of calculating \$2^k\$ and \$3^k\$ sequentially, you do those calculations in parallel. But then you have to wait for both of them to complete before proceeding. I would expect that the overhead of setting up the async calls would negate any performance benefit.

Suppose you had a dozen friends helping you. Would you ask one of them to go off and calculate \$2^6\$, the second friend to calculate \$3^6\$, and report the results back to you so that you can check whether 6 is a solution? No, that would be crazy. You would be better off asking the first friend to check whether 6 is a solution, the second friend to check 7, the third friend to try 8, etc.  Then each of them has a chance to do some substantial work independently instead of waiting for each other's results.

Moreover, calculating \$3^k\$ would be a trivial multiplication if you already knew \$3^{k-1}\$. You should be able to avoid calling pow() altogether if you are trying increasing \$k\$.


Single Linked List Implementation in Ruby The #traverse_until method is a helper function that traverses the linked list and returns the node and index of the node once a condition (in a block) is met or until the end of the list. I would greatly appreciate feedback about verbosity, efficiency, readability, etc.

module DataStructures
  class Node
    attr_accessor :value, :next_node

    def initialize(value = nil, next_node = nil)
      @value = value
      @next_node = next_node
    end
  end

  class LinkedList
    attr_accessor :head

    def initialize(element = nil)
      element.nil? ? @head = nil : @head = Node.new(element)
    end

    def append(data)
      prepend(data) if @head.nil?
      current = @head
      until current.next_node.nil?
        current = current.next_node
      end
      current.next_node = Node.new(data)
    end

    def prepend(data)
      @head = Node.new(data, @head)
    end

    def find(data)
      result = traverse_until { |node, index| node.value == data }
      result[:node].value == data ? result[:index] : "Data not found."
    end

    def contains? (data)
      result = traverse_until { |node, index| node.value == data }
      result[:node].value == data
    end

    def node_at_index(index)
      result = traverse_until { |node, i| i == index }
      "Node at index #{index}: #{result[:node].value}"
    end

    def head
      @head.nil? ? "Empty list" : @head.value
    end

    def tail
      last_node = traverse_until { |node, index| node.next_node.nil? }
      last_node[:node].value
    end

    def size
      last_node = traverse_until { |node, index| node.next_node.nil? }
      "List Size: #{last_node[:index] + 1}"
    end

    def insert_at(data, index)
      current = @head

      (index - 1).times do |n|
        current = current.next_node
      end

      node_shifted_right = current.next_node
      current.next_node = Node.new(data, node_shifted_right)
    end

    def remove_at(index)
      node_after_index = @head
      previous = node_after_index
      (index + 1).times do |n|
        node_after_index = node_after_index.next_node
      end

      (index - 1).times do |n|
        previous = previous.next_node
      end

      previous.next_node = node_after_index
    end

    def pop
      node_before = nil
      current = @head
      until current.next_node.nil?
        node_before = current
        current = current.next_node
      end
      node_before.next_node = nil
    end

    def to_s
      result = ""
      current = @head
      loop do
        result += "(#{current.value}) -> "
        break if current.next_node.nil?
        current = current.next_node
      end
      result += "nil"
    end

    private

    def traverse_until
      current = @head
      index = 0
      until current.next_node.nil? || yield(current, index)
        current = current.next_node
        index += 1
      end
      {node: current, index: index}
    end
  end
end

Firstly, your code's broken. If you instantiate a list with no initial value, and then append to it, it creates two nodes:

list = LinkedList.new
list.append("foo")
list.size #=> "List size: 2"

Secondly, implement an #each method to iterate through values and include Enumerable. That'll give a ton of methods for free: #find, #count, #to_a, and many others. Something like this should do:

def each
  node = @head
  until node.nil?
    yield node.value
    node = node.next_node
  end
end

Thirdly, return useful values. #size should not return a string. Neither should #head, #node_at_index, or #find. #size should return an int, and the others should return nil. Never ever return easy-to-print strings unless that's explicitly called for. Currently, you can get puzzling stuff like this:

some_list.head #=> "Empty list" (the head's value is the string "Empty list")
some_list.size #=> "List size: 2" (so, no, not empty)

And you can get useless stuff like this:

if some_array.size < some_list.size # ArgumentError: comparison of Fixnum with String failed
  ...

I trust you see the problem here.

Speaking of return values, you seem to have taken some care that your methods don't return a raw Node instance (which is smart, since nodes are internal/private to the linked-list)... but you made @head accessible from anywhere with attr_accessor. So some_list.head = "foo" is perfectly legal, and will break things. Also, #append and #prepend implicitly return Node instances.

Fourth, you might want to keep track of the tail as well as the head. Many of your methods rely on finding the tail one way of another, so why not keep it around? Or at least use your own #tail method for these cases, rather than do it differently each time.

Other stuff:

Check the indices passed to #insert_at and #remove_at and raise an appropriate error if the index is out-of-bounds. Otherwise you'll just get a NoMethodError which doesn't explain much.
Don't put assignments into ternary branches; use the branch to determine the value to assign. I.e. this

element.nil? ? @head = nil : @head = Node.new(element)

should be

@head = element.nil? ? nil : Node.new(element)

of course, in this particular case, @head will be nil by default, do you could just as well do:

@head = Node.new(element) unless element.nil?

Your #to_s would be a lot cleaner if you collected all the values in an array, used map to add parentheses, and join to add the "->" arrows. If you include Enumerable as described above, your #to_s could just be:

def to_s
  values = map { |value| "(#{value})" }
  values << "nil"
  values.join(" -> ")
end


Parse backslash-delimited hierarchy path into separate columns I have rows in a database table with a hierarchypath column. The values in the column are hierarchy paths with up to four levels:
HIERARCHYPATH
---------------------------------------------------
FACILITIES \ FIREHALL \ PLUMBING
FACILITIES \ PARK
ROADS \ GRASS/TURF BLVD MAINTENANCE
ROADS \ SIDEWALKS \ REPLACEMENT
FACILITIES \ PARKING - MAIN ST
RECREATION \ BANDSHELL \ PROPERTY \ BUILDING-GENERAL
FACILITIES

I've written a Jython 2.7 automation script that parses the levels at the \ delimiter and inserts the values into individual level columns:

CLASSL1
CLASSL2
CLASSL3
CLASSL4

#Example:
s = "FACILITIES \ FIREHALL \ PLUMBING"

col_prefix = "CLASSL"

#Note: The term "mbo" is an IBM Maximo concept.
#It can be thought of as the currently selected record in the application.

for i in range(0, 4):
    try:
        mbo.setValue(col_prefix + str(i+1), s.split(' \\ ')[i])
    except:
        #Null-out any existing values that were not overriden
        mbo.setValueNull(col_prefix + str(i+1))

HIERARCHYPATH                       CLASSL1       CLASSL2       CLASSL3      CLASSL4
--------------------------------    ----------    ----------    ----------   -----------
FACILITIES \ FIREHALL \ PLUMBING    FACILITIES    FIREHALL      PLUMBING     null

I'm relatively new to coding. How can the script be improved?

Normalization
A fully-normalized schema would not have four path component columns in a table; it would have one table with a string column for the path component, and a foreign key to its parent (and/or child), conceptually similar to a linked list. Advantages to this approach would be support for an arbitrarily long path. Disadvantage is likely decreased performance, though the scale of your problem is not clear.
Otherwise, I find it odd that you're parsing a path, and storing the result of the parse back into the database alongside the original data. This is a fast-enough operation that surely any advantage to caching parse results is outweighed by the disadvantage of increased storage and fetch time.
The only solid justification I can think of for such a thing is when (1) you know that certain interesting strings exist in fixed locations in the path, and (2) you need very fast, indexed querying on them, and (3) such querying is too slow when doing the usual pattern-matching approach on the original path.
Iteration
Don't use range for your loop; use enumerate. Also use a raw string due to your backslash.
for i, part in enumerate(s.split(r' \ '), 1):

Bare except
Generally this is a bad idea. At the least, catch Exception instead, or more preferably the actual exception type you expect to see. If it's only an index error because you're trying to index into a split that has fewer than four components, just avoid that situation altogether:
parts = s.split(r' \ ')
for i, part in enumerate(parts, 1):
    mbo.setValue(col_prefix + str(i), part)
for i in range(len(parts)+1, 5):
    mbo.setValueNull(col_prefix + str(i))


Glob-Like Search in VBA The Dir() function in VB and VBA can accept wildcard characters (* and ?) but only in the basename of the path.

Dir("path\*.txt")

is a valid request but.

Dir("path\*\*.txt")

will return Error 52 "Bad File Name or Number", because  * is an invalid folder or file name.  I created the ability to use wildcards in the folder names using my
os
module and my
List
 class.

Example Call

It's not a true glob search as you can't search like "*.{cpp,hpp}", but this is far better than nothing.  Example:

Debug.Print Glob("C:\users\ptwales", "????-herp\*\*.txt").ToString
' ["C:\users\ptwales\derp-herp\flerp\merp.txt", ...]

Client code must specify a starting directory.  Current working directory is not implied.

Implementation

Note that os.SubItems and os.SubFolders each now return a List.

Public Function Glob(ByVal root As String, ByVal pattern As String) As List

    Dim pat_list As New List
    pat_list.Extend Split(pattern, os.SEP)

    Set Glob = GlobRecurse(root, pat_list, 1)

End Function
Private Function GlobRecurse(ByVal root As String, ByVal pat_list As List, ByVal index As Integer) As List

    If index = pat_list.Count Then
        Set GlobRecurse = os.SubItems(root, pat_list(index))
    Else

        Set GlobRecurse = New List

        Dim folder As Variant
        For Each folder In os.SubFolders(root, pat_list(index))
            GlobRecurse.Extend GlobRecurse(folder, pat_list, index + 1)
        Next folder

    End If

End Function

It's a bit awkward, particularly that GlobRecurse checks the pat_list.Count each iteration.  It feels too much like scheme and I should be using a for loop instead.

Naming, naming, naming.

I've never heard of a "glob search", but I like the idea of implementing it in vba. One thing I know, is that I don't expect vba to read like python, so os.SEP would be clearer as os.PathSeparator, or even better, as path.Separator. Now I do have a tendency to go by .net naming standards, but really, anything other than a YELLCASE SEP would be cleaner and clearer.

Even the python community seems to agree that using os.path.sep over os.sep makes for clearer code - I'd use os.path.sep to make it very clear that it's the path separator and I recommend you use os.path.sep for clarity, since it's a path separator, not an OS separator. So I'd go with path.Separator, or path.Sep to maintain Pythonish naming.

  Looking at your linked question, I see @RubberDuck already recommended using better names for your constants; sorry to reiterate the same advice here.

Underscores

I know python loves snake_case, but in vba that kind of casing is potentially confusing (for class methods, more specifically) - I'd recommend sticking to language's PascalCase naming convention, and using a consistent camelCase for locals and parameters, for readability's sake (the actual VB/VBA conventions would go PascalCase all the way, but I find that maddening). I find pat_list would really benefit from simply being called patterns, or more accurately, patternParts - the _list suffix feels Hungarian, and seems to reflect the type, which could just as well be a regular Collection - I like that you're using your own List type though.

Parameters

The documentation I've found actually calls its sole parameter pathname:

  glob.glob(pathname)

  Return a possibly-empty list of path names that match pathname, which must be a string containing a path specification. pathname can be either absolute (like /usr/src/Python-1.5/Makefile) or relative (like ../../Tools/*/*.gif), and can contain shell-style wildcards.

You haven't shown us how the client code might use this function, and it's.. pretty hard to guess just by looking at this post, without diving into the os implementation - I'd have to jump to the definition of os.SubItems to figure out why I need to pass a root parameter on top of a pattern.

Looking at your added example call, I think it could be worth combining the two parameters into a single pathName parameter, making the function callable like this:

?Glob("C:\users\ptwales\????-herp\*\*.txt").ToString

The difficulty would be to parse the string and differenciate the root from the pattern - splitting on path separator and using anything to the left of a part that contains any wildcard as the root, and everything else as the pattern, would certainly be possible, and would make it a better API IMO; the root and pattern parts are really two parts of the same string, that you've split in two because it's more convenient to do so since the implementation calls os.SubItems which wants a root parameter - in other words, splitting that string in two is really only leaking implementation details into the API.

Function as a local variable

I don't like this:

Set GlobRecurse = New List
...
GlobRecurse.Extend GlobRecurse(folder, pat_list, index + 1)

While this certainly works, it is rather ugly. Not because of multiple assignments (roughly requivalent to multiple [non-returning] return statements), but because the function's name is being used a local List variable, happily being appended to.

I would much rather see this:

Private Function GlobRecurse(ByVal root As String, ByVal pat_list As List, ByVal index As Integer) As List

    Dim result As List

    If index = pat_list.Count Then
        Set result = os.SubItems(root, pat_list(index))
    Else

        Set result = New List

        Dim folder As Variant
        For Each folder In os.SubFolders(root, pat_list(index))
            result.Extend GlobRecurse(folder, pat_list, index + 1)
        Next folder

    End If

    Set GlobRecurse = result

End Function

Especially with the recursive nature of the function, you'll want to avoid repeating the function's name for no reason. My rule of thumb, is to never read from the function identifier, only to assign it.


Shift elements left by n indices in a list For the following question, the function
• should mutate the original list
• should NOT create any new lists
• should NOT return anything
Functions that do not create new lists are said to be ”in place.” Such functions are often desirable because they do not require extra memory to operate.
Define shift_left, a function that takes a list and shifts each element in the list to the left by n indices. If elements start ”falling off” on the left, they are placed back on the right. NOTE: you may assume that n is a non-negative integer.
def shift_left(lst, n):
"""Shifts the elements of lst over by n indices.
>>> lst = [1, 2, 3, 4, 5]
>>> shift_left(lst, 2)
>>> lst
[3, 4, 5, 1, 2]
"""

Below is the solution:
def shift_left(lst, n):
    """Shifts the lst over by n indices

    >>> lst = [1, 2, 3, 4, 5]
    >>> shift_left(lst, 2)
    >>> lst
    [3, 4, 5, 1, 2]
    """
    assert (n > 0), "n should be non-negative integer"
    def shift(ntimes):
        if ntimes == 0:
            return
        else:
            temp = lst[0]
            for index in range(len(lst) - 1):
                lst[index] = lst[index + 1]
            lst[index + 1] = temp
            return shift(ntimes-1)
    return shift(n)

I have yet to learn about raising exceptions in Python.
Can this code be more elegant by following a recursive approach? As of now, space (stack frame) usage can be considered secondary.

Some suggestions:

The assignment says “ you may assume that n is a non-negative integer”, but the condition you check is “n > 0”. Since n = 0 is non-negative (and corresponds to no mutation of the list), you should handle that as well.
An assert should only really be used to check that the program isn’t in an impossible situation. It would be preferable to use exceptions, and it’s very simple to do so:

if n < 0:
    raise ValueError("Input %d is not a non-negative integer" % n)

(Alternatively, you could implement negative indices as right-shifting, but I’ll leave that for now.)
You can condense the for loop in shift with a list slice: the following code achieves the same effect:

lst[:-1] = lst[1:]

Everything except the last element in lst is assigned to the element one to its left in the original list.

You can extend this list slicing to do away with the temp variable (which is probably a bit more memory efficient), and do the list slicing in a single line. Shifting one position to the left is achieved with the line

lst[:] = lst[1:] + [lst[0]]

At that point, the shift() function is almost so simple that you can do away with it entirely, and recurse only on the shift_left() function. Here’s what I reduced it to:

if n == 0:
    return
else:
    lst[:] = lst[1:] + [lst[0]]
    shift_left(lst, n-1)


Prompt user for DateTime I'm working on a project where I have to prompt a user a enter several dates. This is the code I came up with for prompting the user for the day, month, and year.

Console.WriteLine("Day: ");
var dateDay = Console.ReadLine();
var dateDayInt = Convert.ToInt32(dateDay);

Console.WriteLine("Month: ");
var dateMonth = Console.ReadLine();
var dateMonthInt = Convert.ToInt32(dateMonth);

Console.WriteLine("Year: ");
var dateYear = Console.ReadLine();
var dateyearInt = Convert.ToInt32(dateYear);

DateTime myDate = new DateTime(dateYearInt, dateMonthInt, dateDayInt, 00, 00, 00, 000);

I'm using this date in a method, so it needs to be in this format.

As you can see, it's a ton of repetitive code. If I even add one or two more dates, my project will become very cluttered. Let's say I need to get 20 dates. Is there an easier way to do this, like only having one prompt per date?

Note that I'm not very familiar with C#, so the code that I write may not actually compile.

  Let's say I need to get 20 dates. Is there an easier way to do this, like only having one prompt per date?

There's a much easier way to do this, and that is to extract this to it's own separate method. This method should do just what your code is doing now: prompt the user for the day, the month, and the year, and should return a DateTime.

public static DateTime PromptDateTime()
{
    Console.WriteLine("Day: ");
    var day = Convert.ToInt32(Console.ReadLine());

    Console.WriteLine("Month: ");
    var month = Convert.ToInt32(Console.ReadLine());

    ... year...

    return new DateTime(year, month, day, 0, 0, 0, 0);
}

Need to call this method twenty times? Go ahead (you'd probably want to use a loop though). With an extracted method like the one above, you won't have to worry about having cluttered code.

Validate Input

This is how you are getting input:

var dateDay = Console.ReadLine();
var dateDayInt = Convert.ToInt32(dateDay);

What if the user enters something like this?

  LOOK MA! NO VALIDATION

What's your code going to do? It's going to throw an error because Convert.ToInt32 doesn't quite know how to parse that.

To validate the user's input, you should use int.TryParse. This method takes a string and an output variable and attempts to parse the string into a number and puts the parsed number in the output variable. If the parse is unsuccessful, then this method returns false.

What this method could look like in action:

var input = Console.ReadLine();
var num;
if(int.TryParse(input, out num))
{
    Valid!
} else
{
   Invalid!
}

Of course, this is a little much to put inside this method you have (considering the fact that you'll have to keep looping to get the user's proper input). Therefore, you should extract this into a method.

public static int ReadInteger() {
    while(true) {
        var input = Console.ReadLine();
        var num;
        if(int.TryParse(input, out num))
        {
            return num;
        }
    }
}

The above method keeps on looping until proper input is receive. As you may notice, this is not very suitable for unit testing as STDIN is forced. However, as Dan Lyons commented:

  Console.WriteLine utilizes Console.Out for everything - if the method took in a TextWriter, he could use Console.Out in "production", but a StringWriter in test.

Now, with proper validation, your extracted method would look like this:

public static DateTime PromptDateTime()
{
    Console.WriteLine("Day: ");
    var day = ReadInteger();

    Console.WriteLine("Month: ");
    var month = ReadInteger();

    ... year...

    return new DateTime(year, month, day, 0, 0, 0, 0);
}

As svick mentioned, this is still technically not enough validation. What if, for the month, the user enters

  13

That doesn't make much sense, does it?

This, of course, can lead to some complications. Some months had 30 days and others have 31. Validation in general should be mostly simple; you just check that the month number is between 1 and 12 (or 0 and 11, but be consistent), and that the year number is between 1 and the current year (unless you want to handle BC/BCE, in which you'd have to do some extra checking).

However, this still doesn't solve the days-in-month problem. A solution could be to create a map/table showing the months and the corresponding amount of days and use that to look up whether the day number is valid. However, then you encounter February (yay!) with which you have to do further validation. Honestly, your best bet would be to go with Zack's solution as it is more idiomatic and would probably be easier to do parsing with (although, I don't know how it handles that days-in-month issue either).


C++ how to use std::any_cast and avoid else if statements I have created a Node class that I am using for various projects in my data structures class(at school) and I am overloading the insertion operator <<. I am using std::any for my container datatype inside Node class and am using std::any_cast to insert this data into a stream whenever possible. Std::any_cast requires that I know the datatype beforehand, and so I've come up with a hacky way to get this information by detangling the value typename into a string and then using a ton of else if statements to compare this string, as you can see in my code below... I'm looking for a different solution or even some advice about why this approach is fundamentally wrong and how to go about making better programming decisions. Lay it on me, I need help and can take it!

friend std::ostream& operator << (std::ostream& out, Node& n){

    std::string t;
    t = abi::__cxa_demangle(n.value.type().name(), 0, 0, 0);;

    if (t.find("string") != std::string::npos) { out << std::any_cast<std::string>(n.value); }
    else if (t == "unsigned long long int") { out << std::any_cast<unsigned long long int>(n.value); }
    else if (t == "unsigned long int") { out << std::any_cast<unsigned long int>(n.value); }
    else if (t == "long long int") { out << std::any_cast<long long int>(n.value); }
    else if (t == "long int") { out << std::any_cast<long int>(n.value); }
    else if (t == "unsigned short int") { out << std::any_cast<unsigned short int>(n.value); }
    else if (t == "short int"){ out << std::any_cast<short int>(n.value); }
    else if (t == "int") { out << std::any_cast<int>(n.value); }
    else if (t == "long double") { out << std::any_cast<long double>(n.value); }
    else if (t == "double") { out << std::any_cast<double>(n.value); }
    else if (t == "float") { out << std::any_cast<float>(n.value); }
    else if (t == "bool") { out << std::any_cast<bool>(n.value); }
    else if (t == "unsigned char") { out << std::any_cast<unsigned char>(n.value); }
    else if (t == "signed char") { out << std::any_cast<signed char>(n.value); }
    else if (t == "char") { out << std::any_cast<char>(n.value); }
    else { out << t; };

    return out;
};

First, this call to __cxa_demangle returns a buffer allocated with malloc and which must be freed. This isn't freed, so this code has a memory leak.

Second, as I understand it, __cxa_demangle does not exist in MSVC, so this is not portable.

Third, the .find("string") catches more than std::string. It would match std::wstring, std::stringstream, void (*)(std::string).

Fourth, this code relies essentially on knowing the potential types that are in that std::any. That is knowledge at a distance, and if you allow more types at the place of construction, you need more types here.

Fifth, if the type isn't one of those types, the error is silent and the output is the type contained, not the value. That could be harmless, but it could be a real problem. If you allowed type Foo in the any, and neglected to update this location, you wouldn't be able to tell if the output Foo was that type name or the value of a string stored in the any.

The suggestion of the other answer is to constrain the types stored using the type storing them. Instead of any use std::variant<std::string, long long, unsigned long long, double>. I added the string and unsigned options relative to the other answer. Then just visit:

std::visit([&](const auto& value) { out << value; }, n.value);

This is a very strong option.

A second option would be to erase not almost every aspect of the type (as with std::any) but instead leave the interface you need, e.g. erase everything but the stream operator if that's all you need:

    printable p = 2.3;
    std::cout << p << "\n";
    p = "foo";
    std::cout << p << "\n";

Example: https://godbolt.org/z/NtJqWk

(edit: added a missing virtual destructor to impl_base)


Setting .conf files for Nginx I use Nginx as a server, and I currently use this for configuration of each my sites. Basically, I have multiple files like this located in /etc/nginx/conf.d/.

#example.conf
server {
  listen             80;
  server_name        www.my-site.com  my-site.com;
  root               /var/www/html/my-site.com;

  location / {
         rewrite ^/category/(.*)$ /category.php?id=$1 last;
         rewrite ^/profile/(.*)$ /profile.php?id=$1 last;
         try_files $uri $uri/ /index.php?$query_string;
  }

  index  index.html index.htm index.php;

  error_page  404              /404.html;
        location = /var/www/html/nginx/error/404.html {
        root       /var/www/html/nginx/error/;
  }

  error_page   500 502 503 504  /50x.html;
       location = /var/www/html/nginx/error/50x.html {
       root        /var/www/html/nginx/error/error/;
  }

  location ~ \.php$ {
       fastcgi_pass   127.0.0.1:9000;
       fastcgi_index  index.php;
       fastcgi_param  SCRIPT_FILENAME  $document_root$fastcgi_script_name;
       include        fastcgi_params;
  }
}

So, I have 6 sites with their own file like this, and since I am about to do a complete re-install of my server, I would like to know how I can improve this configuration, both security and optimization wise.

The config looks great. However, a few points:

If your server responds to both the www and non-www version of the site, just leave a wildcard. So server name should read:

server_name    *.my-site.com;

You shouldn't put your configs in /etc/nginx/conf.d. Instead, place them in /etc/nginx/sites-available and then symlink them (ln -s) to /etc/nginx/sites-enabled. So, your /etc/nginx directory should look like:

 /etc/nginx/
     nginx.conf
     sites-available/
     my-site.conf
     my-other-site.conf
     yet-another-site.conf
 sites-enabled/
     my-site.conf -> /etc/nginx/sites-available/my-site.conf
     my-other-site.conf -> /etc/nginx/sites-available/my-other-site.conf
     yet-another-site.conf -> /etc/nginx/sites-available/yet-another-site.conf


Find the subarray with the max sum In an interview I was asked to solve the following problem:

  find a subarray with max sum

I have written a piece of code for the same.  I need your help in reviewing this code.

package com.ankit.rnd;

public class MaxSubArrSum {

    int largestSum=0;
    int previousLargestSum=0;

    public static void main(String[] args) {
        // int [] array = {-2,1,-3,4,-1,2,1,-5,4};
        int [] array = {-5,1,-3,7,-1,2,1,-4,6};
        // int [] array = {-2,-3,-4,2};

        MaxSubArrSum obj = new MaxSubArrSum();

        for(int varindex=0;varindex<array.length;varindex++){

            // int sumis =new MaxSubArrSum().findSum(varindex,array);
            // System.out.println("sumis::" +sumis);
            obj.splitCurrentArray(varindex,array);
        }
    }

    private void splitCurrentArray(int in, int[] arr) {

        int [] tempArr = new int[arr.length-in];
        for(int i=in;i<arr.length;i++){
            if(in ==0){
                tempArr[i] = arr[i];
            }
            else{
                tempArr[i-in] = arr[i];
            }
        }

        int sum =findSum(in, tempArr);
        System.out.println("Previous Largest Sum::" + previousLargestSum);
        System.out.println("Largest Sum found:" + sum);
    }

    @SuppressWarnings("unused")
    private int findSum(int start,int [] array) {

        int[] currentArray ={};

        int [] largestArray = new int[array.length];
        int sum=0;
        /*for(int i=start;i<array.length;i++){*/
            for(int i=0;i<array.length;i++){
            //a little inefficient here as it always create an array with size more than total number of elements that should be there in the temp array.
            int psuedoIndex=i;
            if(start==0){
                currentArray = new int[i + 1];
                for (int j = 0; j <= i; j++) {
                    currentArray[j] = array[j];

                    psuedoIndex=psuedoIndex+1;
                }
            }
            else {
                currentArray = new int[i+1];
                for (int j = 0; j <= i; j++) {
                    currentArray[j] = array[j];

                    /*
                     * if(psuedoIndex == array.length){ //needs a fix. as we
                     * have reached the end of the array. //currentArray[j] =
                     * array[psuedoIndex-1]; currentArray[j] = 0; break; } else{
                     *
                     * currentArray[j] = array[psuedoIndex]; is commented out
                     * because it missed the element in the previous array.
                     *
                     *
                     * //currentArray[j] = array[psuedoIndex]; currentArray[j] =
                     * array[j]; }
                     */

                    psuedoIndex=psuedoIndex+1;
                }
            }

            if((sum = calculate(currentArray))>largestSum){
                previousLargestSum=largestSum;
                largestSum=sum;
                for(int k=0;k<currentArray.length;k++){
                    System.out.print(currentArray[k] + "|");

                }
                System.out.println("");
            }
        }
        return largestSum;
    }

    private int calculate(int [] currentArr){
        int sumOfElements =0;
        for(int index=0;index<currentArr.length;index++){
            sumOfElements +=currentArr[index];

        }
        //System.out.println("sum is:" + sumOfElements);
        return sumOfElements;
    }
}

Reading and debugging your code and two other answers pops up. (And a third one popped up while writing this answer)

There's a whole lot to say about your code. I will provide comments about your existing code and also comment about a different way to think about the problem.

You have a whole lot of commented code, and you don't write why it has been commented. Either way, I would recommend removing all your commented code before putting it up for review.

The //needs a fix comments here almost made me close the question for not being working code. Your code works (even though it is inefficient) so remove all such commented code:

                /*
                 * if(psuedoIndex == array.length){ //needs a fix. as we
                 * have reached the end of the array. //currentArray[j] =
                 * array[psuedoIndex-1]; currentArray[j] = 0; break; } else{

        if(in ==0){
            tempArr[i] = arr[i];
        }
        else{
            tempArr[i-in] = arr[i];
        }

There is no need to treat in ==0 as a special case here. Your else can take care of that as well. Replace all this with:

tempArr[i-in] = arr[i];

            currentArray = new int[i + 1];
            currentArray = new int[i+1];

Please use consistent spacing. And this applies to all your code. I recommend using spaces around + signs and such.

Compare

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

vs.

for(int i=0;i<array.length;i++){

I prefer the first version. No need to make code too compact. Itwillonlymakeithardertoread (right?).

if((sum = calculate(currentArray))>largestSum){

Sure, this works, but I think it would be more readable by splitting it on two lines.

int sum = calculate(currentArray);
if (sum > largestSum) {

private int calculate(int [] currentArr){

Non-optimal spacing in both the method declaration and method body.
Unclear method name. calculate what exactly? Rename it to calculateSum, please.
Perfect use-case for a for-each loop.
Here's what I would do:

private int calculate(int[] currentArr) {
    int sumOfElements = 0;
    for (int value : currentArr) {
        sumOfElements += value;
    }
    return sumOfElements;
}

for(int k=0;k<currentArray.length;k++){
    System.out.print(currentArray[k] + "|");
}

There is a Arrays.toString method, use it.

System.out.print(Arrays.toString(currentArray));

A different approach

You're using way too many for-loops for my taste.

Consider your starting array:

int[] array = {-5,1,-3,7,-1,2,1,-4,6};

It starts with a negative number, and as you want to find the sub-array with the largest sum, there's no need to start counting on a negative number.

So, skipping that one we have these left:

int[] array = {1,-3,7,-1,2,1,-4,6};

By starting on \$1\$ and looping until you encounter a negative number, you can see that \$1 + -3 = -2\$, which makes this part unnecessary to check. It will not improve your array. The highest sum we can create out of this is simply \$1\$ for now.

Skipping the \$1\$ and \$-3\$ and we end up with:

int[] array = {7,-1,2,1,-4,6};

Now let's start looping. \$7 + -1 = 6\$ so that's good, let's continue.

\$2 + 1 + -4 = -1\$ OK, that ended up being a not so pleasant experience, but considering the \$6\$ we have from before, we end up with \$5\$ so let's continue.

\$6\$ now that's good. And now the array has ended. Adding this to our previous result and we end up with \$5 + 6 = 11\$ and we have our final result.

There's no need to start the original looping at 2 which will only count 2,1,-4,6 as the 7,-1 in the beginning of the array will give a more positive result than starting from this 2.

By taking this in consideration, you can increase the speed of your algorithm a lot.

The code for this approach can be found and review in my Code Review question

Edit: Another addition. You don't provide a method to retrieve the result without printing it. Your way of returning the result seems to be to simply print it to the console. That's not a good way. You should have provided a method like this: int[] maxSubArraySum(int[] array)


Text-based menu to manipulate a database text file I am making a program that lets a user manipulate a database (a text file).

In the code I am posting, I show only 2 of the menu choices, namely "createdb" and "deletedb", and a few functions I made to desperately make the functions more compact. But my problem is that the pattern is similiar for all the other menu options. I ask the user to either enter the name of the database or "b" to return to the menu, and then check if the file exists.

Is there a way I can easily separate this to make my code more compact? I tried to do this part in the menu and have the choice functions be of type

FilePath -> IO ()

But then my menu looked really terrible. Here is a small part of the code:

type Choice = (String, String, IO ())

choices :: [Choice]
choices =
    [("a", "create a database", createdb),
    ("b", "delete a database", deletedb),
    ("c", "insert an entry to a database", insert),
    ("d", "print a database", selectall),
    ("e", "select entries from a database", select),
    -- more similiar choices

menu :: IO ()
menu = do
    (mapM_ putStrLn . map showChoice) choices
    c <- get "Enter the letter corresonding to the action of choice:"
    case filter ((== c) . fst3) choices of
    [] -> back "Not a valid choice. Try again"
    (_, _, f) : _ -> f

createdb :: IO ()
createdb = do
    n <- maybeName
    if isNothing n then menu else do
    let name = fromJust n
    fp <- maybeFile name
    if isJust fp
    then back $ "Error: \"" ++ name ++ "\" already exist."
    else do
        cols <- get "Enter unique column names in the form n1,n2,...,n (No spaces):"
        let spl = (splitOnComma . toLower') cols
        case filter (== True) (hasDuplicates spl : map (elem ' ') spl) of
            [] -> writeFile (name ++ ".txt") (cols ++ "\n")
            _  -> back "Error: Column names must be unique and have no spaces."

deletedb :: IO ()
deletedb = do
    n <- maybeName
    if isNothing n then menu else do
        let name = fromJust n
        fp <- maybeFile name
        if isJust fp
        then removeFile (fromJust fp)
        else back $ "Error: Could not find " ++ name

maybeName :: IO (Maybe String)
maybeName = do
    input <- get "Enter database name or 'b' to go back to the menu."
    return $ case input of
        "b" -> Nothing
        _   -> Just input

maybeFile :: String -> IO (Maybe FilePath)
maybeFile name = do
    let fn = name ++ ".txt"
    exists <- doesFileExist fn
    return $ if exists then Just fn else Nothing

back :: String -> IO ()
back msg = do
    putStrLn msg
    menu

get :: String -> IO String
get msg = do
    putStrLn msg
    getLine

I'd do Maybe FilePath -> IO ():

menu :: IO ()
menu = do
    (mapM_ putStrLn . map showChoice) choices
    c <- get "Enter the letter corresonding to the action of choice:"
    case filter ((== c) . fst3) choices of
    [] -> back "Not a valid choice. Try again"
    (_, _, f) : _ -> maybeName >>= maybe menu (maybeFile >=> f)

createdb :: Maybe FilePath -> IO ()
createdb (Just file) = back $ "Error: \"" ++ file ++ "\" already exists."
createdb Nothing = do
  cols <- get "Enter unique column names in the form n1,n2,...,n (No spaces):"
  let spl = splitOnComma $ toLower' cols
  if hasDuplicates spl || any (elem ' ') spl
    then back "Error: Column names must be unique and have no spaces."
    else writeFile (name ++ ".txt") (cols ++ "\n")

deletedb :: Maybe FilePath -> IO ()
deletedb Nothing = back $ "Error: Could not find that database."
deletedb (Just file) = removeFile file


Iterator-Pattern for an iterator that returns one objects that changes I wrote an algorithm that returns all k-sized subgraphs of another given graph. Because I don't want to make the runtime of the next() call of the iterator depended on k, I don't want to return a new object every time. Instead I have one fixed instance-variable subG in the iterator that is changed between the calls und always represents the current subgraph that was calculated. It acts as a view that can be copied if needed if you want to use it further.
This method works fine and this is not my problem.

If I want to use hasNext(), I'd need to calculate the next subgraph, which would change the view subG as a side-effect. This is not wanted. Currently I'm using my own interface for this iterator:

/**
 * Interface for all iterators that always return the same object, but change it in order to return the next value.
 * Therefore, the object should not be changed unintentionally, because that would result in unwanted side-effects.
 * This prevents the use of "hasNext()" as this would have to calculate the next value for the object, but this method
 * is not expected to have side-effects. The only method that changes the object is generateNext(), which also is
 * its only purpose.
 *
 * The standard pattern for using this interface would be:
 * while (iter.hasCurrent()) {
 *     doSomethingWith(iter.getCurrent());
 *     iter.generateNext();
 * }
 */
public interface BlindIterator<T> {

    /**
     * @return True iff the current element is a valid return.
     */
    boolean hasCurrent();

    /**
     * @return Returns the current element, but does NOT generate the next element. This method can be called
     * as often as wanted, without any side-effects.
     */
    T getCurrent();

    /**Generates the next element, which can then be retrieved with getCurrent(). This method thus only provides
     * this side-effect. If it is called while the current element is invalid, it may produce and exception,
     * depending on the implementation on the iterator.
     */
    void generateNext();
}

I've never written something like this. Is there a better pattern for this, if any?

I think most people would be surprised and annoyed by an iterator that mutated previously yielded values every time you called next. While it makes sense to take what you can from the concept of an iterator, the thing you're trying to make isn't an iterator. (Perhaps the idea of a "cursor" would be more applicable, IDK.)

Your proposed interface {hasCurrent, getCurrent, generateNext} seems fine. I do have some questions/suggestions.

If not x.hasCurrent(), does x.getCurrent() return null, or throw an exception, or what?
Depending on 1 (particularly if it doesn't throw an exception), .getCurrent() sounds more like a read-only property, in which case just call it .current(). (or maybe even just .value)
.generateNext() specifically doesn't make anything (because we're supposing that would be expensive); it changes the thing in question. Therefore maybe call it .mutate() or .becomeNext()
A void return is usually a wasted opportunity, even if just for a little extra debugging context. Exactly what will be best to return here depends on exactly how this is going to be used. Returning boolean success might save you a call to .hasNext(); returning T might save you a call to .current(). Probably boolean is better, but it's hard to say.


Collatz Conjecture Meet of 2 Numbers I need the most efficient way to calculate when the Collatz Conjecture of two numbers, a and b respectively, are the same. Here is my algorithm, which is too slow:

while(true) {
        int a = sc.nextInt();
        int b = sc.nextInt();

        if(a == 0 && b == 0) {
            break;
        }

        TreeMap<Integer, Integer> visited = new TreeMap<>();
        visited.put(a, 0);

        int A = a;
        int Acount = 0;

        while(A != 1) {
            Acount++;

            if(A % 2 == 0) {
                A /= 2;
            } else {
                A = A * 3 + 1;
            }

            if(!visited.containsKey(A)) {
                visited.put(A, Acount);
            }
        }

        int B = b;
        int Bcount = 0;

        while(B != 1) {
            if(visited.containsKey(B)) {
                out.println(a + " needs " + visited.get(B) + " steps, " + b + " needs " + Bcount + " steps, they meet at " + B);
                break;
            }

            if(B % 2 == 0) {
                B /= 2;
            } else {
                B = B * 3 + 1;
            }

            Bcount++;
        }

    }

After looking around on Github, I found this, and it worked, although it's essentially the exact same thing.

Original problem

How do I fix my Java code to make it more efficient? I tried it literally at least four times.

Integer Overflow

I suspect that your problem has to do with integer overflow and not with your algorithm itself.  If you don't guard against integer overflow, doing A = A * 3 + 1 could result in a negative value, and you may end up in an infinite loop where you never reach 1.  Try changing your variables to long and resubmit.

By the way, to test for infinite loop cases, try the starting value 113383.

Other suggestions

You could use a HashMap instead of a TreeMap.  A HashMap has \$O(1)\$ insert and lookup compared to \$O(\log n)\$ for a TreeMap.  The difference for this program will be very small though.
In the A loop, you shouldn't have to check if visited.containsKey(A) because the collatz sequence never repeats until you reach 1.  You can just always add A to the map.
If b is 1, your program never prints anything.


Extract numbers from the head of a string in BASH script In my current BASH script project I am processing a string, which cannot have spaces.

At one point I get to where it is possible to have a series of digits. The series can be of any length, including zero, up to the abilities of the shell to handle command line arguments, and need not be a valid decimal integer. Leading zeros are acceptable, decimal points are not, comma separators could be handled for human compatibility if needed.

At the end I should have two strings: the collected digits (stripped of commas if allowed) and the remainder of the original string following the last digit in the series. The original string has been partially consumed by other sections, and the remainder string will be consumed by other sections, which may include a return to this section if another series of digits becomes valid. The collection of digits will be handed off to another section to deal with, including any validation which might be needed.

The snippet I've settled on is this:

original_group="$1"
source_group="$original_group"
number_group=''
while case "${source_group::1}" in
    [[:digit:]] )
        number_group+="${source_group::1}"
        source_group="${source_group:1}"
        ;;
#    ',' )
#        source_group="${source_group:1}"
#        ;;
    * )
        false
esac; do :; done

# Output not normally in the script.
results_top="Removing numbers from the head of %s gives:\n"
results_list="\tDigits: %s\n\tBalance: %s\n"
printf "$results_top" "'$original_group'"
printf "$results_list" "'$number_group'" "'$source_group'"

The possible contents of the source, once it reaches this point, are in one of four types:

An empty string, (the last character could have sent it here and the empty has not been checked for)

Results should be a pair of empty strings

A non-empty string which begins with a non-digit character

Results should be an empty string for the number list and the untouched source for the remainder

A string with one, or more, digits, and nothing more

Results should be the digits for the number list and an empty string for the remainder

A string with one, or more, digits followed by one, or more, additional non-digit characters

Results should be the digits for the number list and the sub-string beginning with the first non-digit character for the remainder

In all four cases, there cannot be an error triggered by the process. (There are no invalid characters, and the series of digits may not be a 'number' in a mathematical sense; it could be an account number, for example.) Validation of the number belongs elsewhere, and the contents of the string are consumed, whether or not the results are valid for some other purpose.

Performance is not a major issue, it is a shell script. In addition, this section is unlikely to be executed more than 3 times in any execution, commonly once if at all. Likewise, Bashisms are not a concern here, as the remainder of the script is replete with such.

Part of my reasoning for using this snippet is the convenience of adding/removing support for commas in the number series, and the ease of changing from [[:digit:]] to [[:xdigit:]] should I need to implement the same general logic for hexadecimal digits later.

Is there something inherently wrong with this method? Or, is there a different method which is simply better, cleaner, or easier?

I avoided RegEx as either extractor, or loop conditional, under the belief that when not needed, RegEx introduces needless overhead. (Yes, performance is not a major issue here, but bad habits are worth avoiding as well.)

bash is slow to append and index into strings: both operations are \$O(n)\$.  Your approach does as many indexes and appends as there are digits in the input, making it \$O(n^2)\$.  And processing a string by chomping the first character repeatedly will be \$O(n^2)\$ in almost every language.

Have a look at these timings for large inputs, using your code:

$ for x in 10000 20000 40000 80000; do echo $x; time bash dig.sh $( tr -dc 0-9 < /dev/urandom | head -c $x ) >/dev/null ; done

input size (1000's of digits)  runtime (seconds)
         10                      1.1
         20                      4.4
         40                     17.7
         80                     71.6

Almost perfectly quadratic!  It's possible to do better, and in fewer lines.

Your code is implementing a state machine, just like a regular expression would, but much less efficiently.  So the regex isn't needless overhead; it's very necessary  (and profitable) "underhead":

dig() {
    [[ $1 =~ ^([0-9,]*)(.*) ]] # this always matches
    digits="${BASH_REMATCH[1]//,}"
    remain="${BASH_REMATCH[2]}"
    printf "input\t'%s'\tdigits\t'%s'\tremain\t'%s'\n" "$1" "$digits" "$remain"
}

+ dig ''
input   ''      digits  ''      remain  ''
+ dig 1234
input   '1234'  digits  '1234'  remain  ''
+ dig 1,234
input   '1,234' digits  '1234'  remain  ''
+ dig abc12
input   'abc12' digits  ''      remain  'abc12'
+ dig 12z34
input   '12z34' digits  '12'    remain  'z34'

$ for x in 10000 20000 40000 80000; do echo $x; time dig $( tr -dc 0-9 < /dev/urandom | head -c $x ) >/dev/null ; done

input size (1000's of digits)  runtime (seconds)
         10                      0.006
         20                      0.010
         40                      0.018
         80                      0.032

Behold: a two-thousand-fold speedup of the slowest case!


Munging select rows in CSV files using Pandas I'm learning Pandas. I have a project where I need to munge some CSV files and resave them as CSV. I can use dictionaries and CSV module, but decided to use DataFrames to get more exposure and practice with Pandas.

The task is to sum() values for some keys while others are not summed. My solution was to add a column called "Line" where items with the same line number would be summed.

sales = [{"Line": 1, "KEY": "Apples", "AMOUNT": 3.99},
         {"Line": 1, "KEY": "Apples", "AMOUNT": 3.99},
         {"Line": 2, "KEY": "Oranges", "AMOUNT": 5.99},
         {"Line": 3, "KEY": "Pears", "AMOUNT": 2.99},
         {"Line": 3, "KEY": "Pears", "AMOUNT": 2.99},
         {"Line": 4, "KEY": "Carrots", "AMOUNT": .99},
         {"Line": 5, "KEY": "Carrots", "AMOUNT": .99},
        ]

df = pd.DataFrame(sales)

Now, Pandas groupby might seem like a perfect solution, but since I need to export the munged file as CSV, this seemed like something I could do only to have to undo.

# Find duplicate Line entries
# Subset df into just duplicate `Line` values
df_tmp = df[df.duplicated(subset="Line", keep=False)]

# Save a list of Line numbers to sum
line_dups = df_tmp['Line'].unique()

for x in line_dups:
    # Sum every line in the DF; one value sum is unchanged
    # asum = df.loc[df['Line'] == x, 'AMOUNT'].sum()
    # or
    # Subset the subset
    df_tmp2 = df_tmp[df_tmp["Line"] == x]
    # sum the sub-subset
    asum = df_tmp2['AMOUNT'].sum()
    # set the value of all keys with the same Line value
    df.loc[df['Line'] == x, 'AMOUNT'] = asum
    # take the inverse of the duplicate subset on the original df
    # Keep only the first duplicate line
    df2 = df[~df.duplicated(subset="Line", keep='first')]

The solution is effective, df2:

  AMOUNT    KEY     Line
0   7.98    Apples  1
2   5.99    Oranges 2
3   5.98    Pears   3
5   0.99    Carrots 4
6   0.99    Carrots 5

And yet, this solution is sort of hairy with all of the temp dataframes. Maybe a more seasoned Pandas user will have something to add which might help me better understand Pandas?

If you have the Line column in your CSV itself, you can replace what you have with:

df.groupby(['Line', 'KEY']).agg({'AMOUNT': 'sum'}).reset_index()


Is ToIEnumerable good practice? I have an extension method that does this:

public static IEnumerable<T> ToIEnumerable<T>(this T source) where T : new()
{
    return new[] { source };
}

I need to be able to convert any object to an IEnumerable.

For example when I use the .NET Except method:

public static IEnumerable<TSource> Except<TSource>(this IEnumerable<TSource> first)

I can call Except by doing the following:

return objects.Except(this.ToIEnumerable());

Is ToIEnumerable<T> good practice or should I solve the problem in differently?

While I don't see why ToIEnumerable is bad practice, in this specific case I think it is. You're comparing with only one object; why not just attach a where clause on to things? I.e.

return objects.Where((other) => other != this);

or possibly

return objects.Where((other) => !Object.ReferenceEquals(other, this));

Not only will this perform better than Except here, but it's easier to understand, and avoids the entire ToIEnumerable thing entirely.


Code to generate a dictionary from two lists I created a script to perform a union on 2 lists: keys and words. I needed to create a dictionary from them. If there are more keys than words, missing words are replaced with "None". Any excess words are removed.

How can I improve this code to be more compact and perform better?

#! /usr/bin/env python3
import random
import string
from itertools import zip_longest
def generator(listKeys,listWords):

    diff = len(listKeys)-len(listWords)
    if diff > 0:
        for i in range(0,diff):
            listWords.append(None)
    elif diff<0:
        listWords = listWords[0:len(listKeys)]

    done = dict(zip(listKeys,listWords))
    return done

if __name__ == "__main__":
    listKeys = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n']
    listWords = [94, 87, 92, 5, 75, 91, 60, 5]
    print("Keys more values:\nKeys: %s \nValues %s\nResult: %s\n" % (listKeys, listWords,generator(listKeys,listWords)))

    listKeys = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k']
    listWords = [9, 65, 18, 80, 2, 16, 81, 98, 73, 2, 88, 30, 1, 78, 92]
    print("Values more keys:\nKeys: %s \nValues %s\nResult: %s\n" % (listKeys, listWords,generator(listKeys,listWords)))

    listKeys = []
    listWords = []

    for i in range(0,random.randrange(5,15)):
        listKeys.append(list(string.ascii_lowercase)[i])

    for i in range(0,random.randrange(6,16)):
        listWords.append(random.randrange(0,100))

    print("Random:\nKeys: %s \nValues %s\nResult: %s\n" % (listKeys, listWords,generator(listKeys,listWords)))

zip and itertools.zip_longest do exactly what you are looking for, you only need to choose between the two depending on the lengths of the lists:

def generator(listKeys, listWords):
    if len(listKeys) > len(listWords):
        zipped = zip_longest(listKeys, listWords)
    else:
        zipped = zip(listKeys, listWords)
    return dict(zipped)

This can even be shortened to

def generator(listKeys, listWords):
    my_zip = zip_longest if len(listKeys) > len(listWords) else zip
    return dict(my_zip(listKeys, listWords))

Note that generator is not a good name for this function as the word generator has a particular meaning in Python.


Passing functors to standard algorithms I created 3 different implementations (which produce the same results) using the count_if function from STL. I wish to check the number of elements in the vector which are strictly less than 6.

These are:

C++ lambda function
Global function (no functor)
Using class and creating my own functor

These implementations work, but, how can I improve them into a more efficient and robust solutions?

#include <vector>
#include <algorithm>
#include <numeric>
#include <functional>
#include <iostream>

using namespace std;

// GLOBAL function (not FUNCTOR) See below for FUNCTOR implementation!
template <int num>
bool lessThan(int x)
{
    return(x < num); // return TRUE if the value is less than NUM
}

// Paramterized Function with more robust FUNCTOR implementation (written by me) (I know stl has this built in)
class LessThan
{
private:
    int m_x; // the value we wish to be less than
public:
    LessThan(int val) : m_x(val) {} // custom constructor: assign a value to x
    bool operator() (int upper) // all values must be strictly less than the upper bound
    {
        return (m_x < upper);
    }
};

int main()
{
    vector<int> vec = { 2, 7, 4, 5, 7, 7};

    // Using new C++ Lambda function implementation
    int x = count_if(vec.begin(), vec.end(), [](int x) {return x < 6; }); // using C++ Lambda Function (just a function with no name)
    cout <<"There exists " << x << " integers in vec which are strictly less than 6." << endl;

    // Using global lessThan6 implementation (NOT FUNCTOR)
    int y = count_if(vec.begin(), vec.end(), lessThan<6>); // not very robust
    cout << "There exists " << y << " integers in vec which are strictly less than 6." << endl;

    // Using MY OWN FUNCTOR implementation
    int z = count_if(vec.begin(), vec.end(), LessThan(6)); // robust
    cout << "There exists " << z << " integers in vec which are strictly less than 6." << endl;

}

I'm not sure entirely what you're asking. If you're asking for which of three is "better" code between:

int x = count_if(vec.begin(), vec.end(), [](int x) {return x < 6; });
int y = count_if(vec.begin(), vec.end(), lessThan<6>);
int z = count_if(vec.begin(), vec.end(), LessThan(6));

I would say x then z then y. The reason for this is that the lambda solution is the only one where the actual functor is in-line with the algorithm. Anybody reading that code knows exactly what x should be immediately. There is no question. It is clear (to the extent that lambda syntax is) and concise.

z would be the C++03 way of doing this thing. It's... ok. We wrote that code for many years. The problem is the body of the functor is often hundreds of lines removed from the call, maybe in a different file... so readers of your code will have to find it. And it's never going to be perfectly clear either (who knows, maybe you named your functor weird and it's returning 6 < x? Naming things is really hard)

y is the worst. Having 6 as a template argument adds nothing to your performance (it's not like you're optimizing out a comparison), has equal readability loss compared to the lambda with the functor, and on top of that, can't even be called with a variable. With the other two, I can easily write:

int threshold = get_threshold();
int x = count_if(vec.begin(), vec.end(), [=](int x) {return x < threshold; });
int z = count_if(vec.begin(), vec.end(), LessThan(threshold));

But there's no way to pass threshold in as a template.

Typically, for algorithms, strongly prefer lambda unless (1) the algorithm call is a small part of the function and (2) the functor is very long and complicated. I say both must be met because something like this is still perfectly acceptable in my opinion:

int countBySomeCriteria() {
    return count_if(vec.begin(), vec.end(), [=](Foo const&) {
        /* some 15 line complicated functor */
    });
}

whereas if that call were surrounded by a lot of other logic, it would be much harder to easily make out.

Lambdas got added to the language basically to make stuff like this easier to write and easier to read. You should use them because they succeed in that goal.

PS. There is a fourth way to pass a functor: the lambda that was a lambda before C++11 lambdas: Boost.Lambda:

int q = count_if(vec.begin(), vec.end(), _1 < 6);

Make of that what you will.


Transpose of a very large matrix with fewer than 1021 rows with Python I have a matrix of fewer than 1021 rows. I'd like to take its transpose, but it is too big for GNU datamash and awk solutions to fit into memory.

To keep memory overhead low, my thought is that I keep a list of read-only file pointers to the start of each row. I then read out bytes from each pointer until I hit a delimiter (tab or newline).

Because I have fewer than 1021 rows, I won't hit the usual 1024 OS-based file pointer limit.

Once I have read out a field from all file pointers, I write a newline character and start over, reading a field from all file pointers, and again, until no more bytes are available:

#!/usr/bin/env python

import sys
import os

try:
  in_fn = sys.argv[1]
except ValueError as ve:
  sys.exit(-1)

def get_size(fn):
  st = os.stat(fn)
  return st.st_size

#
# 1) Read in file offsets to start of new line
# 2) Open up a file pointer to that offset
# 3) Process each file pointer to get a value until a delimiter is hit, then write it as a row of output
#

size = get_size(in_fn)

fps = []
new_fp = open(in_fn, 'r')
new_fp.seek(0, 0)
fps.append(new_fp)

with open(in_fn, 'r') as f:
  byte = f.read(1)
  while byte:
    if byte == '\n':
      new_offset = f.tell()
      if new_offset < size:
        new_fp = open(in_fn, 'r')
        new_fp.seek(new_offset, 0)
        fps.append(new_fp)
      else:
        break
    byte = f.read(1)

while size > 0:
  for fi, f in enumerate(fps):
    byte = f.read(1)
    size -= 1
    while byte:
      if byte == '\t' or byte == '\n':
        if fi != len(fps) - 1: sys.stdout.write('\t')
        break
      sys.stdout.write('%s' % (byte))
      byte = f.read(1)
      size -= 1
  sys.stdout.write('\n')

for f in fps:
  f.close()

Is there anything I can do to improve the performance of this? Reading and processing a set of file pointers one byte at a time seems quite expensive. However, I need to find newline characters to build offsets and file pointers. Is there a cleverer/faster way (in Python) to find the byte offsets of newlines?

The golden rule of I/O performance is never read a block twice.  You're reading each block thousands of times.

I made a thousand-row test file with records of about 2MB each:

tr -dc '[:alnum:]' < /dev/urandom  | fold -w2000000 | head -1000 > test.txt

And modified the fps-building portion of your code thusly:

#!/usr/bin/env python
from prettyprinter import pprint
import sys
import os
import re

try:
  in_fn = sys.argv[1]
except ValueError as ve:
  sys.exit(-1)

sz = os.path.getsize(in_fn)
fps = [ open(in_fn, 'r') ]
bufsz = 100 * 2**20 # MB

with open(in_fn, 'r') as f:
  offset = 0
  buf = f.read(bufsz)
  while len(buf)>0:
    for p in [m.start() for m in re.finditer(r'\n', buf)]:
      new_pos = offset+p+1
      if new_pos < sz:
        new_fp = open(in_fn, 'r')
        new_fp.seek(new_pos, 0)
        fps.append(new_fp)

    offset = f.tell()
    buf = f.read(bufsz)

pprint( list( map( lambda f: f.tell(), fps ) ) )

This runs 65 times faster than the bytewise version on my machine.

The actual transposition will benefit similarly from this treatment.   Keep a buffer for each row and append to it with a read when it's empty.

If you want to keep the logic simpler, you can compromise (with a performance penalty) by reading a few kilobytes at a time and rewind-seeking  the filehandle back to the first tab character.  This would remove the need to keep an array of buffers.