Timestamp when file last modified in C# I have the following code in PHP that appends a file path with a timestamp of when it was last modified:

function AssetTimestamp($filename)
{
    if (file_exists($filename)) {
        return $filename . '?' . @filemtime($filename);
    } else {
        return $filename;
    }
}

I have written the equivalent in C#

public static string AssetTimestamp(string filename)
    {
        if (System.IO.File.Exists(HttpContext.Current.Server.MapPath(filename)))
        {
            DateTime dt = System.IO.File.GetLastWriteTimeUtc(HttpContext.Current.Server.MapPath(filename));
            Int32 unixTimestamp = (Int32)(dt.Subtract(new DateTime(1970, 1, 1))).TotalSeconds;
            return System.Web.VirtualPathUtility.ToAbsolute(filename) + "?" + unixTimestamp;
        }
        else
        {
            return System.Web.VirtualPathUtility.ToAbsolute(filename);
        }
    }

But as you can see, the code is HUGE in comparison! Can anyone suggest some better ways to handle this, as I can't believe that PHP can do all of that in just one line compared to all the crazy MapPath and converting of dates etc.

The conversion of the code isn't that easy because of two key differences between PHP and ASP.NET:

.NET doesn't do Unix timestamps, so you'll have to introduce a conversion from DateTime to timestamp.
ASP.NET doesn't set the current directory to your application's directory, so relative paths aren't going to point to files existing in your web application.

So all code you have there is relevant. Not shorter, but more readable and less repetitive is the following:

using System.IO;

public static string AssetTimestamp(string filename)
{
    string filePath = HttpContext.Current.Server.MapPath(filename);
    string absoluteUri = System.Web.VirtualPathUtility.ToAbsolute(filename);

    if (!File.Exists(filePath))
    {
        return absoluteUri;
    }

    DateTime dt = File.GetLastWriteTimeUtc(filePath);
    int unixTimestamp = (int)(dt.Subtract(new DateTime(1970, 1, 1))).TotalSeconds;
    return absoluteUri + "?" + unixTimestamp;
}


Re-arranging an matrix of numbers to a numeric list If I have an array of numbers like this:

1  3  4  6  7
1  2
2  4  5  9
5  7
1  2  3  5

Is there a quick way to take all of the unique numbers and arrange them into a single column, like this?

1
2
3
4
5
6
7
9

The approach I have works, but takes a very long time for large matrix:

Sub Test1
Call RecordArrange
Call RemoveDuplicates2
End Sub

Private Sub RecordArrange()
Worksheets("List").Activate
Dim Rng As Range
Dim i As Long
i = 1
Application.ScreenUpdating = False
Application.EnableEvents = False
Application.Calculation = xlManual
Application.DisplayStatusBar = False
Application.EnableEvents = False

Dim lastRow As Long
lastRow = Range("A1").End(xlDown).row

While i <= lastRow

Set Rng = Range("A" & i)

If IsEmpty(Rng.Offset(0, 1).Value) = False Then
Rng.Offset(0, 1).Copy
Rng.Offset(1, 0).Insert Shift:=xlDown
Rng.Offset(0, 1).Delete Shift:=xlToLeft

Else: i = i + 1

End If
Wend
Columns("A:A").Select
    ActiveWorkbook.Worksheets("List").Sort.SortFields.Clear
    ActiveWorkbook.Worksheets("List").Sort.SortFields.Add Key:=Range("A1"), _
        SortOn:=xlSortOnValues, Order:=xlAscending, DataOption:=xlSortNormal
    With ActiveWorkbook.Worksheets("List").Sort
        .SetRange Range("A1:A8000")
        .Header = xlNo
        .MatchCase = False
        .Orientation = xlTopToBottom
        .SortMethod = xlPinYin
        .Apply
    End With
    Application.EnableEvents = True
Application.Calculation = xlAutomatic
Application.DisplayStatusBar = True
Application.EnableEvents = True
End Sub

Private Sub RemoveDuplicates2()

Dim Rng As Range
Dim i As Long
i = 1
Application.ScreenUpdating = False
Application.EnableEvents = False
Application.Calculation = xlManual
Application.DisplayStatusBar = False
Application.EnableEvents = False
Dim lastRow As Long
lastRow = Range("A1").End(xlDown).row

While i <= lastRow
Set Rng = Range("A" & i)

If Rng = Rng.Offset(1, 0) And IsEmpty(Rng.Value) = False Then
Rng.Delete Shift:=xlUp

ElseIf Rng <> Rng.Offset(1, 0) And IsEmpty(Rng.Value) = False Then
i = i + 1
ElseIf Application.WorksheetFunction.CountA(Rng) = 0 Then
i = i + 1
Else: i = i + 1

End If

Wend
    Application.EnableEvents = True
Application.Calculation = xlAutomatic
Application.DisplayStatusBar = True
Application.EnableEvents = True
End Sub

This does work, but it seems like a very roundabout approach. Is there a better way?

I started writing this before the other answer was posted - it's a good answer.

I would also use a dictionary because dictionaries cannot have duplicate keys

Option Explicit

Private Sub MatrixToColumn()

    Dim myMatrix As Range
    Set myMatrix = Range("A1:E5")
    Dim myDictionary As Object
    Set myDictionary = CreateObject("Scripting.Dictionary")
    Dim myCell As Range
    Dim rowNumber As Long
    rowNumber = 1
    Dim Key As Variant

    For Each myCell In myMatrix
        If Not IsEmpty(myCell) Then myDictionary(myCell.Value) = 1
    Next

    If myDictionary.Count > 1 Then
       For Each Key In myDictionary.keys
            Cells(rowNumber, 10) = Key
            rowNumber = rowNumber + 1
       Next
    End If
End Sub

Now, to review your code.

Indenting, none of your code is indented. It's good practice to indent all of your code that way Labels will stick out as obvious.
Sub Test1 - I know you're just testing, but that's a very bad name for a sub.
Call - You don't need to Call subs, it's obsolete. Instead just use Sub argument, argument
Worksheets("List").Activate - Be sure to avoid things like .Activate - 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 - https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba-macros .
lastRow = Range("A1").End(xlDown).Row this isn't a great way to do this, the standard way is lastRow = Cells(Rows.Count, 1).End(xlUp).Row - see this answer on SO by a very talented excel MVP
Everything you're doing on the sheet can be done in the code, in a dictionary or at least in an array. That increases speed by a significant amount
Dim Rng - it's a bad name and it doesn't follow Standard VBA naming conventions which hascamelCase for local variables and PascalCase for other variables and names.
Variable names - give your variables meaningful names. Instead of Rng you could make it myMatrix or something similar.

For future reference:

If Rng = Rng.Offset(1, 0) And IsEmpty(Rng.Value) = False Then
Rng.Delete Shift:=xlUp

ElseIf Rng <> Rng.Offset(1, 0) And IsEmpty(Rng.Value) = False Then
i = i + 1
ElseIf Application.WorksheetFunction.CountA(Rng) = 0 Then
i = i + 1
Else: i = i + 1

End If

This piece of code can be optimized

If Not IsEmpty(Rng.Value) Then
    If Rng = Rng.Offset(1) Then Rng.Delete shift:=xlUp
Else: i = i + 1
End If

Your While loops aren't the preferred way in VBA; instead use a For loop-

For i = 1 to lastRow
Next

If IsEmpty(Rng.Offset(0, 1).Value) = False Then

I prefer to use a Not instead of If False because If usually checks for True

If Not IsEmpty(rng.offset(,1)) then

Instead of calculating lastRow twice, just pass it from one sub to the other

Private Sub RemoveDuplicates2(ByVal lastRow as Long)

Now you can

RemoveDuplicates2 lastRow

You could even pass your range as well

Private Sub RemoveDuplicates(ByVal lastRow as Long, ByVal myRange as Range)

This isn't great either

ActiveWorkbook.Worksheets("List")

Worksheets have a CodeName property - View Properties window (F4) and the (Name) field can be used as the worksheet name. This way you can avoid Sheets("List") and instead just use List.


Project Euler #10 in Ruby - summation of primes I'd like to have some feedback as to the performances of the algorithm.

class Sumprime
    attr_accessor :number

    def initialize(number)
        self.number = number
        prime_serie
    end

    def is_prime?(num)
        counter = 2
        root_num = Math.sqrt(num).ceil
            while counter <= root_num
                    if num % counter == 0 && num != counter
                        return false
                        break
                    else
                        counter += 1
                    end
            end
            return true
    end

    def percentage_prime
        ((prime_serie.size / number.to_f) * 100)
    end

    def prime_serie
        prime_array = [1]
        counter = 2
        while counter < number
            if is_prime?(counter)
                prime_array << counter
                counter += 1
            end
            counter += 1
        end
        prime_array.inject(:+)
    end
end

Instead of checking whether each number from 2 until n is prime, you could generate all the primes that are less or equal to n using an algorithm like the Sieve of Eratosthenes.

A trivial implementation of the sieve in ruby is:

def sieve(n)
  sieve = []
  root = Math.sqrt(n)
  (3..root).step(2) do |i|
    if !sieve[i]
      (i**2..n).step(i) do |j|
        sieve[j] = true
      end
    end
  end
  [2] + (3..n).step(2).select{ |i| !sieve[i] }
end

Thus, your code could be rewritten as:

class NewSumprime
  attr_accessor :number

  def initialize(number)
    self.number = number
  end

  def sieve(n)
    sieve = []
    root = Math.sqrt(n)
    (3..root).step(2) do |i|
      if !sieve[i]
        (i**2..n).step(i) do |j|
          sieve[j] = true
        end
      end
    end
    [2] + (3..n).step(2).select{ |i| !sieve[i] }
  end

  def prime_serie
    sieve(number).inject(&:+)
  end
end

A simple benchmark on my PC gives:

ts = Time.now.to_f ; Sumprime.new(1000000).prime_serie ; Time.now.to_f - ts
=> 11.137484788894653
ts = Time.now.to_f ; NewSumprime.new(1000000).prime_serie ; Time.now.to_f - ts
=> 0.493511438369751


2-factor authentication code generator in Python 3 Google has an Authenticator app that, given a secret and the time, shows a 6-digit code. The code cycles every 30 seconds. I wrote an implementation in Python 3 (can be run in Python 2 with some changes) that optionally generates a qrcode for a new user and/or just shows the current code displayed in the Authenticator app.

import hmac, base64, struct, hashlib, time, pyqrcode, random, sys, os

def get_hotp_token(secret, intervals_no):
    key = base64.b32decode(secret)
    msg = struct.pack(">Q", intervals_no)
    h = hmac.new(key, msg, hashlib.sha1).digest()
    o = h[19] & 15
    h = (struct.unpack(">I", h[o:o+4])[0] & 0x7fffffff) % 1000000
    return h

def get_totp_token(secret):
    return get_hotp_token(secret, intervals_no=int(time.time())//30)

# This works whenever I run it, but I'm not 100% sure it conforms to RFC standards
# Also, I'm not sure if the secret is every allowed to not be 16 bytes
def generate_secret(length=16):
    random_secret = ""
    characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ234567"
    for _ in range(length):
        random_secret += random.choice(characters)
    return random_secret

# Pads secret with zeroes and separates with a space
def format_secret(secret, to_length=6):
    padded = '0'*(6-len(str(secret)))+str(secret)
    return padded[:3] + ' ' + padded[3:]

# Sets up a user given a label, username, and generates/takes a secret
# Is there a better way to do the "y/n: " stuff?
def setup_user():
    label = input("Label: ").replace(' ', '%20')
    user = input("User: ").replace(' ', '%20')
    secret = input("Auto-generate secret? y/n: ")

    if secret.lower() == "y":
        secret = generate_secret()
    else:
        secret = input("Secret: ")

    return label, user, secret

secret = "DID NOT SET SECRET"

# Again, is there a better way to do the "y/n: "?
if input("Setup user? y/n: ").lower() == "y":
    label, user, secret = setup_user()

    if input("Show qr-code? y/n: ").lower().strip() == "y":
        # I'm unsure if this URL breaks when certain characters are in the label or username
        qr_url = "otpauth://totp/%s:%s?secret=%s&issuer=%s" % (label, user, secret, label)
        url = pyqrcode.create(qr_url)
        os.system('cls' if os.name == 'nt' else 'clear')
        # Is it possible to make the qrcode smaller when printed in Terminal/Console?
        print(url.terminal(quiet_zone=1))
else:
    secret = input("Secret: ")
    os.system('cls' if os.name == 'nt' else 'clear')

print("User secret: ", secret)

sys.stdout.write("\r%s" % format_secret(get_totp_token(secret)))
sys.stdout.flush()
# This next line is my code to time-sync the timer. I'm not sure if it actually works though.
# I think it might have to do with using modular arithmetic on a float
time.sleep(1 - time.time() % 1)

while True:
    # These next couple of lines format a progress bar for how many seconds are left until a new code.
    seconds = int(30 - time.time() % 30)
    lines = "-" * int(seconds)# / 3)
    blocks = "█" * (30 - len(lines))
    progress = "|" + blocks + lines + "|"

    sys.stdout.write("\r%s" % format_secret(get_totp_token(secret)) + "\t" + progress)
    sys.stdout.flush()

    time.sleep(1)

This uses the pyqrcode library, which does not ship standard with Python.

I noticed that sometimes, the code does not change exactly when it does in the Authenticator app. This may be a problem with my time-syncing code.

What are your thoughts on quality of code, usability, pythonic-ness, and correctness?

Code quality

Each import should have its own, seperate line:

import hmac
import base64
import struct
import hashlib
import time
import pyqrcode
import random
import sys
import os

Top-level functions should be seperated using two blank lines:

def foo():
    foobar()

def bar():
    foobar()

A lot of your comments should actually be docstrings.

Security

Don't use the random module for security purposes. Skimming through the documentation, red banners warn you not to use it for security purposes and instead to use the secrets module. Note that secrets is only part of the standard library in Python 3.6 and upwards. If you need to run this on other versions of Python, which is quite likely, make use of random.SystemRandom (or os.urandom() directly).
Use getpass.getpass() instead of plain input() for gathering secrets like passwords or private codes. [getpass documentation]

General recommendations

Since pyqrcode isn't part of the standard library, you may want to wrap import pyqrcode in a try:/except: statement:

try:
    import pyqrcode
except ModuleNotFoundError:
    print("Couldn't import pyqrcode.")
    sys.exit(1)

If you plan to release this as a module, it's a very good idea to add documentation (docstrings, as well as a README for quick reference), so others understand how the code works and how they should use it.


Keep div fixed when it is inside it's container on scroll The required behaviour is :

The yellow span must be positioned relatively to viewport (position:fixed;) when it is inside the pink div.
The height of yellow span must always be the same as viewport height minus 100px. It has a fixed width in px.
The yellow span must be verticaly centered in the viewport (this rule is modified by rule 4).
On scroll, the yellow span must never be outside the boundaries of the pink div. If the page is scrolled further than the pink div, the yellow span must be absolutely positionned to the bottom of the pink div and scroll up with it
If the page is scrolled back up to the pink div, the yellow span must go back to fixed position (as in rule 1 and 2).

I know sticky positioning does exaclty that but I can't use it because of poor browser support. So I made this snippet and would like to know if it can be optimized or shortened :

$(document).scroll(function() {
  var span = $('span'),
    div = $('div'),
    spanHeight = span.outerHeight(),
    divHeight = div.height(),
    spanOffset = span.offset().top + spanHeight,
    divOffset = div.offset().top + divHeight;

  if (spanOffset >= divOffset) {
    span.addClass('bottom');
    var windowScroll = $(window).scrollTop() + $(window).height() - 50;
    if (spanOffset > windowScroll) {
      span.removeClass('bottom');
    }
  }
});
*{margin:0;}
div {
  position: relative;
  background: pink;
  height: 800px;
  margin-bottom: 800px;
}
span {
  position: fixed;
  right: 0;
  bottom: 50px;
  width: 10%;
  height: calc(100vh - 100px);
  background: gold;
}
.bottom {
  position: absolute;
  bottom: 0;
  right: 0;
}

  some content

This is not really a full answer yet, but after correcting my interpretation of your question (as reflected by comments under it) I keep having some incertainties, and a comment is not wide enough to clearly express them.

Anyway I can already notice some possible improvements, not depending on any further precision:

Cosmetic: the right: 0; property can be suppressed from span.bottom rule, since it doesn't change compared to span one.
Performance:

since their value never changes, declaration of span, div, and divHeight can be moved outside of the scroll() event.
we also can fix $(window) by declaring it as var win at the same level.

Operation: depending on the window size, the initial state may not follow your requirements, e.g.:

When running the code snippet and immediately switching to full page the yellow span should have the bottom class, while it doesn't.
The same happens when toggling between full page and reduced iframe: if the state should change when reaching the new window size, it doesn't till scrolling happens.

Then to avoid that, the computation for affecting/dropping bottom class should occur not only when document.scroll() but also window.resize() event happens.
Additionally, each of those events should be initially fired (although in practice only one is enough).

Here is the so modified snippet:

var span = $('span'),
    div = $('div'),
    win = $(window),
    divHeight = div.height();
$(document).scroll(compute).scroll();
$(window).resize(compute).resize();

function compute() {
  var spanHeight = span.outerHeight(),
      spanOffset = span.offset().top + spanHeight,
      divOffset = div.offset().top + divHeight;

  if (spanOffset >= divOffset) {
    span.addClass('bottom');
    var windowScroll = win.scrollTop() + win.height() - 50;
    if (spanOffset > windowScroll) {
      span.removeClass('bottom');
    }
  }
  console.log('spanOffset='+spanOffset, 'windowScroll='+($(window).scrollTop() + $(window).height() - 50));
}
*{margin:0;}
div {
  position: relative;
  background: pink;
  height: 800px;
  margin-bottom: 800px;
}
span {
  position: fixed;
  right: 0;
  bottom: 50px;
  width: 10%;
  height: calc(100vh - 100px);
  background: gold;
}
.bottom {
  position: absolute;
  bottom: 0;
}

  some content

Now let me explain what keeps puzzling me.
Both snippets (yours and mine) have the following wrong behaviour: as of a certain window height (couldn't precisely figure out yet), once switched to bottom the yellow span will never go back when scrolling up.

So before thinking further I'd want to be certain of what is precisely your need. Expressed independently from the current implementation, it seems to me that it is like follows:

as a fundamental requirement, the yellow span height must always be equal to window height minus a given height (say X, in this case 100px), and must not change
if not upset by the rule #3 below, the yellow span must be vertically centered in the window (its bottom is half the above given X)
taking over the rule #2, the yellow span must never cross the pink div (the bottom of the former can't move lower than the latter's one)

For now I remain pending. Thanks for confirming or correcting me.


IReadOnlyCollection with notifications It is not related to GUI. I have a class with:

Private List, which is edited inside the class.
Open IReadOnlyCollection property for access to private List
Public event for notification on List changing

A list stores objects with properties. The class can changed an item's properties and notify about it. The user can only read an item's properties. I only picked this option.

class MortarInfo
{
    public double Angle { get; private set; }
    public bool State { get; private set; }
    public MortarInfo(double angle, bool state)
    {
        Angle = angle;
        State = state;
    }
}
class Mortars
{
    private List<MortarInfo> _mortarsInfo;
    private ReadOnlyCollection<MortarInfo> _readOnlyMortarsInfo;

    public Mortars()
    {
        _mortarsInfo = new List<MortarInfo>();
        _readOnlyMortarsInfo = _mortarsInfo.AsReadOnly();
    }

    public ReadOnlyCollection<MortarInfo> MortarsInfo
    {
        get { return _readOnlyMortarsInfo; }
    }
    public event ListChangedEventHandler MortarsInfoChanged;

    public void UpdateAngle(int index, double newAngle)
    {
        if (_mortarsInfo[index].Angle != newAngle)
        {
            _mortarsInfo[index] = new MortarInfo(newAngle, _mortarsInfo[index].State);
            if (MortarsInfoChanged != null)
            {
                var arg = new ListChangedEventArgs(ListChangedType.ItemChanged, index);
                MortarsInfoChanged(this, arg);
            }
        }
    }
}

It seems good but I have some comments on this,

Why do you create a new ListItem in every property change and not just replace to old property with the new one?

_list[index].Property1 = property1;

You raise the event without null check. Do this:

var listChanged = ListChanged;
if (listChanged != null)
    listChanged(this,new ListChangedEventArgs(ListChangedType.ItemChanged, index));

As a side note, if Property1 or Property2 are complex objects, It's recommended to create for them you own equal methods (Equal, GetHashCode, == and !=). If the are generic, implement IEquatable<T>


Process bunches of records in seperate threads and wait for them all to finish I feel like there is a better way of writing this code which waits for multiple threads to finish their work (i.e. without using two loops), but my colleagues and I are drawing a blank.

private static void ProcessRecords(long[] recordIds)
{
    var threads = new List<Thread>();
    var recordsPerThread = recordIds.Length / Settings.Default.WorkerThreads + 1;

    for (var i = 0; i < Settings.Default.WorkerThreads; i++)
    {
        var t = new Thread(new ParameterizedThreadStart(Program.SetOwner));
        threads.Add(t);
        t.Start(recordIds.Skip(i * recordsPerThread).Take(recordsPerThread));
    }

    // Wait until all threads have completed
    foreach (var t in threads)
    {
        t.Join();
    }
}

private static void SetOwner(object o)
{
    var recordIds = o as IEnumerable<long>;

    using (Database d = Program.GetDatabase())
    {
        Account owner = Program.GetOwner(d);

        foreach (long id in recordIds)
        {
            try
            {
                var record = d.GetRecordById(id);
                record.Owner = owner;
                record.Save();
            }
            catch (Exception ex)
            {
                // code to log error
            }
        }
    }
}

So your original approach looked something like this:

recordIds.AsParallel()
    .WithDegreeOfParallelism(...)
    .ForAll(id => SetOwner(id));

which involved a separate call to SetOwner for each record. Because SetOwner has a lot of overhead per call, it's indeed much better to let it operate on batches rather than individual items. But you don't need to use threads for that directly:

recordIds
    .SplitIntoBatches(Settings.Default.WorkerThreads)
    .AsParallel()
    .WithDegreeOfParallelism(Settings.Default.WorkerThreads)
    .ForAll(batch => SetOwner(batch));

public static IEnumerable<IEnumerable<T>> SplitIntoBatches<T>(this IEnumerable<T> items, int numberOfBatches)
{
    var batchSize = (items.Count() + numberOfBatches - 1) / numberOfBatches;
    for (int i = 0; i < numberOfBatches; i++)
        yield return items.Skip(i * batchSize).Take(batchSize);
}


Performance of loading fields I wrote a class with this function. Can I improve my code performance in the bottom of the function? I don't like the while loops!

public function loadFields(array $fields, $as_object = Config::FIELDS_AS_OBJECT) {
        $connection = $this->connection;
        $sql = "SELECT ".implode(",", $fields)." FROM ".$this->table." WHERE lang = ?";
        if (!($stmt = $connection->prepare($sql))){
            throw new MysqliPrepareException($connection);
        }
        if(!($stmt->bind_param('s',$this->lang))){
            throw new MysqliBindParamException($connection);
        }
        if(!$stmt->execute()){
            throw new MysqliExecuteException($connection);
        }
        $stmt->store_result();
        $variables = array();
        $data = array();
        $meta = $stmt->result_metadata();
        while ($field = $meta->fetch_field()) {
            $variables[] = &$data[$field->name];
        }
        call_user_func_array(array($stmt, 'bind_result'), $variables);
        $i = 0;
        while ($stmt->fetch()) {
            $array[$i] = array();
            foreach ($data as $k => $v) {
                $array[$i][$k] = $v;
            }
            $i++;
        }
        $array = (!isset($array[1]) ? $array[0] : $array);
        return ($as_object == true ? (object) $array : $array);
}

I'd imagine a $stmt->fetchAll() would be more performant than the while-loop and $stmt->fetch(). Rather than paging over an entire set of records one at a time, try returning the results from the database in-bulk. Your code is making a trip to the database with each iteration of your while loop. Your code will still look virtually identical:

$i = 0;
$r = $stmt->fetchAll();
foreach ($r as $data) {
    $array[$i] = array();
    foreach ($data as $k => $v) {
        $array[$i][$k] = $v;
    }
    $i++;
}

If you're using mysqli (native driver only), and not PDO's MySQL, you could do this:

$i = 0;
$r = $stmt->get_result();
$r = $r->fetchAll();
foreach ($r as $data) {
    $array[$i] = array();
    foreach ($data as $k => $v) {
        $array[$i][$k] = $v;
    }
    $i++;
}

You shouldn't worry about loops per se, but performance hits that you see, whether they come from loops or not. This is only possible if you're profiling your code.


Is using beforeFind() event in background to dynamic update SQL queries a good practice? I have a Projects table, which I need to ORDER BY code column. But problem is code is not a simple INT, but a VARCHAR specifying multiple level (like 01-15_33/23). So I need to sort by first part, then next, and so on.

One of the option I found is to replace order field everywhere with a complex array

    $orderAsc = [
        "cast(Projects.code as unsigned)",
        "cast(SUBSTRING_INDEX(Projects.code,'_', -1) as unsigned)",
        "cast(SUBSTRING_INDEX(Projects.code,'-', -1) as unsigned)",
        "cast(SUBSTRING_INDEX(Projects.code,'/', -1) as unsigned)",
        "Projects.code"
    ];

This is the other solution I wrote.

public function beforeFind(Event $event, Query $query, ArrayObject $options, $primary) {
    $commonOrder = ["Projects.code REGEXP '^[0-9]+' DESC"];

    $orderAsc = [
        "cast(Projects.code as unsigned)",
        "cast(SUBSTRING_INDEX(Projects.code,'_', -1) as unsigned)",
        "cast(SUBSTRING_INDEX(Projects.code,'-', -1) as unsigned)",
        "cast(SUBSTRING_INDEX(Projects.code,'/', -1) as unsigned)",
        "Projects.code"
    ];
    $orderDesc = array_map(function($value) { return $value . ' DESC'; }, $orderAsc);

    $orderAsc = implode(',', array_merge($commonOrder, $orderAsc));
    $orderDesc = implode(',', array_merge($commonOrder, $orderDesc));

    // Order project ascending if no order is specified
    $query->order($orderAsc);

    // Process ['Projects.code' => 'DESC'] and ['Projects.code' => 'ASC'] intelligently
    // Doesn't support other notation like ['Projects.code ASC'] for now
    $query->traverse(function ($conditions, $clause) use ($orderAsc, $orderDesc) {
        if (!empty($conditions)) {
            $conditions->iterateParts(function ($c, &$key) use ($orderAsc, $orderDesc) {
                if ('projects.code=asc' == strtolower("$key=$c")) {
                    $c = $orderAsc;
                    $key = null;
                }
                if ('projects.code=desc' == strtolower("$key=$c")) {
                    $c = $orderDesc;
                    $key = null;
                }
                return $c;
            });
        }
    }, ['order']);
}

It process everything in background. And you don't have to change your code at all anywhere.

One of the other advantage I found is, it works well with Pagination feature which generate ORM query in background. But I feel a developer who is new to this project won't understand what's happening at all.

Any feedback regarding the code I wrote or the thinking behind it is welcomed.

It might lead to a ton of refactoring, but have you considered breaking up your code field into multiple columns, and simply ordering by them all?

Beyond that, magic in callbacks is always risky as you've pointed out. But with such a complex requirement for ordering, doing it manually each time doesn't make a lot of sense either.


Implementing The Proper Undead's Cave Generator I recently stumbled onto a very old article about procedural dungeon generation on this site. Unfortunately, the author did not provide any code, only a description of the algorithm he used. Wanting to use this in my game, I tried to write my own generator following their algorithm:

local random = math.random

local function grid()
    return {
        min = 0,
        max = 0,

        init = function(self, min, max)
            for i = min, max do
                self[i] = {}
            end

            self.min = min
            self.max = max
        end,

        set = function(self, x, y, c)
            if x >= self.min and x <= self.max then
                if y >= self.min and y <= self.max then
                    self[x][y] = c
                end
            end
        end,

        print = function(self)
            for y = self.max, self.min, -1 do
                for x = self.min, self.max do
                    io.write(self[x][y] or "#")
                end

                io.write"\n"
            end
        end
    }
end

local function point(x, y)
    p = {
        x = x,
        y = y,

        print = function(self)
            io.write("<" .. self.x .. ", " .. self.y .. ">\n")
        end,
    }

    setmetatable(p, {__eq = function(self, right)
        return self.x == right.x and self.y == right.y
    end})

    return p
end

local function contains_point(array, p)
    for i = 1, #array do
        if array[i] == p then
            return true
        end
    end

    return false
end

local function shuffle(array)
    local size = #array

    for i = 1, size do
        local swap = random(1, size)
        array[i], array[swap] = array[swap], array[i]
    end

    return array
end

local function offsets(p)
    return {
        point(p.x, p.y+1),
        point(p.x, p.y-1),
        point(p.x+1, p.y),
        point(p.x-1, p.y),
    }
end

local function circular_iterator_of(items, start)
    local index = start - 1
    return function()
        local ret =  items[index + 1]
        index = index + 1
        index = index % #items + 1
        return ret
    end
end

local function carve(list)
    local index = 1
    while index < #list + 1 do
        local currentPoint = list[index]
        local last
        if (index == #list) then
            last = true
        end

        local numChecks = random(last and 1 or 0, 4)
        local neighbors = circular_iterator_of(shuffle(offsets(currentPoint)), 1)

        for i = 1, numChecks do
            local neighbor = neighbors()

            if not contains_point(list, neighbor) then
                table.insert(list, neighbor)
            end
        end

        index = index + 1
    end
end

function generate_map(width, minTilesCarved, seed)
    local map = grid()
    map:init(-width/2, width/2)

    local start = point(0, 0)

    local carvedList = {}
    table.insert(carvedList, start)

    math.randomseed(seed)

    repeat
        carve(carvedList)
    until #carvedList > minTilesCarved

    for i = 1, #carvedList do
        p = carvedList[i]
        if i == 1 then
            map:set(p.x, p.y, "D")
        elseif i == #carvedList then
            map:set(p.x, p.y, "U")
        else
            map:set(p.x, p.y, ".")
        end
    end

    return map
end

map = generate_map(64, 170, os.time())

map:print()

First time posting here, please be somewhat gentle :)

Note:

While it doesn't appear to be on topic for this site, bonus points will be awarded to those who can suggest ways to make my algorithm produce results closer to those on the original site (my caves feel too chunky in general).

Ideal Caves:

My Results:


Are these list-comprehensions written the fastest possible way? This is a simple repeatable question regarding the usage of Python3's comprehensions:

Could the Python3 syntax be used in another way to speed up the process, further so the gap between Python3 and numpy would be reduced? (see chart below).

The results & code:

Above: Runtime test of Python3's list comprehension, dict comprehension, python for loop and numpy

Update 1 Above: Runtime test without power-functions including Janne Karila suggestion to break down the power function to x * x * x

Update2 Above: Runtime test without powerfunctions and with Justin Peels recommendation to use return list(a.values())

#/usr/bin/python3 -

import sys
from datetime import datetime
import numpy
import matplotlib.pyplot as pyplot

def doingnothing(n):
    for i in range(n):
        pass
    return []

def numpysum(n): #NPY
    a = numpy.arange(n) ** 2.
    b = numpy.arange(n) ** 3.
    c = a + b
    return c

def listexpression(n):  #LE
    #return [x**2+x**3 for x in range(n)]
    return [x*x+x*x*x for x in range(n)]

def dictcomprehension(n):  #DC
    # a = {x:x**2+x**3 for x in range(n)}
    a = {x:x*x+x*x*x for x in range(n)}
    # return [a[key] for key in a]
    return list(a.values())

def pythonsum(n): #PS
    a = list(range(n))
    b = list(range(n))
    c = []

    for i in range(len(a)):
            # a[i] = i ** 2.
            a[i] = i * i
            # b[i] = i ** 3.
            b[i] = i * i * i
            c.append(a[i] + b[i])
    return c

def runtimetest(size,verbose=True):
    v = verbose
    d = {'DoingNothing':None,
         'NPY':None,
         'LE':None,
         'DC':None,
         'PS':None}

    start = datetime.now()
    c = doingnothing(size)
    d['DoingNothing'] = datetime.now() - start

    start = datetime.now()
    c = numpysum(size)
    d['NPY'] = datetime.now() - start

    start = datetime.now()
    c = listexpression(size)
    d['LE'] = datetime.now() - start

    start = datetime.now()
    c = dictcomprehension(size)
    d['DC'] = datetime.now() - start

    start = datetime.now()
    c = pythonsum(size)
    d['PS'] = datetime.now() - start

    if v:
        for key in sorted(d):
            print(key, "took", round(d[key].seconds + d[key].microseconds/10**6,6), " sec.")
    else:
        return d

def view(results):
    """
    result['header']=['DoingNothing','NPY','LE','DC','PS']
    result[3000]=[0.0, 0.0, 0.002, 0.003, 0.003001]
    """
    if 'header' in results.keys():
        results.pop('header')

    steps,DN,NPY,LE,DC,PS=[],[],[],[],[],[] #I love multiple assignment!

    for step in sorted(results):
        steps.append(step)
        DN.append(results[step][0])
        NPY.append(results[step][4])
        LE.append(results[step][5])
        DC.append(results[step][6])
        PS.append(results[step][4])

    pyplot.plot(steps,DN)
    pyplot.plot(steps,NPY)
    pyplot.plot(steps,LE)
    pyplot.plot(steps,DC)
    pyplot.plot(steps,PS)
    pyplot.legend(['Empty Loop', 'Numpy.Arange', 'List Comprehension', 'Dict Comprehension', 'Python for Loop'], loc='upper left')
    scale = 'linear'
    pyplot.xscale(scale)
    pyplot.yscale(scale)
    pyplot.title('runtime test')
    pyplot.xlabel('length of list')
    pyplot.ylabel('runtime in seconds')

    pyplot.show()

def longruntimetest(length):
    if length<10**4:
        length=10**4

    result = {}
    result['header']=['DoingNothing','NPY','LE','DC','PS']
    for step in range(10**3, length+1,10**3):
        t=runtimetest(step,verbose=False)
        result[step]=[t['DoingNothing'],t['NPY'],t['LE'],t['DC'],t['PS']]
        for i in range(len(result[step])):
            result[step][i]=round(result[step][i].seconds
                                  +result[step][i].microseconds/10**6,6)

    view(result)

def main():
    if sys.version_info <= (3, 2):
        sys.stdout.write("Sorry, requires Python 3.2.x, not Python 2.x\n")
        sys.exit(1)

    testsize=10**5
    runtimetest(testsize)
    longruntimetest(testsize)

if __name__=='__main__':
    main()

def listexpression(n):  #LE
    return [x * x * (1 + x) for x in range(n)]

is over 3 times faster than your version with x**2+x**3 (from 8.47 ms to 2.31 ms with n = 10000).

Though, NumPy will also benefit from regrouping the expression this way.


Inserting data into an SQLite database I've execute this code in Android for inserting 100+ rows in SQLite database.
My current approach takes more time to initialize the database.

private void getInitialization() {
    try {
        dao = new IdentifyImageDao(MainActivity.this);
        dao.open();
        level = new Level();

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

            level.setLevelNo(Level.LEVEL_NO_ARRAY[i]);
            level.setTotalGuess(0);
            level.setTotalImages(Level.TOTAL_IMAGES_PER_LEVEL[i]);
            if (i < 2)
                level.setLevelStatus(Level.LEVEL_UNLOCK);
            else
                level.setLevelStatus(Level.LEVEL_LOCK);
            dao.insertLevel(level);
        }

        images = new Images();
        // IMAGES_ID.length = 150
        for ( int i = 0; i < Images.IMAGES_ID.length; i++ ) {
            images.setLogoId(Images.IMAGES_ID[i]);
            images.setLogoCheck(Images.IMAGE_UNCHECK);
            dao.insertImages(images);
        }
        dao.close();
    } catch (Exception e) {

    }
}

Database method for image insertion

public long insertImages(Images images) throws SQLException {
    ContentValues value = new ContentValues();
    value.put(LOGO_NO, images.getLogoId());
    value.put(LOGO_CHEECK, images.getLogoCheck());
    return database.insert(TABLE_LOGO_MASTER, null, value);
}

Please review my code and suggest a more efficient and faster approach for inserting multiple data.

void getInitialization()

The name of the method implies that we would get something but the return value is void. Maybe a better name would be in order. How about Initialize() ?

an empty catch block is always a very bad sign. If you want to swallow any exception, you should clearly state with a comment why you are swallowing the exceptions.

it is always recommended to use braces {} for single statment if to make the code less error prone.

images = new Images(); where is images declared ? It would be better to have this as a method scoped variable instead of reusing a class level one.
In addition the name Images implies that it contains multiple Image items which it doesn't do.

If an exception is thrown from inside the try block after the call to  dao.open(); the dao will stay open.
You should use a finally block to close the dao.


Leave at most max_dup consecutive duplicate elements in a list I need to remove excessive duplicates of a particular element from the list, that is, if there's more than max_dup duplicates of selected elements in a slice of the list, leave at most max_dup elements in place. Obviously, stability (retaining order of elements in the list) is a must.

Example:

>>> remove_excessive_duplicates([1, '', '', '', 2, '', 3, '', '', '', '', '', 4, ''])
[1, '', '', 2, '', 3, '', '', 4, '']

The code I wrote:

def remove_excessive_duplicates(lst, dup='', max_dup=2):
    new_lst = []
    dcnt = 0
    for elem in lst:
        if elem == dup:
            dcnt += 1
        else:
            dcnt = 0
        if dcnt > max_dup:
            continue
        new_lst.append(elem)
    return new_lst

It works, but I'm wondering if there's some cleverer way to do it, or the one that uses logic of some Python builtins to achieve the same goal.

This is a one-liner using the features of the itertools module. I'll try to show how you might find this via a step-by-step transformation.

When a function returns a list of items, it's often more convenient (in Python) to refactor it so that it generates the items one at a time using the yield statement. This has some advantages: (i) if the caller is also processing the items one at a time, then you don't need to allocate the whole list in memory; (ii) the first item becomes available immediately, without having to wait for all the items; (iii) if the items are arriving from a file or some other form of I/O, you may be able to process early items in parallel with loading later ones; (iv) a generator can operate on a potentially infinite stream of items without running out of memory.

def remove_excessive_duplicates(iterable, dup='', max_dup=2):
    """Generate items in iterable, but at most max_dup of any consecutive
    sequence of items equal to dup.

    >>> input = [1, 0, 0, 0, 2, 0, 3, 0, 0, 0, 0, 0, 4, 0]
    >>> list(remove_excessive_duplicates(input, dup=0))
    [1, 0, 0, 2, 0, 3, 0, 0, 4, 0]

    """
    count = 0
    for item in iterable:
        if item == dup:
            count += 1
        else:
            count = 0
        if count <= max_dup:
            yield item

(I've also taken the opportunity of adding a docstring and a doctest, and improving some of the variable names.)
What's going on here is that we are finding groups of identical items in the input, and for this Python provides itertools.groupby:

for key, group in groupby(iterable):
    if key == dup:
        count = 0
        for item in group:
            if count >= max_dup:
                break
            yield item
            count += 1
    else:
        yield from group

This has the advantage that we don't need to maintain the running count in the else case, saving some work.
In the if case, we want to yield at most max_dup items from the iterable group. This can be done using itertools.islice:

for key, group in groupby(iterable):
    if key == dup:
        yield from islice(group, max_dup)
    else:
        yield from group

We can factor out the repeated yield from:

for key, group in groupby(iterable):
    yield from islice(group, max_dup) if key == dup else group

That would a sensible place to stop. But to make this into a single expression we can note that the effect of this code is that for each group in the iterable, we construct an output iterable (either islice(...) or group, depending on the key), and the result consists of the concatenation of these iterables. Concatenating a bunch of iterables is a job for the obscurely named itertools.chain.from_iterable:

return chain.from_iterable(islice(group, max_dup) if key == dup else group
                           for key, group in groupby(iterable))


Simplifying a Scrabble Word Finder - Python I'm very new to coding and am trying to work through projects for practical review of what I know, but I would like to be able to simplify/improve what I've made.

This is a project I saw online that I tried to tackle today.  The task was to create something that would provide a list of the best possible words to play given a set of letters on your rack.

scores = {"a": 1, "c": 3, "b": 3, "e": 1, "d": 2, "g": 2,
         "f": 4, "i": 1, "h": 4, "k": 5, "j": 8, "m": 3,
         "l": 1, "o": 1, "n": 1, "q": 10, "p": 3, "s": 1,
         "r": 1, "u": 1, "t": 1, "w": 4, "v": 4, "y": 4,
         "x": 8, "z": 10}

def calc_score(word):
    """Calculate the score of a given word."""
    word_score = 0
    for x in word:
        word_score += scores[x]
    return word_score

def look_up():
    """Create the variable containing the master list of words."""
    read_dict = open("sowpods.txt", "r")
    master = read_dict.read()
    read_dict.close()
    master = master.split("\n")
    return master

word_master = look_up()

def rack_check(f_rack):
    """Check the letters in the rack against the SOWPOD dictionary and
    append valid words to a list."""
    valid_list = []
    for item in word_master:
        letter_bank = list(f_rack)
        for letter in item:
            if letter in letter_bank:
                valid = True
                letter_bank.remove(letter)
            else:
                valid = False
                break
        if valid == True:
            valid_list.append(item)
    return valid_list

def dict_maker():
    """Create a dictionary of the words with their scores."""
    valid_dict = {}
    f_list = rack_check(list((raw_input("Letters: ")).lower()))
    for i in f_list:
        valid_dict[i] = calc_score(i)
    return valid_dict

def list_print():
    """Print a list of the words in descending order of value."""
    dict = dict_maker()
    sort_dict = sorted(dict.items(), key=lambda k: k[1], reverse=True)
    for x in sort_dict:
        print x[0], "-", x[1]

list_print()

Calling the "list_print()" function runs through all the other necessary functions, but I feel like it could be cleaner.  Any feedback would be greatly appreciated.

The program is pretty good and clean so far, the docstrings are clear and it's easy to follow what's going on.

The scores dictionary is a constant, so it should be all uppercase.

Use a context manager to open the text file. This way you don't have
   to worry about the file not being closed, as the context manager does it for you.

def look_up():
    """Create the variable containing the master list of words."""
    with open('sowpods.txt', 'r') as f:
        return f.read().split('\n')

def calc_score(word):
    """Calculate the score of a given word."""
    word_score = 0
    for x in word:
        word_score += scores[x]
    return word_score

This can be rewritten using a list comprehension.

def calculate_score(word):
    """Calculate the score of a given word."""
    return sum([SCORES[letter] for letter in word])

As you can see, we don't need to initialize an empty variable. Python is a self-documenting language.x isn't really a descriptive name, so I took the liberty to change it to letter. This one is a nitpick really, but I also change the function name to calculate_score.

Staying on comprehensions, you can use a dictionary comprehension in dict_maker. Here, we create a list of letters using list comprehension and then return a dictionary containing the word and its score.

def dict_maker():
    """Create a dictionary of the words with their scores."""
    letters = [letter for letter in raw_input('Letters: ').lower()]
    return {word: calculate_score(word) for word in rack_check(letters)}

I'm a little short on time to look over the logic, so I will finish the rest later, but here's some quick observations.

In list_print you assigned a dictionary using dict as its name. This is fairly uncommon practice, as it shadows a builtin. More on builtins.
word_master = look_up() should be contained in rank_check function. Furthermore, you can even replace for item in word_master: with for item in look_up()


Raising a number to an integral power I'm currently learning Java along with the Stanford online "Programming Methodology" course and, while it is awesome, I do lack any kind of human feedback.

This exercise was to create a method that took a floating-point value x and an integer k and returned x to the power of k. The method needed to also be able to correctly calculate the result when k is negative using the relationship \$x^{-k} = \frac{1}{x^k}\$.

/*
 * File: RealPower.java
 * ---------------------
 * This Method takes a floating-point value 'x'
 * and an integer value 'k' and returns 'x to the power
 * of k'. Method is written to correctly calculate
 * even when 'k' is negative.
 * Program displays a table of values generated using
 * the raiseRealToPower method. where x = PI
 * and y is generated by 'i' variable in 'for' loop.
 * ---------------------
 * coded by Dartias
 */

import acm.program.*;

public class RealPower extends ConsoleProgram {

    public void run() {

        println("This program raises displays PI to the power of -4 through 4.");

/*
 * assigns 'x' the named constant value stored in PI
 */

        double x = PI;

/*
 * this loop uses the raiseToRealPower method to generate the
 * values for powers of -4 through 0.
 */

        for(int i = -4; i <= 0; i++){

            println(x + " to the power of "+ i + " is " + raiseRealToPower( x, i));
        }

/*
 * this loop uses the raiseToRealPower method to generate the
 * values for powers 1 through 4.
 */

        for(int i = 1; i <= 4; i++){

            println(x + " to the power of "+ i + " is " + raiseRealToPower( x, i));
        }

    }

/*
 * this method calculates x to the power of k.
 * in the event that 'k' is a negative number
 * the method uses the Math.abs method to convert it
 * to a positive number and then uses the formula
 * x(power of -k) = (1/x(power of k)) to ensure the
 * negative exponential powers generate correctly.
 */

private double raiseRealToPower(double x, int k){

    double total = x;

    if(k >=0){
        for(int i = 1; i < k; i++){
            total *= x;
        }} else {
            k = Math.abs(k);
            for(int i = 1; i < k; i++){
                total *= x;
            }
            total = (1/total);
        }
    return total;
    }

/*
 * defines the named constant PI and assigns it a value.
 */

private static final double PI = 3.14159265;

}

Your function fails to calculate \$x^0\$ correctly. I would expect it to return Double.NaN when \$x = 0\$, and 1 otherwise.


Math quiz for teachers & students I have been programming a maths quiz that can be used for teachers and I have have been trying to make the code as short as possible so it is easier to understand.

If there is any way I could make it more concise, please tell me and explain the programming behind it.

import sys
import random

def get_bool_input(prompt=''):
    while True:
        val = input(prompt).lower()
        if val == 'yes':
            return True
        elif val == 'no':
            return False
        else:
            sys.exit("Not a valid input (yes/no is expected) please try again")

status = input("Are you a teacher or student? Press 1 if you are a student or 2 if you are a teacher")

if status == "1":
    score=0
    name=input("What is your name?")
    print ("Alright",name,"welcome to your maths quiz")

    level_of_difficulty = int(input(("What level of difficulty are you working at?\n"
                                 "Press 1 for low, 2 for intermediate "
                                    "or 3 for high\n")))
    if level_of_difficulty not in (1,2,3):
        sys.exit("That is not a valid level of difficulty, please try again")

    if level_of_difficulty == 3:
        ops = ['+', '-', '*', '/']
    else:
        ops = ['+', '-', '*']

    for question_num in range(1, 11):
        if level_of_difficulty == 1:
            number_1 = random.randrange(1, 10)
            number_2 = random.randrange(1, 10)
        else:
            number_1 = random.randrange(1, 20)
            number_2 = random.randrange(1, 20)

        operation = random.choice(ops)
        maths = round(eval(str(number_1) + operation + str(number_2)),5)
        print('\nQuestion number: {}'.format(question_num))
        print ("The question is",number_1,operation,number_2)

        answer = float(input("What is your answer: "))
        if answer == maths:
            print("Correct")
            score = score + 1
        else:
            print ("Incorrect. The actual answer is",maths)

    if score >5:
        print("Well done you scored",score,"out of 10")
    else:
        print("Unfortunately you only scored",score,"out of 10. Better luck next time")

    class_number = input("Before your score is saved ,are you in class 1, 2 or 3? Press the matching number")
    if class_number not in ("1","2","3"):
        sys.exit("That is not a valid class, unfortunately your score cannot be saved, please try again")
    else:
        filename = (class_number + "txt")

        with open(filename, 'a') as f:
            f.write("\n" + str(name) + " scored " + str(score) +  " on difficulty level " + str(level_of_difficulty))

        with open(filename, 'a') as f:
            f = open(filename, "r")
            lines = [line for line in f if line.strip()]
            f.close()
            lines.sort()

        if get_bool_input("Do you wish to view previous results for your class"):
            for line in lines:
                print (line)
        else:
            sys.exit("Thanks for taking part in the quiz, your teacher should discuss your score with you later")

if status == "2":
    class_number = input("Which classes scores would you like to see? Press 1 for class 1, 2 for class 2 or 3 for class 3")
    if class_number not in (1,2,3):
        sys.exit("That is not a valid class")
    filename = (class_number + "txt")
    with open(filename, 'a') as f:
        f = open(filename, "r")
        lines = [line for line in f if line.strip()]
        f.close()
        lines.sort()
        for line in lines:
            print (line)

First thoughts:

The user is prompted to enter the class number after taking the test, and if it's not a valid number the program exits. This means they lose the results of the test. You could ask that question up front.

There's a large gap between the check to see if status is '1' and to see if status is '2'. I'd put the code for the student and teacher in functions, so that the result looks more readable:

def student_questions():
    pass

def teacher_questions():
    pass

if status == "1":
     student_questions()
elif status == "2":
     teacher_questions()

There's a lot of prompting the user for input and then checking to see if it's valid - that could easily be a function.

def ask_question(question, valid_answers):
    while ...
         print ...
         input ...
         if input is valid: return input

The only other comment I'd make straight away is that while choosing the operation at random works, it does mean that one student might get all divisions, others get a broad selection, and so on. Perhaps there needs to be a check to make sure that each student gets at least one of every type? Or a non-random selection?


How do I make this function faster? I have all large file of all the order sold in one week. This file gives one line for every order. We have over 5000 orders a day. It read the file line by line and then adds  the sale to a database of sales. to I really need to boost performance.

   static void sold4weeks1()
    {
        string sold, asin;
        string[] lines = System.IO.File.ReadAllLines(@"C:\out\qqqqq.txt");

        inventoryBLL u = new inventoryBLL();
        try
        {
            foreach (string line in lines)
            {

                char[] tabs = { '\t' };
                string[] words = line.Split(tabs);
                asin = words[12];
                sold = words[14];

                if (words[0].Substring(0, 3) == "S01") continue;
                try
                {
                    if (words[4] == "Shipped" || words[4] == "Unshipped")
                    {

                        u.setSoldin28(asin, Convert.ToInt16(sold));
                        Console.WriteLine("Update");
                    }
                }
                catch
                {
                }
            }
        }
        catch
        { }
    }

Is u.setSoldin28(asin, Convert.ToInt16(sold)); writing to the database?

Save up all your changes and make them all under a single connection when the parse is finished. That should save quite a bit of time.

You have alot of other issues with this code though. I strongly urge you to look into proper exception handling techniques.


A faster way to look up for a url in a WebRequest/WebResponse html What I'm trying to do here is get an RSS feed and append an Enclosure XML node to each item, which has a link to a video file (wmv).

Try the below code with

url = "http://www.microsoft.com/events/series/digitalblackbelt.aspx?tab=rss"

for eg to get the point

The performance hit here consists of two parts: the LONG "foreach" loop, along with the HUGE number of requests done to retrieve the media link (marked in the code by the comment "This part needs attention"). Any advice concerning how to get the media link a much faster manner would really be appreciated!

You can get a glimpse of what that code do, by comparing this feed (the URL):

http://www.microsoft.com/events/series/digitalblackbelt.aspx?tab=rss

to this feed (created by the code below), note that it will be slow and might give error, if it did just refresh the page:

http://mshady.apphb.com/feeds/index?url=http://www.microsoft.com/events/series/digitalblackbelt.aspx?tab=rss

public class FeedsController : Controller
{
    private XmlDocument _Xml;

    [OutputCache(VaryByParam = "url", Duration=86400)]
    public string Index(string url)
    {
        string feed = "";

        _Xml = new XmlDocument();
        _Xml.Load(url);

        foreach (XmlNode node in _Xml.ChildNodes[0].ChildNodes[0].ChildNodes)
        {
            if (node.Name == "item")
            {
                XmlNode childnode = GetEnclosureXmlNode(node);
                node.AppendChild(childnode);
            }
        }

        feed =  _Xml.OuterXml;

        return feed;
    }

    private string GetMediaUrl(string articleUrl)
    {
        string mediaUrl = "";

        ////////////////////////////////////////////
        // This part needs attention
        ////////////////////////////////////////////
        Regex regex = new Regex(@"WMV Download");
        var request = HttpWebRequest.Create(articleUrl);
        var response = request.GetResponse();
        string responseHtml = new StreamReader(response.GetResponseStream()).ReadToEnd();
        var match = regex.Match(responseHtml);
        ////////////////////////////////////////////
        // This part needs attention
        ////////////////////////////////////////////

        if (match.Success)
            mediaUrl = match.Groups[1].Value;

        return mediaUrl;
    }

    private XmlNode GetEnclosureXmlNode(XmlNode node)
    {
        string articleUrl = node["link"].InnerText;
        string mediaUrl = GetMediaUrl(articleUrl);

        XmlElement childnode = CreateEnclosureXmlNode(mediaUrl);
        return childnode;
    }

    private XmlElement CreateEnclosureXmlNode(string mediaUrl)
    {
        XmlElement childnode = _Xml.CreateElement("enclosure");
        childnode.SetAttribute("url", mediaUrl);
        childnode.SetAttribute("type", "video/wmv");
        return childnode;
    }
}

I would bet that the regex.Match is the performance problem, as Jeff Atwood describes here.
One thing that I would do is move regex definition to a static variable outside of the function defined as below:
static Regex regex = new Regex(@"WMV Download", RegexOptions.Compiled);

That's the only real thing that jumps out at me. Try taking a look at this blog post for more ideas and information.
Should give you something to try until guys with more experience glance their eyes over it :)


Coordinating camera stage movement and data readout The purpose of this code is important, so here's a quick rundown.

This code runs controls a physical machine which utilizes a camera and a moving stage holding an object to be imaged.  Imaging can be broken down into two parts; exposing (collecting photons) and readout (reading the formatted image from the camera over USB).

Stage moves are performed asynchronously.  The object being imaged is broken up into many (700+) "tiles" which must be imaged separately, so one move + expose + readout for each tile.

An optimization which saves ~3 minutes per imaging operation is to begin moving to tile N+1 after exposing tile N but before readout has completed.  We then wait for readout to complete, expose the current tile, and repeat the process again.

This leads to a loop structure that, while correct, is a bit confusing.  It's not a horrible piece of code, but it's been bugging me and I'd like to spend some time today refactoring it.  Problem is, I haven't come up with any great ideas.

Here's the loop structure:

void ScanLoopExample()
{
    var tiles = GetAllTiles();
    var firstTile = tiles.First();

    // get into position for the first tile
    MoveStageAsync(firstTile.X, firstTile.Y, firstTile.Z);

    foreach(var nextTile in tiles.Skip(1))
    {
        // wait for the move to complete
        WaitForMotionEnd();

        // expose over the first tile
        ExposeTile();

        // get into position for the next tile
        // we *must* make this move right here in
        // order to save time moving while the camera is busy
        MoveStageAsync(nextTile.X, nextTile.Y, nextTile.Z);

        // wait for the image to arrive from the camera,
        // we cannot expose again until this completes
        WaitForReadout();
    }

    // because we only moved to the last tile
    // without capturing, capture it here
    WaitForMotionEnd();
    ExposeTile();
    WaitForReadout();
}

Like I said, not the worst code ever, but there are a couple of things I don't like:

I don't like that part of the logic is separated from the loop itself (the move to the first tile).
I don't like that the foreach variable is the tile to move to, not the tile being captured.  It has slightly confused two coworkers I showed it to (though they got it quick enough after I explained the process).
I don't like that the capture code (wait + expose + readout) must be repeated after the loop is finished.

I've tried a couple of variants, but nothing I have come up with can get all of the capture logic in the body of the loop.  This is a bit of an obsessive need to make the code as pretty as possible, but hey, it's the weekend and I'm not on the clock.

The tricky bit is that we must begin the asynchronous move of the stage after exposure but before readout.  That places a strict rule on the ordering of operations in the loop.

Since what matters here is the ordering of the operations, not what the operations mean, I'm going to use these abbreviations:

A - MoveStageAsync
B - WaitForMotionEnd
C - ExposeTile
D - WaitForReadout

(I'm trying to explain how I got my version using these abbreviations, if you find them confusing, skip to the code.)

So with 3 iterations, your code is doing this: ABCADBCADBCD, and the naive, slow version you optimized from did this: ABCDABCDABCD.  If we wrap parentheses around the part that is in the loop body, we can abbreviate what your code is doing as A(BCAD)BCD and the unoptimized version as (ABCD).  Assuming that we have to keep the same order as what your code is currently doing, we can shift what's in the body of the loop slightly to get AB(CADB)CD and ABC(ADBC)D.  There isn't a way to shift the loop all the way to the beginning or end and do things in the same order you're currently doing them in.

If we try to find an ordering that lets us just toss the whole thing in a loop (like (ABCD), only efficient), we have to move to the first location first (nothing else makes sense), we have to wait for the motion to end next (nothing else makes sense, again), then we have to expose before we can wait for the exposure.  So the only way to do it with everything in the loop was the original, slow version.

So we have 3 possibilities: A(BCAD)BCD, AB(CADB)CD, and ABC(ADBC)D.  If we look at what those 3 would mean, I think AB(CADB)CD is the most readable.

Here it is as code:

MoveStageAsync(firstTile.X, firstTile.Y, firstTile.Z);
WaitForMotionEnd();

foreach(var nextTile in tiles.Skip(1))
{
    ExposeTile();

    MoveStageAsync(nextTile.X, nextTile.Y, nextTile.Z);

    WaitForReadout();
    WaitForMotionEnd();
}

ExposeTile();
WaitForReadout();

There are a few nice things about it: the setup and teardown around the loop both make sense as standalone actions, the waits in the loop body are both next to each other, and the loop body as a whole makes sense as a standalone action.  Although the loop index is a tile, it's really a loop over the transitions from one tile to another.


Changing the prototype of the String object Are there any pitfalls in changing the prototype of the String object? I'd like to change the word substr to "left":

String.prototype.left = function(n) {
    return this.substr(0,n)
}

Is this ok, or should I just get over it?

Next, I'm going to write a .right(), a .mid(), a .val() and a .int().

Extending native objects is bad practice.

When you're expecting nothing, then something shows up. That's natural behavior of prototypes.
Forwards compatibility. The same reason why Object.keys(someObjectInstance) is designed that way instead of someObjectInstance.keys() - because some implementations may already have used keys as a property. So what if ECMAScript 9 wants to use left() and right() for some string operation, your app will break.

For alternatives:

You can create your own functions in a separate namespace, that looks like the native functions. For instance, the native forEach looks like array.forEach(fn). Underscore writes it differently with _.forEach(array,fn).
You can extend native objects. Mileage may vary on this one as some (not sure which, but most) native objects are not subclassable.
You could create "wrapper objects" that just wrap the object into another object, who has methods that operate on the wrapped object. A good example is jQuery which wraps an array of stuff in a "jQuery object" which hosts a myriad of DOM manipulation methods that operate on that set.
Create a custom object that doesn't necessarily extend/inherit/wrap anything. Something like var string = new CustomString('hello world').


Pattern matching in an array or collection If I had an array of integers like

+--------------------------------------------+
| 1| 0| 1| 2| 0| 1| 0| 1| 0|-1| 0| 2| 0|-2| 0|  > VALUES
+--------------------------------------------+
| 0| 1| 2| 3| 4| 5| 6| 7| 8| 9|10|11|12|13|14|  > INDEX
+--------------------------------------------+

I am looking for 0 which occurs 7 times in the array above.

But I need to find indexes of 0s in the array like so:

I can find 0's that occur only after a slot with a Abs(value) > 1
(rising of falling)
I can find 0's that occur only after a slot with a value > 1
(rising)
I can find 0's that occur only after slot with a negative Abs(value)

  1 (falling)

So, looking at array above my logic should show indexes

1, 4, 12, 14 - 1st and REST (rising or falling)
1, 4, 12     - 1st and RISING
1, 14        - 1st and FALLING

The indexes above are valid since the previous value was (Abs(previous) > 1). On the other hand, 0's at index 6, 8, and 10 are of no interest since the previous array value was either 1 or -1 (so Abs(previous) <= 1)

My code below works but I feel there could be a simpler more elegant solution to this

public static void Main(string[] args)
{
    int[] array = new int[15] { 1, 0, 1, 2, 0, 1, 0, 1, 0, -1, 0, 2, 0, -2, 0 };

    bool found1 = true;
    for (int i = 0; i < array.Length; i++)
    {
        if (array[i] == 0)
        {
            if (found1)
            {
                Console.WriteLine(i); //print 1st index of 0
                found1 = false;
            }
            else
            {
                if (Math.Abs(array[i - 1] - array[i]) > 1)
                {
                    Console.WriteLine("All: " + i);  // print every other that fits
                    //1, 4, 12, 14
                }
            }
        }
    }

    found1 = true;
    for (int i = 0; i < array.Length; i++)
    {
        if (array[i] == 0)
        {
            if (found1)
            {
                Console.WriteLine(i); //print 1st index of 0
                found1 = false;
            }
            else
            {
                if (Math.Abs(array[i - 1] - array[i]) > 1
                    && (array[i - 1] - array[i]) > 1)
                {
                    Console.WriteLine("Up: " + i);  // print rising
                                                    //1, 4, 12
                }
            }
        }
    }

    found1 = true;
    for (int i = 0; i < array.Length; i++)
    {
        if (array[i] == 0)
        {
            if (found1)
            {
                Console.WriteLine(i); //print 1st index of 0
                found1 = false;
            }
            else
            {
                if (Math.Abs(array[i - 1] - array[i]) > 1
                    && (array[i - 1] - array[i]) < 1)
                {
                    Console.WriteLine("Down: " + i);  // print falling
                                                      //1, 14
                }
            }
        }
    }

    Console.ReadLine();
}

You are unnecessarily looping through the array 3 times (only one loop is required). Its not clear how you will be using the results, but assuming you need to store them, then the code can be simplified to

int[] array = new int[15] { 1, 0, 1, 2, 0, 1, 0, 1, 0, -1, 0, 2, 0, -2, 0 };
// Define 3 lists from the results
List<int> all = new List<int>();
List<int> up = new List<int>();
List<int> down = new List<int>();
// Get the index of the first zero value and add to the lists
int first = Array.IndexOf(array, 0);
all.Add(first);
up.Add(first);
down.Add(first);
// Loop the remaining items in the array and add to the appropriate lists
for (int i = first + 1; i < array.Length; i++)
{
    int current = array[i];
    int previous = array[i - 1];
    if (current != 0)
    {
        continue;
    }
    if (Math.Abs(previous - current) > 1)
    {
        all.Add(i);
        if (previous - current > 1)
        {
            up.Add(i);
        }
        else if (previous - current < 1)
        {
            down.Add(i);
        }
    }
}

The result is

all contains [1, 4, 12, 14], up contains [1, 4, 12] and down contains [1, 14]


C++17 enum macro with to_string operator I created a simple enum macro that creates a to_string method for the enum.  This has been done before, but my version is designed to compile quickly.  From what I can tell, existing libraries use deep preprocessor macros and lots of template instantiations which is taxing for large enums.
Here is the code:
#include <array>
#include <string_view>

#define EnumWithUnderlying(Name, NTy, ... )\
enum class Name : NTy {__VA_ARGS__, End };\
namespace std {\
    std::string_view to_string(enum Name enumIn) noexcept {\
        constexpr static string_view fullstr = #__VA_ARGS__ ", End ";\
        struct BeginLen {\
            std::size_t begin;\
            std::size_t len;\
        };\
        using ArrIdxTy = std::array<BeginLen, NTy(Name::End)+1>;\
        constexpr static ArrIdxTy begins = [&]() {\
            ArrIdxTy ret = {};\
            std::size_t jbegin = 0;\
            std::size_t jend = 0;\
            enum ParserState {Before,In,After};\
            ParserState state = Before;\
            for (std::size_t i = 0; i < fullstr.size(); ++i) {\
                auto isSpace = [](char ch) -> bool {\
                    return ch == ' ' || ch == '\t' || ch == '\n';\
                };\
                switch (state) {\
                    case Before:\
                        if (!isSpace(fullstr[i])) {\
                            ret[jbegin].begin = i;\
                            jbegin++;\
                            state = In;\
                        } else {\
                            break;\
                        }\
                    case In:\
                        if (isSpace(fullstr[i]) || fullstr[i] == ',') {\
                            ret[jend].len = i - ret[jend].begin;\
                            jend++;\
                            state = fullstr[i] == ',' ? Before : After;\
                        } else {\
                            break;\
                        }\
                        break;\
                    case After:\
                        if (fullstr[i] == ',') {\
                            state = Before;\
                        }\
                        break;\
                }\
            }\
            return ret;\
        }();\
        using ArrStrTy = std::array<std::string_view, NTy(Name::End)+1>;\
        constexpr static ArrStrTy strs = [&]() {\
            ArrStrTy ret = {};\
            for (std::size_t i = 0; i < begins.size(); ++i) {\
                ret[i] = std::string_view(fullstr.begin() + begins[i].begin, begins[i].len);\
            }\
            return ret;\
        }();\
        return strs[NTy(enumIn)];\
    };\
}
#define Enum(Name,...) EnumWithUnderlying(Name, int, __VA_ARGS__)

Example usage:
#include <iostream>
#include "enum.h"

EnumWithUnderlying(Move, char,
  UP,
  DOWN,
  LEFT,
  RIGHT
);

Enum(Letter, A, B);

int main() {
std::cout <<
    std::to_string(Move::UP) << " " <<
    std::to_string(Move::DOWN) << " " <<
    std::to_string(Move::LEFT) << " " <<
    std::to_string(Move::RIGHT) << " " <<
    int(Move::End) << " " <<
    std::to_string(Letter::A) << " " <<
    std::to_string(Letter::B) << " " <<
    std::endl;
}

I'm open to any comments, but I don't want to do anything that makes it take longer to compile.
Also, I noticed that making to_string constexpr/getting rid of the static members seems to generate worse code.  I don't fully understand why that is since I'd expect a constexpr version to be strictly faster.
For the record this is also on github

I’m really not a fan of this idea, on a lot of levels. First of all, it’s extremely limited; it only works for the most basic enum definitions. As soon as you add initializers or attributes, chaos ensues. Second, it’s just absurdly overcomplicated for what should be trivial code. The fact that you’re having problems with code gen should be a warning sign. There is no way I could justify the debugging effort if I tried to use this and something went sideways.
In my opinion, manually rolling out a to_string() function for an enum isn’t that much extra work. Certainly not enough to justify the level of complexity of this macro. And if your primary concern is compile time (really? that’s the most important thing? how many enums do you have, and how big are they that this is even measurable, let alone the most crucial issue?), then a manually rolled function will compile MUCH faster than the code in that macro. Yeah, sure, you have to repeat yourself a little bit… but not that much. Certainly not enough to justify a state machine parser for a string (that also only works in the simplest cases).
But okay, let me set aside my qualms about the overall idea, and focus on the code itself.
#define EnumWithUnderlying(Name, NTy, ... )\
enum class Name : NTy {__VA_ARGS__, End };\

You don’t include any comments or documentation to explain the logic of the macro, so I don’t know if this extra End enumerator is a feature or a bug. In other words, I don’t know if there is a reason someone might want End added to their enumerators, or if you just added it to make your code work.
Assuming it’s just there to make things work… the only purpose I can make out for it is that you use it to figure out the size of your enumerator arrays, such as in:
using ArrIdxTy = std::array<BeginLen, NTy(Name::End)+1>;

This “works” only so long as none of the enumerators have any initializers (that don’t happen to equal what their default values would be), but aside from being brittle, it’s also unnecessary. If all you need is the count of initializers, then all you need is something as simple as:
template <typename... Args>
constexpr auto count_args(Args&&...) noexcept
{
    return sizeof...(Args);
}

using ArrIdxTy = std::array<BeginLen, count_args(__VA_ARGS__) + 1>;

If End serves some other purpose, then fine, there’s no need to remove it… but in that case you still might want to get the number of enumerators using a count function like the one above. That way your macro won’t break if any enumerators have initialzers. (Well, it won’t break in that particular way, anyway.)
namespace std {\
    std::string_view to_string(enum Name enumIn) noexcept {\

There are a very limited number of cases where you are allowed to add stuff to std—really just a few templates that you are allowed to specialize in fairly restricted ways. Adding a completely new function? Definitely not allowed. No, not even if it’s an overload of an existing function.
To make matters worse, the function lies. When I call to_string()… I expect to get a string. Not a view to a string.
I figure that your goal is to be able to write std::to_string(enum_value)… but that goal is misguided. When I see std::some_func() that I’ve never seen before, the very first thing I do is go straight to my favourite standard library reference (cppreference) and read the docs. But I won’t find this function there, now will I?
You might respond: “But it does the same thing std::to_string() does for all the types it is defined for!” Does it, though? If I do auto x = Move::UP; std::cout << std::to_string(x);, it will print “UP”. But if I do auto x = SOME_INT_CONSTANT; std::to_string(x); it will not print “SOME_INT_CONSTANT”. std::to_string(int) prints the (locale-flavoured) value of an int… not its name… yet your enumerator-specific std::to_string() functions print the name. Not the same thing, now is it?
You might respond: “But what it does is so obvious!” Is it, though? I mean, ignoring that it actually doesn’t do what it claims to (it returns a std::string_view, not a std::string), converting an enumerator value to a string doesn’t necessarily mean returning the enumerator’s name. It might actually make more sense to return the enumerator’s numeric value as a string in some cases.
So in summary:

std::to_string() is the wrong name for this function; and
even if it weren’t, you can’t use it.

So what’s the right name? Well, why not just to_string() in the same namespace as the enum? (Well, better would be to_string_view(), but one thing at a time.) ADL would find the function when you need it, so you’d only have to call to_string(enum_value)… which is actually shorter than std::to_string(enum_value).
(It might also make sense to define a to_string() function in terms of a to_string_view() function. That’s actually how I usually do it.)
Now to get into the meat of the code… what this does (as I understand it) is take the variadic macro arguments, translate the bunch of them into a single string, then use a state machine to parse out the enumerator names into an array. As I mentioned above, your parser is too simplistic—it can’t handle initializers or attributes—but let’s ignore that issue for now.
The biggest problem with your strategy is the assumption that all the enumerators in an enumeration have their default “natural” values. If they don’t, you probably have UB, because your enumerator values won’t be valid indexes into the array.
A less critical issue is that if you really only want to support trivial enumerations (that is, no initializers), that’s fine… but in that case, this seems wildly over-complicated. All you need to do to find the Nth enumerator is find the (N−1)th comma and the Nth comma, take everything between them, then trim the whitespace. Special case the first and last enumerators (or don’t! add a leading and trailing comma to the string to simplify the algorithm!), and you’re done.
If you intend to support general enumerations, then this design just won’t work: you can’t simply cast the enumerator to a number and use that as an index. You need a map. No, not a std::map; that would be overkill. std::array<std::tuple<Name, std::string_view>> would suffice. To actually implement this, all you’d need is some kind of preprocessor for-each—either roll your own or use one from a library—then you could just do something like this:
// Takes an arbitrary enumerator string with arbitrary whitespace:
//     *   "foo"
//     *   "foo = N"
//     *   "foo [[attribute]]"
//     *   "foo [[attribute]] = N"
// and extracts just the enumerator ("foo").
constexpr auto extract_enumerator(std::string_view) -> std::string_view;

#define ENUM_STR_MAP_ITEM(enumerator) std::tuple{enumerator, extract_enumerator(#enumerator)},

constexpr auto enumerator_string_map = std::array{
    FOREACH(ENUM_STR_MAP_ITEM, __VA_ARGS__)
};

And to_string_view() could use that map.
struct BeginLen {\
    std::size_t begin;\
    std::size_t len;\
};\

So, your strategy is to parse the string into an array of BeginLen objects, then transform that into an array of std::string_view. But… why? A std::string_view is literally just a “begin” and a “len” already. The entire last quarter of your function is literally just a transform that does std::string_view{begin, len} for each BeginLen. Why not parse the enumerator strings directly into std::string_views and skip that last bit?
std::size_t jbegin = 0;\
std::size_t jend = 0;\

Why do you need two indexes to keep track of which enumerator you’re currently parsing? (I mean, I assume that’s what those variables are for, in lieu of any comments or even clear names.) Couldn’t you drop jend, remove the jbegin++;, and replace jend with jbegin in the In case?
Summary
You have a number of critical bugs and conformance issues:

You are not allowed to add new functions to namespace std. (An overload of an existing function is still a new function.)
Your method for determining the number of enumerators is broken. (It only works for trivial enums, and it adds a new enumerator, which is not ideal (unless there’s another reason for it?).)
Your method for mapping enumerators to their strings is broken. (You assume a trivial enum, where you can cast the enumerator value to an index.)
Your method for parsing enumerator strings will only work properly for trivial enums. (For non-trivial enums it will return weird results, but technically (so far as I can tell) won’t actually trigger UB.)

I know your primary performance concern is the compile time, but just in case you do care about run time performance… this code is extremely inefficient for what it actually does. It sounds like you don’t care; you figure it will all be constexpred away. Well, maybe, maybe not. There doesn’t seem to be any real reason not to make it more efficient in either case.
Style-wise, my biggest complaint would have to be the complete lack of any commenting or documentation. I know commenting a macro isn’t the easiest thing to do. But it’s still worth it; especially for a macro as complicated as this.
It also doesn’t help that most of your identifiers are terrible. jbegin? What’s “j”? ArrStrTy? Whut? A struct that contains only the members begin and len being called BeginLen? That’s really not helpful at all. A function called to_string() that doesn’t actually convert to a string?
The other thing that raises my hackles is that your macros are mixed-case. That’s just BEGGING for trouble. (Even more alarming is that you use mixed-case macros WHILE ALSO USING MIXED-CASE IDENTIFIERS!!! There’s a point where you’ve loaded a gun, pointed it at your foot, taken the safety off, and started feathering the trigger, and if you end up blowing your foot off, no one can possibly take you seriously for being surprised.)
(I’d also raise complaints about the header file being named “enum.h”. “.h” is for C header files. For C++ header files, you should use “.hpp”, “.hxx”, “.hh”, or something similar.)
Overall design-wise… I might be convinced of the utility of a make-an-enum-with-a-built-in-to_string macro. But not if that macro is as complex as this. A macro should make my code easier… if a macro makes things harder (to understand, to debug), then it’s not worth it. If you could make this macro much, MUCH simpler (I have to figure out an ad hoc state machine just to get enumerator strings?!), then it might be worthwhile.


Architecture and error handling in simple java program I'm trying to get my basic coding principles right but there's still a long way to go. This is an exercise for CodeGym. They logic itself isn't very hard and I passed the exercise easily but I'd like my code reviewed with the following in mind:

In terms of the general architecture, I wasn't quite sure if I needed two methods. It felt tidier to me to have a method for the first task (copy) and one for the second (append) but I couldn't figure out a way to avoid calling the second method from the first.
Should read the inputs from the console in the main method and pass the result to the class? Or it totally depends on the context of the application I'm working for?
About error handling, I got completely lost. After doing some research I just decided to let the caller handle the error but I'm sure there's some stuff missing. I wasn't sure for example if I should have two try blocks on the first method but it also depended on the answer to the first point (architecture), so I gave up.
It's my first time using Javadocs, so I'm not sure what I wrote is enough
I know "Solution" is not the ideal name for the class, but I used the class created by CodeGym.

Please feel free to tear it apart :)

Thanks in advance!

public class Solution {

    public static void main(String[] args)  {
        FileProcessor fileProcessor = new FileProcessor();
        try {
            fileProcessor.copyFile();
        } catch (IOException e) {
            System.out.println("General I/O exception: " + e.getMessage());
            e.printStackTrace();
        }
    }

import java.io.*;

/**
 *  This class reads 3 filenames from console,
 *  copies content from the second file to the first and subsequently
 *  appends content from third file to first file.
 */

public class FileProcessor {
    // Study comment: Solution using "var" didn't work in this IntelliJ
    /**
     * Reads inputs, copies second file to first and calls method to append file
     * @throws IOException
     */
    public void copyFile() throws IOException {
        String outputFile;
        String sourceToCopy;
        String sourceToAppend;

         try (BufferedReader reader = new BufferedReader(new InputStreamReader(System.in))) {
             System.out.println("Enter a filename for the output:");
             outputFile = reader.readLine();
             System.out.println("Enter the path of the file to be copied");
             sourceToCopy = reader.readLine();
             System.out.println("Enter the path of the file to be appended");
             sourceToAppend = reader.readLine();
         }

        try (FileOutputStream out = new FileOutputStream(outputFile);
             FileInputStream copy = new FileInputStream(sourceToCopy)) {
            byte[] buffer = new byte[1024];
            while (copy.available() > 0) {
                int count = copy.read(buffer);
                out.write(buffer, 0, count);
            }
        }

        appendFile(sourceToAppend, outputFile);
    }

    /**
     *
     * @param source file to append
     * @param output final result file
     * @throws IOException
     */
    public void appendFile(String source, String output) throws IOException {
        try (FileOutputStream appended = new FileOutputStream(output, true);
             FileInputStream toAppend = new FileInputStream(source)
        ) {
            byte[] buffer = new byte[1024];
            while (toAppend.available() > 0) {
                int count = toAppend.read(buffer);
                appended.write(buffer, 0, count);
            }
        }
    }
}

General architecture

A useful architecture guideline is:

By introducing classes and methods, I create the high-level language that I want
to use for expressing my solution.

So, It's a good idea to start from the most abstract top-level, and later implement the methods needed at that top-level code. So I'd like to rephrase your program (top-level) as follows:

Read the output filename: String outputFile = readWithPrompt(reader, "Enter a filename for the output:");
Read the first input filename: String sourceToCopy = readWithPrompt(reader, "Enter the path of the file to be copied");
Read the second input filename: String sourceToAppend = readWithPrompt(reader, "Enter the path of the file to be appended");
Combine the files: combineFiles(sourceToCopy, sourceToAppend, outputFile);

This keeps user interface and business logic apart, which I highly recommend!

Reading the three file names is three times the same, so I used a common phrase (= calling the same method, just with different prompting strings).

The main business of your program is to combine two input files into one output file, so I made that explicit at the top-level.

I like your distinct append() method. It's useful on its own, not only in your combine-two-files setting. You could even implement the whole thing by first creating an empty file, and then twice append some source file to it.

Where to read user input

Always separate user interface from doing things. Imagine that tomorrow you want to combine all files from one directory with a common footer (e.g. your company's legal stuff). Then you no longer want to ask the user for every single file, but you'll still be combining pairs of input files into an output. You can then easily re-use a clean business method, but not one mixing user I/O and business.

Error handling

I like your exception handling in the main method.

The single most important guideline is: A method should throw an exception if it couldn't do its job.

So, if during the files copying something went wrong, you ended up with a missing or corrupt result. The caller of combineFiles() can normally assume that after that call, the output file has the correct content. If not, he needs to know about that, and that's done by throwing an exception (or not catching an exception that came from some inner calls).

Exception handling is a big topic with diverging opinions between the professionals, too much to go into more details here. As long as you follow the guideline, you can't be going wrong.

Javadoc

Your Jacadocs are mostly fine. I wish most real-world programs were documented like that. Sigh!

  Reads inputs, copies second file to first and calls method to append file

Here's one quirk: you say "calls method", and that's breaking encapsulation. Javadocs should describe a method as a black box, how it behaves to the outside world. For the Javadoc, it doesn't matter if the method does its job all by itself or by calling hundreds of other methods. Only describe things that have an effect visible to your caller.

When describing the append() method, you thoughts were probably withing your overall task, so the text shows parts of what the append() method is meant for ("final result file") in the whole program instead of just describing what it does (append one file to the existing contents of another file).

Class names, packages

Nothing to complain about your class names.

"Solution" is not the most inventive, but a valid name for such a compact program.

"FileProcessor" is a perfect name for a class like yours.

But when you start to do serious stuff, you should get into the habit of organizing your classes into packages with unique names, with low risk of colliding with packages from external libraries, typically by using something like a reversed domain name or identity e.g. com.facebook.username, something that's unlikely to be used by someone else.

Finally, I'd like to warn about wildcard import statements like your import java.io.*;. This makes all class names from the java.io package visible in your code. At a first glance, that's fine, you can abbreviate java.io.FileOutputStream to just FileOutputStream in your code.

But maybe, in some future Java version, a class named FileProcessor gets included into the java.io package. Then the confusion starts: does FileProcessor mean your class or the one from java.io?

While exactly that is well-defined (it will be your class as long as you're in the same package), wildcard imports can cause real trouble in related, similar situations. Every decent, modern IDE has the ability to manage individual imports for you, so replacing the wildcard import with the necessary individual imports should be "one click" and then avoids the risk I described here.

Indentation

The indentation of your code looks mostly fine, but a few lines are out of sync. So probably, you did at least some of it manually. In your IDE, look for automated code indentation or formatting. No need to waste time hitting the space key multiple times, and as a result of the automated feature, your code always looks tidy.


Dynamically resizing panels based on browser window size I've just finished building my responsive carousel currently trying to optimize my code to make it more efficient. I've created a number of if conditions in my carousel, what does it sets the sizes of each elements based on the condition. Is there a way to reduce the number of if statements.

    var carouselWidth = 0,
        slider = $('.carousel .slider'),
        carouselContainerWidth = $('.carousel').width(),
        carouselPanel = $('.carousel .slider .panel'),
        browserWindow = $(window),
        sliderWidth = $('.carousel .slider').width();

    //Set Different Size of Image Panels based on Browser Size
    ///////////////////////////////////////////////////////////

    //Display 5 Panels
    if ($(browserWindow).width() >= 1500 ) {
        var setPanelWidth = carouselPanel.outerWidth( carouselContainerWidth / 5, true ),
            panelSize = setPanelWidth.outerWidth();
    }

    //Display 4 Panels
    if ($(browserWindow).width() < 1500) {
        var setPanelWidth = carouselPanel.outerWidth( carouselContainerWidth / 3, true ),
            panelSize = setPanelWidth.outerWidth();
    }

    //Display 3 Panels
    if ($(browserWindow).width() < 1120) {
        var setPanelWidth = carouselPanel.outerWidth( carouselContainerWidth / 2, true ),
            panelSize = setPanelWidth.outerWidth();
    }

    //Display 1 Panels
    if ($(browserWindow).width() < 768) {
        var setPanelWidth = carouselPanel.outerWidth( carouselContainerWidth / 1, true ),
            panelSize = setPanelWidth.outerWidth();
    }

The problem is basically calculating the number of panels.
I'm no expert in media queries, but this is how i would solve it using js:

function PanelCount(lowerBound, upperBound, panelCount) {
  this.lowerBound = lowerBound;
  this.upperBound = upperBound;
  this.panelCount = panelCount
  this.isApplicableToContainerWidth = function(containerWidth) {
    return lowerBound <= containerWidth&& containerWidth< upperBound;
  };
}

function getCorrectPanelCount(containerWidth) {
    var panelCounts = [];
    panelCounts.push(new PanelCount(1500, Number.MAX_SAFE_INTEGER, 5));
    panelCounts.push(new PanelCount(1120, 1500, 3));
    panelCounts.push(new PanelCount(768, 1120, 2));
    panelCounts.push(new PanelCount(0, 768, 1));

    for (i = 1; i < panelCounts.length; i++) {
      if (panelCounts[i].isApplicableToContainerWidth(containerWidth)) {
        return panelCounts[i].panelCount;
      }
    }
}

Naming needs some improvement though, PanelCount is a horrible class name


Parsing text file in C++ I have a text file that contains name, age, salary, hoursWorked, randomText and are filled with different delimiters.

Text file:

susan:25-2600,28[asd]
mary:21-2200,38[asd]
john:23-3400,46[asd]

Instead of breaking them into individual strings using the code shown below:

string name,age,salary,hoursWorked,randomText;
ifstream readFile("textfile.txt");

while(getline(readFile,line))   {
    stringstream iss(line);
    getline(iss, name, ':');
    getline(iss, age, '-');
    getline(iss, salary, ',');
    getline(iss, hoursWorked, '[');
    getline(iss, randomText, ']');
}
readFile.close();

What are some better strategies other than coding it this way?

Side note

I declared all the variables to strings because of the getline() method.

I would create a class and define an input operator:

struct Person
{
    std::string name;
    std::string age;
    std::string salary;
    std::string hoursWorked;
    std::string randomText;

    friend std::istream& operator>>(std::istream& str, Person& data)
    {
        std::string line;
        Person tmp;
        if (std::getline(str,line))
        {
            std::stringstream iss(line);
            if ( std::getline(iss, tmp.name, ':')        &&
                 std::getline(iss, tmp.age, '-')         &&
                 std::getline(iss, tmp.salary, ',')      &&
                 std::getline(iss, tmp.hoursWorked, '[') &&
                 std::getline(iss, tmp.randomText, ']'))
             {
                 /* OK: All read operations worked */
                 data.swap(tmp);  // C++03 as this answer was written a long time ago.
             }
             else
             {
                 // One operation failed.
                 // So set the state on the main stream
                 // to indicate failure.
                 str.setstate(std::ios::failbit);
             }
        }
        return str;
    }
    void swap(Person& other) throws() // C++03 as this answer was written a long time ago.
    {
        swap(name,        other.name);
        swap(age,         other.age);
        swap(salary,      other.salary);
        swap(hoursWorked, other.hoursWorked);
        swap(randomText,  other.randomText)
    }
};

Now your code looks like this:

Person   data;
while(readFile >> data)
{
    // Do Stuff
}

PS. I noticed you were using string and ifstream without the std::. This suggests you have using namespace std; in your code. Please don't do that. see Why is “using namespace std;” considered bad practice?

Don't explictly close() a file unless you are going to check that it worked (or are going the re-open). Prefer to let the destructor do the closing (that way it is exception safe). See: My C++ code involving an fstream failed review