Optimizing Laravel Eloquent multiple joins The following is working and readable SQL for Eloquent that I have created. The downside is that it looks more like a 1:1 with SQL and I don't think I'm using the full potential of Eloquent. I thought of using scope queries to shorten it, but for some reasons the selectRaw prevents it (due to multiple join variable scopes).

Data is scattered across three tables and modifying the tables is not an option.

Notes:

$genreIdList is an array of IDs,
$limit is the max number of results requested,
$offset is the starting point of the pull,
selectRaw is necessary as the selected columns are needed.

Here is the original SQL:

SELECT
b.title,
GROUP_CONCAT(DISTINCT(g.name) ORDER BY relevancy, g.name ASC SEPARATOR ' ') as genre_list,
GROUP_CONCAT(DISTINCT CAST(relevancy as char) ORDER BY relevancy ASC SEPARATOR ',') as genre_relevancy,
bg.book_id
FROM book_genres bg
INNER JOIN genre_book g ON bg.genre_id = g.id AND g.id IN (". (($genreIdList) ? $genreIdList : 0) . ")
INNER JOIN book b ON bg.book_id = b.id
INNER JOIN licensors l ON b.licensor_id = l.id
WHERE b.status = 'active' AND b.premium = 'yes' AND l.status = 'active'
GROUP BY bg.book_id

This is the PHP/Eloquent version:

    $relatedBooks = Book::selectRaw("book.title,
        GROUP_CONCAT(DISTINCT(gb.name) ORDER BY bg.relevancy, gb.name ASC SEPARATOR ' ') as genre_list,
         GROUP_CONCAT(DISTINCT CAST(bg.relevancy as char) ORDER BY bg.relevancy ASC SEPARATOR ',') as genre_relevancy,
         bg.book_id ")
        ->join('book_genres AS bg', 'bg.book_id', '=', 'book.id')
        ->join('genre_book AS gb', 'gb.id', '=', 'bg.genre_id')
        ->join('licensors AS l', 'l.id', '=', 'book.licensor_id')
        ->whereIn('gb.id', $genreIdList)
        ->where('book.premium', '=', 'yes')
        ->where('book.status', '=', 'active')
        ->where('l.status', '=', 'active')
        ->take($limit)
        ->offset($offset)
        ->get();

SQL formatting

Your SQL script in my opinion could use some indentation and a few more line breaks.

Variable names

Table & column aliases are useful. But it is a good practice to use a variable name that at least says something about what the table contains.

Consistency

SQL is not case sensitive, but still it is good practice to use one style and stick to it.

Here is the same script that reads much more easily

SELECT
    Bk.title,
    GROUP_CONCAT
        (
        DISTINCT (Gnr.name) ORDER BY Gnr.relevancy,
        Gnr.name ASC
        SEPARATOR ' '
        ) AS genre_list,
    GROUP_CONCAT
        (
        DISTINCT CAST(Gnr.relevancy AS CHAR) ORDER BY Gnr.relevancy ASC
        SEPARATOR ','
        ) AS genre_relevancy,
    BkGnr.book_id
FROM book_genres BkGnr
    INNER JOIN genre_book Gnr
        ON BkGnr.genre_id = Gnr.id
        AND Gnr.id IN (". (($genreIdList) ? $genreIdList : 0) . ")
    INNER JOIN book Bk
        ON BkGnr.book_id = Bk.id
    INNER JOIN licensors Lic
        ON Bk.licensor_id = Lic.id
WHERE Bk.status = 'active'
    AND Bk.premium = 'yes'
    AND Lic.status = 'active'
GROUP BY BkGnr.book_id

SQL functions

This doesn't seem to make sense:

DISTINCT (Gnr.name)

DISTINCT CAST(Gnr.relevancy AS CHAR)

Why would there be duplicate genre name records... wouldn't that defeat the purpose of normalizing into a Genre type of table?

Also I don't think casting as CHAR is a good idea. Use VARCHAR instead. If you know about what the maximum character lenght should be, specify it so RDBMS doesn't have to guess, e.g., VARCHAR(50).

This is a bad idea.

AND Gnr.id IN (". (($genreIdList) ? $genreIdList : 0) . ")

It is bad practice to concatenate PHP and SQL together. Instead, use prepared statements with PDO.

SQL procedure

  This is the PHP/Eloquent version:

This whole section looks messy. You would gain clarity, repeatability and performance by using a SQL stored procedure. I'm going to assume your RDBMS is MySQL as is most common with PHP. Here is what you would execute once in MySQL to create it:

DROP PROCEDURE sp_genreIdList IF EXISTS;
DELIMITER |
CREATE PROCEDURE sp_genreIdList (IN p_genreIdList VARCHAR) -- assuming you are passing a string type parameter to RDBMS
AS
BEGIN
SELECT
    Bk.title,
    GROUP_CONCAT
        (
        DISTINCT (Gnr.name) ORDER BY Gnr.relevancy,
        Gnr.name ASC
        SEPARATOR ' '
        ) AS genre_list,
    GROUP_CONCAT
        (
        DISTINCT CAST(Gnr.relevancy AS CHAR) ORDER BY Gnr.relevancy ASC
        SEPARATOR ','
        ) AS genre_relevancy,
    BkGnr.book_id
FROM book_genres BkGnr
    INNER JOIN genre_book Gnr
        ON BkGnr.genre_id = Gnr.id
        AND Gnr.id IN (". (($genreIdList) ? $genreIdList : 0) . ")
    INNER JOIN book Bk
        ON BkGnr.book_id = Bk.id
    INNER JOIN licensors Lic
        ON Bk.licensor_id = Lic.id
WHERE Bk.status = 'active'
    AND Bk.premium = 'yes'
    AND Lic.status = 'active'
GROUP BY BkGnr.book_id;
END|
DELIMITER ; --make sure you don't forget that line

Then from PHP you simply:

$sql = "CALL sp_genreIdList("$genreIdList");";

Or something similar. And you can do this from multiple scripts in multiple languages and you will still get consistent results from RDBMS.


Find differences between two spreadsheets I have this code that is supposed to compare two Excel sheets. The code is working fine for small comparisons. I did a test run with 7 rows and 2 columns.

The code itself works as follows: it compares the two sheets and copies the differences into a new workbook.

However, the code should be applied to files that have around 16 columns and a lot of rows (around 206700).  It doesn't seem to scale very well to these larger sheets - the process is shown “Not Responding”, for more than ten minutes when I gave up.

This is how my excel file columns looks likem, keeping in mind that most of them contain text and only few times numbers are used.

I'd like to improve the performance with these larger files; any other suggestions for improvement are also welcome.

Sub Compare2WorkSheets(ws1 As Worksheet, ws2 As Worksheet)

  Dim ws1row As Long, ws2row As Long, ws1col As Integer, ws2col As Integer
  Dim maxrow As Long, maxcol As Integer, colval1 As String, colval2 As String
  Dim report As Workbook, difference As Long
  Dim row As Long, col As Integer

  Set report = Workbooks.Add

  With ws1.UsedRange
    ws1row = .Rows.Count
    ws1col = .Columns.Count
  End With

  With ws2.UsedRange
    ws2row = .Rows.Count
    ws2col = .Columns.Count
  End With

  maxrow = ws1row
  maxcol = ws1col
  If maxrow < ws2row Then maxrow = ws2row
  If maxcol < ws2col Then maxcol = ws2col

  difference = 0

  For col = 1 To maxcol
    For row = 1 To maxrow
      colval1 = ""
      colval2 = ""
      colval1 = ws1.Cells(row, col).Formula
      colval2 = ws2.Cells(row, col).Formula
      If colval1 <> colval2 Then
        difference = difference + 1
        Cells(row, col).Formula = colval1 & "<> " & colval2
        Cells(row, col).Interior.Color = 255
        Cells(row, col).Font.ColorIndex = 2
        Cells(row, col).Font.Bold = True
      End If
    Next row
  Next col

  Columns("A:B").ColumnWidth = 25
  report.Saved = True

  If difference = 0 Then
    report.Close False
  End If
  Set report = Nothing
  MsgBox difference & " cells contain different data! ", vbInformation, _
         "Comparing Two       Worksheets"
End Sub

There are some issues with your code:

1) Try to avoid unqualified references, this means always specify the worksheet, when referencing a cell.

2) Try to avoid formatting a lot of single cells, rather format them all at once at the end. Formatting slows down Excel a lot!

3) When handeling a great deal of Ranges, Integer can be insufficient, use Long instead.

Sub Compare2WorkSheets(ws1 As Worksheet, ws2 As Worksheet)

  Dim ws1row As Long, ws2row As Long, ws1col As Long, ws2col As Long
  Dim maxrow As Long, maxcol As Integer, colval1 As String, colval2 As String
  Dim report As Workbook, difference As Long
  Dim row As Long, col As Long
  Dim unionRng as Range

  Set report = Workbooks.Add

  With ws1.UsedRange
    ws1row = .Rows.Count
    ws1col = .Columns.Count
  End With

  With ws2.UsedRange
    ws2row = .Rows.Count
    ws2col = .Columns.Count
  End With

  maxrow = ws1row
  maxcol = ws1col
  If maxrow < ws2row Then maxrow = ws2row
  If maxcol < ws2col Then maxcol = ws2col

  difference = 0

  With report.Sheets(1)
    For col = 1 To maxcol
      For row = 1 To maxrow
        colval1 = ""
        colval2 = ""
        colval1 = ws1.Cells(row, col).Formula
        colval2 = ws2.Cells(row, col).Formula

        If colval1 <> colval2 Then
          difference = difference + 1
          .Cells(row, col).Value = colval1 & "<> " & colval2 'I guess you want to show, that the formulas used are not equal.
          If unionRng is Nothing Then
            Set unionRng = .Cells(row, col)
          Else
            Set unionRng = Application.Union(unionRng, .Cells(row, col))
          End If
        End If
      Next row
    Next col

    .Columns("A:B").ColumnWidth = 25
  End With

  unionRng.Interior.Color = 255
  unionRng.Font.ColorIndex = 2
  unionRng.Font.Bold = True
  report.SaveAs Filename:="report.xlsx"

  If difference = 0 Then
    report.Close False
  End If
  Set report = Nothing
  MsgBox difference & " cells contain different data! ", vbInformation, _
         "Comparing Two       Worksheets"
End Sub
```


Using PHP's password_hash and password_verify for a login function This is my first time using password_hash and password_verify in PHP. Would this be the correct usage of password_hash and password_verify to log the user into the site? Is there anything I could do to make this function more secure?

/**
 * Initiates User Login
 * @param  mixed $data Post data
 * @return mixed Checks if the user wants to be remembered and runs the corresponding function ('loginRemember', 'loginNoRemember')
 */
public function initiateLogin ($data) {
    //Database Connection
    $db = Database::getInstance();

    //Defining values for query
    $username = trim($data['login_username']);

    //Grab password from database based on username
    $query = $db->getConnection()->prepare("SELECT password FROM `users` WHERE username = :username");
    $query->execute(array(
        ':username' => $username
    ));

    //Query Results
    $queryResults = $query->fetch(PDO::FETCH_ASSOC);

    //Check if the password is correct
    if(password_verify($data['login_password'], $queryResults['password'])) {
        if (isset($data['login_remember_me'])) {
            $this->loginRemember($data);
        } else {
            $this->loginNoRemember($data);
        }
    } else {
        echo "Invalid password";
    }
}

Would this be the correct usage of password_hash and password_verify to log the user into the site

Yes. You didn't post your password_hash code, but you are using password_verify correctly, so if your code is working, everything is correct.

  Is there anything I could do to make this function more secure?

Not really. You are using prepared statements as you should, and you are using the currently recommended hashing approach.

Misc

Your in-function comments are just repeating your code, which is not ideal. Readers of your code will notice this and start ignoring all your comments, even important ones.
I do understand the desire to structure your code via these kinds of comments, but it would be better to either just remove them, or to introduce additional functions, and to slightly change your method signature. You could for example add a function such as getUserPassword($connection, $username) (possibly in a User class, or possibly just a method getByUsername($connection, $username) in a user class, which returns a User object, on which you could call getPassword). That way, you don't need the comment, as the code is even clearer about what it does.
I would pass the username to your function, not some data array, from which only the username is needed.
If possible, functions shouldn't have side effects as it makes them harder to (re-)use. Instead of echoing, return true on successful login, and false otherwise. That way, the calling code can decide what to display. That way, you can have all the code that does display something in one place, making it easier to change.


Reading the bytes of a PDF I'm quite a newbie in Python and I want to speed up this method since it takes very long time especially when the size of the input file in Mbs. Also, I couldn't figure out how to use Cython in the for loop. I'm using this function with other functions to compare files byte by byte. Any recommendations?

# this function returns a file bytes in a list
filename1 = 'doc1.pdf'
def byte_target(filename1):
    f = open(filename1, "rb")
    try:
        b = f.read(1)
        tlist = []
        while True:
            # get file bytes
            t = ' '.join(format(ord(x), 'b') for x in b)
            b = f.read(1)
            if not b:
                break
            #add this byte to the list
            tlist.append(t)

            #print b

    finally:
        f.close()
    return tlist

It's not surprising that this is too slow:
you're reading data byte-by-byte.
For faster performance you would need to read larger buffers at a time.

If you want to compare files by content, use the filecmp package.

There are also some glaring problems with this code.
For example, instead of opening a file, doing something in a try block and closing the file handle manually, you should use the recommended with-resources technique:

    with open(filename1, "rb") as f:
        b = f.read(1)
        # ...

Finally, the function name and all variable names are very poor,
and don't help the readers understand their purpose and what you're trying to do.


Simple, flexible, and thread-safe key/value memory cache MemoryCache.cs

public class MemoryCache<TKey, TValue>
{
    private static readonly object Lock = new object();
    private static readonly IDictionary<TKey, TValue> Cache = new ConcurrentDictionary<TKey, TValue>();

    /// <summary>
    /// Function for setting the cache's value when the key isn't found.
    /// </summary>
    public Func<TKey, TValue> Set { get; set; }

    /// <summary>
    /// Retrieves the value marked by the indicated key in a thread-safe way.
    /// If the item is not found, uses Set to retrieve it and store it in the cache before returning the value.
    /// </summary>
    public TValue Get(TKey key)
    {
        lock (Lock)
        {
            if (Cache.TryGetValue(key, out var value) == false)
            {
                value = Set(key);
                Cache.Add(key, value);
            }

            return value;
        }
    }

    /// <summary>
    /// Clears all values in the cache.
    /// </summary>
    public void Clear()
    {
        lock (Lock)
        {
            Cache.Clear();
        }
    }

}

Usage

Set is being used to retrieve the customer information from the database via stored procedure. GetBasicCustomer is a service/repository function for retrieving a customer by customerId or sapCustomerId.

public class CustomerService : ICustomerService
{
    private static readonly MemoryCache<Tuple<int?, string>, Customer> CustomerCache = new MemoryCache<Tuple<int?, string>, Customer>()
    {
        Set = (Tuple<int?, string> key) =>
        {
            var ds = Database.ExecuteStoredProcedure("EXEC usp_Experiment_BasicCustomerInformation @CustomerID, @SAPCustomerID", key.Item1, key.Item2);
            if (ds == null || ds.Tables.Count == 0 || ds.Tables[0].Rows.Count == 0)
            {
                return null;
            }

            var dr = ds.Tables[0].Rows[0];
            return new Customer(
                 id: dr.Field<int>("CustomerID"),
                 sapCustomerId: dr.Field<string>("SAPCustomerID"),
                 companyName: dr.Field<string>("CompanyName"),
                 isSbtCustomer: dr.Field<bool>("IsSBTCustomer")
             );
        }
    };

    /// <summary>
    /// Retrieves the customer's basic information given CustomerID or SAPCustomerID (only one is required, make the other null).
    /// </summary>
    public Customer GetBasicCustomer(int? customerId, string sapCustomerId)
    {
        var key = new Tuple<int?, string>(customerId, sapCustomerId);
        return CustomerCache.Get(key);
    }
}

public CustomerController()
{
    public ActionResult Index(int? customerId, string sapCustomerId)
    {
        var customerInfo = CustomerService.GetBasicCustomer(customerId, sapCustomerId);
        return View(customerInfo);
    }
}

Concerns

I was thinking Set should maybe be named FindAndSet or something like that, but not really sure. I like the aesthetic of Get/Set.
Is ConcurrentDictionary necessary to be thread-safe since I'm using a lock? Is it doing anything for me?

Generally, the code is simple and easy to understand.

Concurrency

It's good that you have a dedicated Lock object. You might consider a more expressive name, but in this case I think you can get away with it.

As you have noticed, using the lock means you gain nothing from the ConcurrentDictionary'2. Since this is such a simple interface, I would keep the lock and ditch the ConcurrentDictionary unless you have a serious (and measurable) performance concern.

However, you should be aware that by locking while you call Set, you are blocking access while you wait for an operation to complete, the length of which you cannot know. Addressing this without changing the class' behaviour would be non-trivial, primarily because it would complicate the case where Set fails, but may be necessary if Set is slow or needs access to the cache itself. If you don't care how many times you call Set, you could just use ConcurrentDictionary.GetOrAdd as mentioned by CharlesNRice in a comment, which would simply things somewhat.

Other

It's good that you have provided some level of inline documentation. I would be inclined to add an explanation of the threading behaviour (e.g. blocks while calling Set, will only call Set once). "a thread-safe way" is pretty meaningless: it's much better to explain the guarantees you will make.
Do you anticipate needing to change Set? If not, you should consider making it readonly and assign it in a constructor. There is a design concern that changing Set means you can't know which version was used by any call to Get from another thread.
Using a Tuple<int?, string> as a union doesn't seem a great idea. Either implement a sound union as a class of struct, or consider having a separate cache for each. If you do wish to keep this API, you should enforce the assumption that exactly one of them is null by explicitly checking and throwing if this is not the case, since it represents an error on the part of the caller, and telling them them sooner rather than later will save them lots of misery and time spent debugging.


Generating all valid dates I've come up with the following code to generate a list of all dates between two given dates:

   def generate_all_valid_dates_in_range(_start_date, _end_date) when _start_date <= _end_date do
    (:calendar.date_to_gregorian_days(_start_date) .. :calendar.date_to_gregorian_days(_end_date))
    |> Enum.to_list
    |> Enum.map (&(:calendar.gregorian_days_to_date(&1)))
  end

I would appreciate any comments on style and if there's some built in function (in either Elixir or Erlang) to achieve this sort of functionality.

I'm probably no better in Elixir than you are, but I will give it a go.

First of all the good parts: I think the function does what it should, and it's fairly neat. Not bad for a language that doesn't even have a concept of date, huh? :-)

Suggestions for improvement:

You should have a @doc attribute and unit tests :-) Remember to document that the range is inclusive (which is not what I would expect).
And if you add @doc, consider adding an example which can be run via a doctest in ExUnit. More info: elixir-lang.org/docs/stable/ex_unit/ExUnit.DocTest.html (thanks to @alxndr).
Consider making the range not inclusive, but rather [from, to) or (from, to]. The former is common in computer science, the latter is common in daily speech.
The _word style is usually used for arguments that aren't used as part of the pattern matching. This is why elixir doesn't warn you if you haven't used an argument whose name starts with an underscore. I would rename to start_date and end_date.
I would introduce an anonymous function working as an alias, to make the first line a bit easier on the eye. See example below.
Your function would give a MatchError on invalid output. I would probably deliberately check it and raise an ArgumentError instead.

Update: The Erlang/Elixir philosophy is let it fail, and therefore this suggestion might be contrary to common E/E style. From the creator himself:

  This is typically how Elixir/Erlang code is written indeed. If a condition is not met, it fails as MatchError or FunctionClauseError. We typically don't add an extra clause saying what went wrong. The upside is that we simply worry about the happy path (forcing us to write more assertive code). The downside is that it may be cryptic sometimes to find exactly what went wrong. We do include the arguments in the stacktrace though. — José Valim

You can neatify the last line to |> Enum.map &:calendar.gregorian_days_to_date/1.

All in all, I would transform it into something like this:

@doc """
Insert insightful documentation here.
"""
def generate_all_valid_dates_in_range(start_date, end_date)
when start_date <= end_date do
  to_days = &:calendar.date_to_gregorian_days/1
  to_days.(start_date) .. to_days.(end_date)
  |> Enum.to_list
  |> Enum.map &:calendar.gregorian_days_to_date/1
end

def generate_all_valid_dates_in_range(start_date, end_date)
when start_date > end_date do
  raise ArgumentError, "start_date must be before end_date"
end

Note that the second guard (when) is there only to highlight what the invariant is. It should catch all calls that the first definition doesn't. If any calls "fall through", we will get a MatchError and know there's something wrong in our logic.

Using the to_days alias, including the guard in the latter function definition and wrapping the guards to make the lines shorter are a matter of taste, and therefore optional.


Converting HashMap data to a Response object I have a HashMap and I want to convert data in it to a Response object. Is it possible to achieve this code that parses the string in a better and optimized and cleaner way, maybe by using streams?

class Converter{

   public static void main(String[] args) {

      Map<String, Long> map = new HashMap<String, Long>();
      map.put("111", 80) // first
      map.put("1A9-ppp", 190) // second
      map.put("98U-6765", 900) // third
      map.put("999-aa-local", 95) // fourth

      List<FinalProduct> products = new ArrayList<>();
      for(String key : map.keySet()){
        FinalProduct response = new FinalProduct();
        String[] str  = key.split("\\-");
        response.id = str[0] //id is always present in key i.e 111, 1A9, 98U,999
        if(str.length == 2) {
             if(str.matches("-?\\d+(\\.\\d+)?")){ // check if is numeric
                  response.code = str[1]; // 6765
             }
             else{
                  response.city = str[1]; //ppp
             }
        }
         if(str.length == 3){
                  response.client = str[1]; //aa
                  response.type = str[2];   // local
         }
        response.qty = map.get[key];
        products.add(response);
      }
   }
}

class FinalProduct{

String id;
String city;
Long code;
String client;
String type;
Long qty;
// getters and setters
}

The only thing you need is a saparate method doing the parsing.

This parse method can either be in a specific converter class that is used in a specific case when you're taking the strings and ammounts from a certain place.

public static FinalProduct parse(String key, Long quantity) {
    //transform into FinalProduct here
}

Or if you'll always start from those business specific Strings you could just provide a constructor for FinalProduct that takes those as input params:

public class FinalProduct{

    private String id;
    private String city;
    private Long code;
    private String client;
    private String type;
    private Long qty;

    public FinalProduct(String codedProduct, Long qty) {
        this.qty = qty;
        //parse coded input string here
    }

    // only getters here, no setters
}

Since the variables are probably not going to change after initializing the class you can make them final and not provide setters. Immutable objects are generaly easier to get right in a production code base (less edge cases when using multi threading for example).

Using streams isn't going to make much of a difference really. The parsing itself isn't going to change and the loop is already as simple as it's going to get. I personally prefer the loop version over the stream version when going through a map:

for loop:

List<FinalProduct> products = new ArrayList<>();
for (Map.Entry<String, Long> entry : map.entrySet()) {
    products.add(new FinalProduct(entry.getKey(), entry.getValue()));
}

stream:

map.entrySet().stream()
        .map(entry -> new FinalProduct(entry.getKey(), entry.getValue()))
        .collect(Collectors.toList());


Lists highly divisible numbers, oeis squence: A002182 My code works (as far as I can tell), so I'm looking mostly for readability and efficiency tips.

Goal of my code: Generate a list of all numbers that have more divisors than any lower number up to a certain limit. In other words this sequence: https://oeis.org/A002182

Background:  The number of divisors of \$n=p_1^{k1} \times p_2^{k2} \times p_3^{k3}\$ is just \$(k_1+1)\times(k_2+1)\times(k_3+1)\$... so the primes themselves don't matter, just the set of exponents.  Thus to be the smallest possible number with that number of divisors, the exponents are required be in order from largest (on the smallest primes) to smallest (on the largest primes).

My code breaks into 3 sections, the first figures out the largest prime needed.  The second generates a list of candidate numbers which is a list of all numbers that fit the descending exponent requirement.  Finally, the third part puts them in order and checks which of the candidates actually have more divisors than any previous numbers.

Final Random question: Is there a name for the middle algorithm? An algorithm that finds a list of all lists that are under a certain threshold?  A more generic version would probably set pows[:marker] = [0] * marker instead, but in my case I only want descending lists.

from numpy import prod

def highlydiv(n):
    """ Returns a list of all numbers with more divisors than any lower numbers, up to a limit n """
    primeprod = 1
    plist =  []
    for p in gen_prime(50): #gen_prime: generates primes up to n, can just use [ 2,  3,  5,  7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47]
        primeprod *= p
        if primeprod <= n:
            plist.append(p)
        else:
            break

    pows = [0 for x in plist]
    breakout = False
    candidate_list = []
    calcsum = 1
    while True:
        marker = 0
        pows[marker] += 1
        calcsum *= 2
        while calcsum > n:
            marker += 1
            if marker == len(pows):
                breakout = True
                break
            pows[marker] += 1
            pows[:marker] = [pows[marker]] * marker
            calcsum = prod([p**k for p, k in zip(plist, pows)])
        if breakout:
            break
        ndivs = prod([x+1 for x in pows])
        candidate_list.append((calcsum,ndivs))

    candidate_list.sort()
    maxdivs = 0
    final_list = []
    for candidate, ndivs in candidate_list:
        if ndivs > maxdivs:
            maxdivs = ndivs
            final_list.append(candidate)
    return final_list

print(highlydiv(1000000))

Consider writing your middle while loop as a separate function - it'll make breaking out of the inner loop easier.

For example, your code could look like this:

def highlydiv(n):
    plist = ...set up plist...
    candidates = divisor_candidates(n, plist)
    ...

def divisor_candidates(n, plist):
    ...set up calcSum, mark, pows, candidates...
    while True:
        ...
        while calcSum > n:
            marker += 1
            if marker > len(pows):
                return candidates
            pows[marker] += 1
            ...
        candidates.append(...)

and you get rid of the breakout flag.

Additionally, this makes the middle part of your program testable.


Advanced Python logging system I've been writing Python for years, but I've only just started using it in bigger programs than just simple server scripts. This means I now also want to use a more advanced logging system. To make things simple I created the following logger:

import logging
from inspect import getframeinfo, stack

logger = logging.getLogger()
formatter = logging.Formatter('%(asctime)s %(name)-7s %(levelname)-8s %(message)s')
file_handler = logging.FileHandler('mm.log')
file_handler.setFormatter(formatter)
stream_handler = logging.StreamHandler()
stream_handler.setFormatter(formatter)
logger.addHandler(file_handler)
logger.addHandler(stream_handler)
logger.setLevel(logging.INFO)

def l(*args, **kwargs):
    level = kwargs.get('level', 'info').lower()
    assert level in ('debug', 'info', 'warning', 'error', 'critical')
    caller = getframeinfo(stack()[1][0])
    file_line = "%s:%d - " % (caller.filename, caller.lineno)
    logger_input = file_line + ' '.join([str(a) for a in args])
    getattr(logger, level)(logger_input)

This logs to stdout and to a logfile and also logs the filename and the line number from which the log is made. It can be used as follows:

from tools import l

l('Log message')  # logs in INFO by default
l('Log message one', 'log message two')  # log multiple messages
l('Log message', level='critical')  # log in critical level

which creates something like this:

2017-10-10 20:47:16,170 root    INFO     test.py:5 - Log message
2017-10-10 20:47:16,171 root    INFO     test.py:14 - Log message one log message two
2017-10-10 20:47:16,171 root    CRITICAL test.py:116 - Log message

I import this in all the other files of my program and it seems to work quite ok. It enables to me to run the program from the command line and see the output, as well as running it as a daemon and seeing the output in the log files later. Furthermore, it's easy for debugging because it also logs the filename and line number, and I can easily change the default logging level with one line. Lastly, the use of a single l shortens the way of logging things. All in all, I'm quite happy with it.

I'm unsure if this is for some reason stupid though. Maybe there are situations which I can't oversee right now which would make this mess up things. Maybe the default conversion to strings is not smart for some reason?

Could anybody enlighten me as to what could be wrong with this or how it could be made better/more robust? All tips are welcome!

PEP-8 Conventions

Top-level functions should be separated (preceded and followed) by two blank lines:

logger.setLevel(logging.INFO)

def l(*args, **kwargs):
     <body>

Don't mix single quotes ' and double quotes " in a single file (or project). You may you use either of them, but

  Pick a rule and stick to it.

Lines should not exceed 79 characters (or 73 characters for documentation).

General concerns

l is a bad name for a function. Always try to pick a name that describes what the function does, like log.
You haven't added any docstrings to your function. If anyone else wants to use your code, they have no way to know what parameters to pass (or what the function actually does). According to PEP-257,

  All modules should normally have docstrings, and all functions and classes exported by a module should also have docstrings.

A good docstring describes what your function does, what parameters it expects, what it returns and if applicable, what Exception it raises if something goes wrong.
assert should really only be used for debugging purposes, not in the 'stable' release. I propose raising a ValueError instead.
Since you're only retrieving the level argument from **kwargs, you may as well make it a keyword argument.

Improved code

Taking into account all of the above, here's my rewritten version of your code:

import logging
from inspect import getframeinfo, stack

logger = logging.getLogger()
formatter = logging.Formatter('
    %(asctime)s %(name)-7s %(levelname)-8s %(message)s
')
file_handler = logging.FileHandler('mm.log')
file_handler.setFormatter(formatter)
stream_handler = logging.StreamHandler()
stream_handler.setFormatter(formatter)
logger.addHandler(file_handler)
logger.addHandler(stream_handler)
logger.setLevel(logging.INFO)

def log(*args, level='info'):
    """Log a message to stdout and a log file.
    Arguments:
    - *args: All messages to be logged (as strings).

    - level: The level for debugging. Any of 'debug', 'info', 'warning',
             'error', 'critical'. Defaults to 'info'.

    Raises:
    - ValueError if an invalid `level` argument is passed.
    """
    LEVELS = ('debug', 'info', 'warning', 'error', 'critical')
    level = level.lower()
    if level not in LEVELS:
        raise ValueError('`level` argument must be any of {}'.format(
            str(LEVELS)[1:-1]
        )
    caller = getframeinfo(stack()[1][0])
    file_line = '%s:%d - ' % (caller.filename, caller.lineno)
    logger_input = file_line + ' '.join([str(a) for a in args])
    getattr(logger, level)(logger_input)

The use of *args in the above snippet breaks backward compatibility!


Delete duplicate entries with lower IDs I have a function that deletes duplicate entries.  The highest ID is kept and the older ones are removed.

function:

DELETE [tableName]
FROM [tableName]
INNER JOIN (SELECT * , ROW_NUMBER() OVER
           (PARTITION BY [fork_id] ORDER BY ID DESC)
           AS RowNumber FROM [tableName])
Numbered ON [tableName].ID = Numbered.ID
WHERE RowNumber > 1

For example, it changes

|------|---------|--------|
|  ID  | fork_id |   Car  |
|------|---------|--------|
|  1   |    2    |  AUDI  | <--- removed
|  2   |    1    |  AUDI  |
|  3   |    2    |  BMW   |
|------|---------|--------|

to

|------|---------|--------|
|  ID  | fork_id |   Car  |
|------|---------|--------|
|  2   |    1    |  AUDI  |
|  3   |    2    |  BMW   |
|------|---------|--------|

The problem with that query is the execution time exceed time when with have many rows (more than 50k) in the table.

I have a primary key for the ID column

I'm In the sql server, I have a limitation about execution time.

  A connection can be cut off by the server for a number of reasons:

  Idle connection longer than 5 minutes.
  Long running query.
  Long running open transaction.
  Excessive resource usage.

  sources

This would probably be better on DBA
You can use a CTE
An index on fork_id should help

with cte as
( SELECT ROW_NUMBER() OVER (PARTITION BY [fork_id] ORDER BY ID DESC) AS RowNumber
  FROM [tableName]
)
delete from cte
WHERE RowNumber > 1

Optimize

select * from cte
WHERE RowNumber > 1

If that is fast it is volume thing and you could delete in batches


d20pfsrd Discord Bot Scraper Abstraction and Deployment I wrote a discord bot that listens for links to the d20pfsrd rulebook in discord chat, checks if they are feats or magic, and then scrapes the page, formats it, and spits it back into chat as formatted text. The pages I'm checking are formulaic enough, so even though the cheerio selectors I'm using are probably fairly brittle, it seems to work in most cases. The text formatters are some ugly beasts though (though the logic is probably unique), and the axios calls could use some cleaning up as well.
My problem is as follows: I suspected a good 40% of the code can be refactored out of existence if I properly generalized it. My solution was to do something that could probably be described as an abuse of the spread operator:
async function getPage(msg, url){
  let message = "placeholder";
  await axios.get(url).then( (response) => {
      message = responder(...selectResponder(url), response);
    }
    ).catch((error) => {
      console.error(error.message);
    });
  sendMessage(message, msg);

}

A week later I'm now realizing I wrote the functions in a way that causes them to read backwards? Aesthetics aside, this is how it ends up working out:
function selectResponder(siteUrl){
  if (siteUrl.startsWith("https://www.d20pfsrd.com/feats/")){
      return feats.featsConfig;
    } else if (siteUrl.startsWith("https://www.d20pfsrd.com/magic/")){
      return magic.magicConfig;
  }
}

function responder(parser, formatter, messageFormatter, response){
    const replyData = parser(response.data);
    const formattedData = formatter(replyData);
    return messageFormatter(formattedData);
}

The idea is that as much unique logic as I could separate out exists in magic.js and feats.js as possible. How can I clean this section up? The github repo is here and I would love and feedback on the entire project. I did most of that because I thought it as weird to have a big if block here and duplicating the axios calls:
client.on('message', msg => {

  if(validateUrl(msg.content)){
    const siteUrl = encodeURI(msg.content);
    getPage(msg, siteUrl);
  }

});

Also I wrote up this gist on how I deployed it to a raspberry pi. That works, so suggestions on how to improve it would be greatly appreciated
In broad strokes this program:

Listens for a message
Validates the message
Gets the page content at the message url
Parses the page content
Re-formats the page content
Converts the page content into a message
Sends the message

Here's how I would do it. Move the logic about which pages a parser consumes into the code for said parser. Also organizing your helper files into objects could reduce complexity in terms of the number of variables you pass around but that's more a matter of personal preference.
class SpecificPageParser {
    match(url) {
        return (
           url.startswith('https://www.d20pfsrd.com/magic/') &&
           !url.endsWith('/magic/')
        );
    }
    format(response) {
        // do your parsing stuff
        const formatted = {...response,'extra':'info'};
        return formatted;
    }
}

const responders = [new SpecificPageParser(),new SomeOtherParser()];

async function getPage(msg, url){
    try {
        const responder = responders.find(responder=>responder.match(url));
        if (!responder) return;
        const response = await axios.get(url);
        const message = responder.format(response);
        sendMessage(message, msg);
    }
    catch(error) {
        console.error(error);
    }
  }
```


Print code-fenced sections of a Markdown document Original code and demo at this gist.

Given a Markdown document like

Here is some text,
and some more text.
```javascript
const message = "This is JavaScript!";
```
More text follows, and then
```javascript
console.log(message);
```

I want to print out the sections in code fences, not including the code
fences, separated by a single blank line:

const message = "This is JavaScript!";

console.log(message);

I came up with the following AWK script, which seems to do the job
nicely:

#!/usr/bin/awk -f
BEGIN { in_code_block = 0 }
/^```/ {
    if (!in_code_block) {
        in_code_block = 1;
        first_line = 1;
    } else {
        in_code_block = 0;
        print "";
    }
}
{
    if (in_code_block && !first_line) {
        print;
    }
    first_line = 0;
}

A goal is for the script to be dependency-minimal. I don't want to have
to install an implementation of CommonMark or an Erlang environment. AWK
fits this bill well.

Correspondingly, a non-goal is for this script to be correct in all
cases: I’m happy to accept false positives on lines starting with
```inline code``` like this, and similar edge cases.

I’m mostly looking for critique of my AWK, with respect to which I am a
total newbie. But any comments are welcome!

While your code looks ok, it could be improved greatly by making use of RS (record separator) and NR number of record, provided you're using GNU awk.

 awk -v RS='```[a-z]*\n' '(NR+1)%2' file

In this case RS is set such that it is catching everything between triple backticks with optional text.

The only awk statement is to print one record out of two.


Goldbach's Conjecture using Sieve of Eratosthenes I created a simple Java program to calculate Goldbach's conjecture and I was wondering how I could improve it (besides using clearer prompt messages).

/* Title: Goldbach Conjecture
 * Author: Harris R
 */

import java.util.Scanner;

public class Goldbach
{
  public static void main(String[] args)
  {
    Scanner scan = new Scanner(System.in);
    System.out.println("Please input an integer to find two prime numbers whose sum add up to it.");
    int input = scan.nextInt();
    if(input < 4)
    {
      System.out.println("Number must be greater than or equal to 4");
      return;
    }
    if(input%2 > 0)
    {
      System.out.println("Number must be even.");
      return;
    }
    long startTime = System.nanoTime();
    int[] primes = generatePrimes(input);
    findPairs(input, primes);
    long endTime = System.nanoTime();
    System.out.println("\n This program took: " + (endTime-startTime) + " nanoseconds or " + ((endTime-startTime)*1e-9) + " seconds.");
  }

  private static int[] generatePrimes(int max)
  {
    boolean[] isComposite = new boolean[max + 1];
    for (int i = 2; i * i <= max; i++)
    {
      if (!isComposite [i])
      {
        for (int j = i; i * j <= max; j++)
        {
          isComposite [i*j] = true;
        }
      }
    }
    int numPrimes = 0;
    for (int i = 2; i <= max; i++)
    {
      if (!isComposite [i]) numPrimes++;
    }
    int [] primes = new int [numPrimes];
    int index = 0;
    for (int i = 2; i <= max; i++)
    {
      if (!isComposite [i]) primes [index++] = i;
    }
    return primes;
  }

  private static void findPairs(int input, int[] primes)
  {
    String primepairs = "";
    for(int i = 0; i < primes.length; i++)
    {
      for(int j = i; j < primes.length; j++)
      {
          int p = primes[i];
          int q = primes[j];
          if(p+q == input)
          {
              primepairs += "\n" + q + " + " + p + " = " + input;
          }
      }
      System.out.println("Program Completion: " + (((double)i/primes.length)*100) + "%");
    }
    System.out.println(primepairs);
  }
}

Efficiency

Post-processing the results from the Sieve of Eratosthenes (isComposite) to produce primes is actually counterproductive.  If the findPairs() code just has a list of primes to work with, then it cannot quickly answer "is n a prime?" for any given n.  Hence, you need to try all pairs of primes, making findPairs() O(|P|2), where |P| is the size of the set of primes.  And that's in addition to the work needed to find those primes.

You would be better off using the raw isComposite array.  Then, for any given p, you can immediately find out whether input - p is also prime.

In your Sieve of Eratosthenes loop, you can use addition instead of multiplication.

Code organization

Mixing input/output code with computation code is poor practice.  I suggest using an Iterator<Integer> to iterate through the results.  Then you can use a nice simple loop in main() to print the results.

Instead of System.out.println(… + … + … + …), use System.out.printf(), which is more readable.  (Actually, System.err would be a better place to send diagnostic output.)

Suggested solution

import java.util.Iterator;
import java.util.NoSuchElementException;
import java.util.Scanner;

public class GoldbachSums implements Iterable<Integer> {
    private final int sum;
    private final boolean[] ineligible;

    public GoldbachSums(int sum) {
        if (sum <= 2) {
            throw new IllegalArgumentException("Number must be greater than 2.");
        }
        if (sum % 2 > 0) {
            throw new IllegalArgumentException("Number must be even.");
        }
        this.sum = sum;
        this.ineligible = oddSieveOfEratosthenes(sum);
    }

    private static boolean[] oddSieveOfEratosthenes(int max) {
        boolean[] ineligible = new boolean[max];
        ineligible[1] = true;
        for (int i = 3; i * i < max; i += 2) {
            if (! ineligible[i]) {
                for (int j = i * i; j < max; j += i) {
                    ineligible[j] = true;
                }
            }
        }
        return ineligible;
    }

    @Override
    public Iterator<Integer> iterator() {
        return new Iterator<Integer>() {
            // 4 is the only sum whose decomposition involves an even prime
            private Integer n = (sum == 4) ? 2 : computeNext(sum - 1);
            private final int halfSum = sum / 2;

            private Integer computeNext(int n) {
                for (int i = n; i >= halfSum; i -= 2) {
                    if (!ineligible[i] && !ineligible[sum - i]) {
                        return sum - i;
                    }
                }
                return null;
            }

            @Override
            public boolean hasNext() {
                return n != null;
            }

            @Override
            public Integer next() {
                if (n == null) {
                    throw new NoSuchElementException();
                } else try {
                    return n;
                } finally {
                    n = computeNext(sum - n - 2);
                }
            }

            // For compatibility with Java < 8
            public void remove() {
                throw new UnsupportedOperationException();
            }
        };
    }

    public static void main(String[] args) {
        try (Scanner scan = new Scanner(System.in)) {
            System.out.print("Please input an integer to decompose as the sum of two primes: ");
            int input = scan.nextInt();

            long startTime = System.nanoTime();
            GoldbachSums goldbach = new GoldbachSums(input);
            long duration = System.nanoTime() - startTime;
            System.err.printf("\nSieve of Eratosthenes took: %d ns or %f seconds.\n",
                              duration, duration * 1e-9);
            duration = System.nanoTime() - startTime;
            for (int addend : goldbach) {
                System.out.printf("%d + %d = %d\n", input - addend, addend, input);
            };
            System.err.printf("\nThis program took: %d ns or %f seconds.\n",
                              duration, duration * 1e-9);
        } catch (IllegalArgumentException badInput) {
            System.out.println(badInput.getMessage());
        }
    }
}


Generate a large block diagonal covariance matrix with exponential decay I am implementing Kalman filtering in R. Part of the problem involves generating a really huge error covariance block-diagonal matrix (dim: 18000 rows x 18000 columns = 324,000,000 entries). We denote this matrix Q. This Q matrix is multiplied by another huge rectangular matrix called the linear operator, denoted by H.

I am able to construct these matrices but it takes a lot of memory and hangs my computer. I am looking at ways to make my code efficient or do the matrix multiplications without actually creating the matrices exclusively.

library(lattice)
library(Matrix)
library(ggplot2)

nrows <- 125

ncols <- 172

p <- ncols*nrows

#--------------------------------------------------------------#
# Compute Qf.OSI, the "constant" model error covariance matrix #
#--------------------------------------------------------------#

  Qvariance <- 1
  Qrho <- 0.8

  Q <- matrix(0, p, p)

  for (alpha in 1:p)
  {
    JJ <- (alpha - 1) %% nrows + 1
    II <- ((alpha - JJ)/ncols) + 1
    #print(paste(II, JJ))

    for (beta in alpha:p)
    {
      LL <- (beta - 1) %% nrows + 1
      KK <- ((beta - LL)/ncols) + 1

      d <- sqrt((LL - JJ)^2 + (KK - II)^2)
      #print(paste(II, JJ, KK, LL, "d = ", d))

      Q[alpha, beta] <-  Q[beta, alpha] <-  Qvariance*(Qrho^d)
    }
  }

  # dn <- (det(Q))^(1/p)
  # print(dn)

  # Determinant of Q is 0
  # Sum of the eigen values of Q is equal to p

  #-------------------------------------------#
  # Create a block-diagonal covariance matrix #
  #-------------------------------------------#

  Qf.OSI <- as.matrix(bdiag(Q,Q))

  print(paste("Dimension of the forecast error covariance matrix, Qf.OSI:")); print(dim(Qf.OSI))

It takes a long time to create the matrix Qf.OSI at the first place. Then I am looking at pre- and post-multiplying Qf.OSI with a linear operator matrix, H, which is of dimension 48 x 18000. The resulting HQf.OSIHt is finally a 48x48 matrix. What is an efficient way to generate the Q matrix? The above form for Q matrix is one of many in the literature. In the below image you will see yet another form for Q (called the Balgovind form) which I haven't implemented but I assume is equally time consuming to generate the matrix in R.

JJ <- (alpha - 1) %% nrows + 1
    II <- ((alpha - JJ)/ncols) + 1

That looks likely to be buggy. I would guess that a is supposed to be an encoding for a pair (row, col), but in that case the same base should be used for the %% and the /.

I would also suggest that if you can't use 0-indexed matrices then you do the offset to 1-based when you access the matrices, and keep the values you manipulate 0-based. See how much simpler this is:

for (rowa in 0:(nrows-1))
{
  for (cola in 0:(ncols-1))
  {
    a = rowa * ncols + cola
    for (rowb in 0:(nrows-1))
    {
      for (colb in 0:(ncols-1))
      {
        b = rowb * ncols + colb
        d = sqrt((rowa - rowb)^2 + (cola - colb)^2)
        Q[a+1, b+1] <- Qvariance * (Qrho^d)
      }
    }
  }
}

Incidentally, since Qvariance is multiplied into every single element you could pull that out and post-multiply the final \$48 \times 48\$ matrix instead.

Now, elimination of the matrix. We have \$(AB)_{i,j} = \sum_k A_{i,k} B_{k,j}\$, so $$(HQH^T)_{i,j} = \sum_k H_{i,k}(QH^T)_{k,j} = \sum_k H_{i,k} \sum_l Q_{k,l} H^T_{l,j} = \sum_k \sum_l H_{i,k} H_{j,l} Q_{k,l}$$ which allows you to restructure the code so as to avoid creating \$Q\$ in memory. However, it is at the cost of using the naïve algorithm for matrix multiplication, and your matrices are large enough that R is probably using a sub-cubic algorithm. So what you might want to do is to instead break it down into chunks: e.g. of size nrows \$\times\$ nrows. I don't know enough R to be certain, but I expect that its index range notation allows you to do this quite cleanly.

Following up on some comments, we can expand \$k = r_1 C + c_1\$, \$l = r_2 C + c_2\$ where \$C\$ is ncols, and get
$$(HQH^T)_{i,j} = \sum_{r_1} \sum_{c_1} \sum_{r_2} \sum_{c_2} H_{i,r_1 C + c_1} H_{j,r_2 C + c_2} Q_{r_1 C + c_1,r_2 C + c_2} \\
= \sigma \sum_{r_1=1}^R \sum_{r_2=1}^R \sum_{c_1=1}^C \sum_{c_2=1}^C H_{i,r_1 C + c_1} H_{j,r_2 C + c_2} \rho^{\sqrt{(r_1-r_2)^2 + (c_1-c_2)^2}} $$

Let \$Q^{(\delta)}\$ be a symmetric \$C \times C\$ matrix with \$Q^{(\delta)}_{i,j} = \rho^{\sqrt{\delta^2 + (i-j)^2}}\$. Then $$(HQH^T)_{i,j} =  \sigma \sum_{r_1=1}^R \sum_{r_2=1}^R \sum_{c_1=1}^C \sum_{c_2=1}^C H_{i,r_1 C + c_1}  Q^{(|r_1-r_2|)}_{c_1,c_2} H_{j,r_2 C + c_2} \\
HQH^T = \sigma \sum_{r_1=1}^R \sum_{r_2=1}^R H_{1..48,r_1 C .. (r_1+1)C}  Q^{(|r_1-r_2|)} H_{1..48,r_2 C..(r_2+1)C}^T $$

and the sum can be regrouped by \$|r_1 - r_2|\$ to calculate each \$Q^{(\delta)}\$ only once. When calculating \$Q^{(\delta)}\$ you can exploit the symmetry without worrying too much about cache coherence, because the whole of \$Q^{(\delta)}\$ should fit in L2 cache.


Binary search implementation in C int bin_search(int* a, int key)
{
    int lo = 0;
    int hi = sizeof(a)/sizeof(int) - 1;

   while(lo <=hi)
   {
        int mid = (lo + hi)/2;

        if(key < a[mid]) { hi = mid - 1;}
        else if (key > a[mid]) { lo = mid + 1;}
        else                    return mid;
    }

    return -1;
}

....

for(i = 0; i < sz; ++i)
{
    printf("%d\n", bin_search(a, i));
}

I think that I have a problem with this, but I can't understand why:

int hi = sizeof(a)/sizeof(int) - 1;

sizeof(a) in this case is sizeof(int*) which is 4 or 8 on most machines. The most simple solution would be passing the length of the input array. Otherwise, your code looks nifty.


Convert an array of doubles to a concatenated Hex string I'm working on a tool that converts data between two systems. System 1 presents me with arrays of latitudes and longitudes as doubles. System 2 requires an input of that array as a single Hex string.

I've been calling the following method to do the conversion, and then appending a string with the results, however I'm finding that it is extremely slow, as it's being called 12-13 million times in a typical conversion.

var string = "";
foreach(var value in list<geo>)
string += (DoubleToHex(value.lat) + DoubleToHex(value.lon))

public static string DoubleToHex(double input)
{
    byte[] byteArray = BitConverter.GetBytes(input);
    StringBuilder hex = new StringBuilder(byteArray.Length * 2);
    foreach (byte b in byteArray)
        hex.AppendFormat("{0:x2}", b);
    return hex.ToString().ToUpper();
}

What alternatives exist that would be quicker then using StringBuilder, (or what else am I missing)?

string is immutable type, whenever you modify a string, it actually creates a new string.

if you convert it to StringBuilder such as :

var sb = new StringBuilder(string.Empty);

foreach (var value in list)
    sb.Append(DoubleToHex(value.lat)).Append(DoubleToHex(value.lon));

You'll gain more performance.

However, the DoubleToHex() also can simplified further to :

public static string DoubleToHex(double input)
{
    return BitConverter.ToString(BitConverter.GetBytes(input)).Replace("-", "");
}

this will boost the performance further.


Python configuration module for a web framework I'm currently writing a web framework and I wanted to add a configuration module for reading environment variables and a configuration toml file.

Here is the code. The comment in the Config class should explain it all.

import os
import sys
import toml

BASEPATH = os.path.dirname(sys.modules['__main__'].__file__)
print(BASEPATH)
FILEPATH = os.path.abspath(os.path.join(BASEPATH, 'config.toml'))
print(FILEPATH)

class ConfigError(KeyError):
    """ Raised when a requested configuration can not be found """

class ConfigBase:
    def __init__(self):
        self.basename = ''
        self.default_options = {}
        raise TypeError('Should not use ConfigBase directly')

    def __getitem__(self, item):
        """ called when access like a dictionary
                config['database']['username']
        """
        default = self.default_options.get(item)
        if isinstance(default, dict):
            return ConfigSection(self.basename + '.' + item, default)
        # Check for env var
        envvar_string = '_'.join(self.basename.split('.') + [item]).upper()
        env = os.getenv(envvar_string)
        if env is None:
            if default is None:
                raise ConfigError(
                    'No configuration found for {}. '
                    'Add the environment variable {} or add'
                    ' the key to your config.toml file'.format(
                        '.'.join((self.basename, item)),
                        envvar_string
                    ))
            return default
        return env

class Config(ConfigBase):
    """ load configuration from envvar or config file
        You can get settings from environment variables or a config.toml file
        Environment variables have a higher precedence over the config file

        Use `config['section']['subsection']['key']` to get the value
        SECTION_SUBSECTION_KEY from environment variables or
        section.subsection.key from a toml file
        (usually written:
            [section.subsection]
            key = value
        in a toml file)
    """
    def __init__(self):
        self.default_options = self.load_config()
        self.basename = ''

    def load_config(self):
        with open(FILEPATH, 'r') as f:
            config = toml.load(f)
            return config

class ConfigSection(ConfigBase):
    def __init__(self, basename, defaults):
        self.basename = basename.lstrip('.')
        self.default_options = defaults

toml is a third-party library, so should be separated from the standard library imports:

import os
import sys

import toml

ConfigBase.__init__ seems a little strange:

def __init__(self):
    self.basename = ''
    self.default_options = {}
    raise TypeError('Should not use ConfigBase directly')

Why bother to set the attributes if you're then going to throw an error? Also you should use NotImplementedError for abstract base classes, by convention.

Config.load_config seems to be internal; are you expecting it to be called from anywhere other than Config.__init__? If not, rename it _load_config to indicate that it's an implementation detail. Also note that the temporary assignment to config is unnecessary:

def load_config(self):
    with open(FILEPATH, 'r') as f:
        return toml.load(f)

I'd also note that it seems weird to add a docstring for __getitem__ that just describes what that method is for, but then not document the non-standard load_config at all. Your target audience should already know what __getattr__ is for (or where to find out).

Talking of __getattr__, there's an awful lot of complexity hiding in there. Every other method you've written is nice and short, but this could do with breaking down. To describe it:

Get the default value
If the default is a dictionary, wrap it up and return it immediately
If not, but there's an appropriate value in the environment, return that
If not, but the default exists, return the default
If not, throw an error

That's how I'd structure the method:

default = self._get_default_value(item)
if isinstance(default, dict):  # or the more general collections.abc.Mapping
    return self._create_config_section(item, default)  # or maybe return ConfigSection(self._create_basename(item), default)
env = self._get_value_from_env(item)
if env is not None:
    return env
if default is not None:
    return default
raise ConfigError(self._create_error_message(item))

This reads like the description I wrote above, operates at a consistent level of abstraction (not delving into the details of e.g. how to get a value from the environment or where the default comes from) and reduces the nesting.

You can split out other obvious helper methods, like self._create_env_var(item), along the way. Doing so might highlight to you that it's a little weird to do the same thing two different ways in the same method:

self.basename + '.' + item
'.'.join((self.basename, item))

Finally, I don't think the distinction between ConfigBase, Config and ConfigSection is really helping you. You interact with them the two child classes in much the same way; only the initialisation differs. Instead, I might be inclined to do:

class Config:

    def __init__(self, basename='', defaults=None):
        self.basename = basename  # should already be lstrip-ped
        if defaults is None:
            defaults = self._load_config()
        self.default_options = defaults

    def __getitem__(self, item):
        ...

    ...


Optimize the CSV Upload function in Ruby On Rails The code is working fine however, I would like to fix code after unless and can I store thread ID in the database too? Is there a way to improve performance?

require 'csv'
    class CsvDb
      class << self
        def convert_save(model_name, csv_data, field_name=nil)
          target_model = model_name.classify.constantize
          csv_file = csv_data.read
          row_headers={}
          counter=0;
          #Thread.new do
                    CSV.parse(csv_file) do |row|
                        if counter==0
                            temp=row
                            row_headers = Hash[temp.map.with_index.to_a]
                            counter +=1
                            next
                        end
                        unless row[row_headers["name"]].nil?
                            temp={}
                            business_type = row[row_headers["business_type_id"]]
                            business_type_id = business_type=='Online' ? 1: business_type=='Local' ? 2: 3
                            temp[:business_type_id]         = business_type_id
                            temp[:user_id]                          = row[row_headers["user_id"]]
                            temp[:name]                                 = row[row_headers["name"]]
                            temp[:country_id]                   = row[row_headers["country_id"]]
                            temp[:homepage]                         = row[row_headers["homepage"]] ||=""
                            temp[:telephone]                        = row[row_headers["telephone"]] ||=""
                            temp[:email]                                = row[row_headers["email"]] ||=""
                            temp[:address]                          = row[row_headers["address"]] ||=""
                            temp[:latitude]                         = row[row_headers["latitude"]]
                            temp[:longitude]                        = row[row_headers["longitude"]]
                            temp[:facebook]                         = row[row_headers["facebook"]] ||=""
                            temp[:twitter]                          = row[row_headers["twitter"]] ||=""
                            temp[:google]                           = row[row_headers["google"]] ||=""
                            temp[:instagram]                        = row[row_headers["instagram"]] ||=""
                            temp[:pinterest]                        = row[row_headers["pinterest"]] ||=""
                            temp[:free_shipping]                = row[row_headers["free_shipping"]]
                            temp[:ship_details]                 = row[row_headers["ship_details"]] ||=""
                            temp[:category_ids]                 = [row[row_headers["category_ids"]]]
                            temp[:style_ids]                        = [row[row_headers["style_ids"]]]
                            temp[:shipping_country_ids] = [row[row_headers["shipping_country_ids"]]]
                            business = target_model.new(temp)
                            business.save
                        end
                    end
                    ActiveRecord::Base.connection.close
          end
        #end
      end
    end

The following code is not tested, maybe some minor changes will be necessary: I'm not 100% sure it will work as is, don't hesitate to tell me if there is anything wrong with the code

Refactoring the existing code

The first thing I did was to set the headers option to true it will enable the parsing of the first line of the CSV and instead of giving you an array for each row it will return an hashmap with keys extracted from the header. It replace the code you wrote to parse the header of the file.

Then I replace the imbricated ternary operations to use a case when expression for the business_type_id which is far more readable.

Next I made two small function to extract all the needed values, I use reduce to automatically fill a hashmap from an array of symboles.

At least I merge all the hashmap in the constructor of the target_model before saving.

CSV.parse(csv_file, {headers: true, header_converters: :symbol}) do |row|
  business_type_id =  case row[:business_type_id]
                        when 'Online' then 1
                        when 'Local' then 2
                        else 3
                      end

  target_model.new( {business_type_id: business_type_id} + extract_required_fields(row) + extract_optionals_fiels(row) )
              .save()

end

def extract_required_fields(row)
  [:user_id, :name, :country_id, :free_shipping, :category_ids, :style_ids, :shipping_country_ids]
    .reduce({}) do |carry, item|
      carry[item] = row[item]
  end
end

def extract_optionals_fiels(row)
  [:homepage, :telephone, :email, :address, :facebook, :twitter, :google, :instagram, :pinterest, :ship_details]
    .reduce({}) do |carry, item|
      carry[item] = row[item] ||= ''
  end
end

Bulk update

But this code, even if it has a better separation of concern and less duplication will not really speed your import process because your performance bottleneck is in the insert operation performed for each item by the save method.

So the solution is to make a bulk update instead of saving each row separately: you'll group the insert !

To do so, you can use the https://github.com/zdennis/activerecord-import gem which will allow you to do something like this :

entities = []
CSV.parse(csv_file, {headers: true, header_converters: :symbol}) do |row|
  business_type_id =  case row[:business_type_id]
                        when 'Online' then 1
                        when 'Local' then 2
                        else 3
                      end

  entities << target_model.new( {business_type_id: business_type_id} + extract_required_fields(row) + extract_optionals_fiels(row) )
  target_model.import entities if entities.size == 100
end

#don't forget to save the remaining entities:
target_model.import entities unless entities.size == 0

In this case we proceed the save operation each 100 row, which will make the import process way faster !

Getting the Thread's ID

To collect the thread's Id, I suggest to use the inspect method of the current_thread, you'll get more information than only the thread's id but you'll be able to have all the needed informations about which thread processed the insert operation.

thread_identifier = Thread.current.inspect


Shorthand expansion exercise from K & R C (ex 3-3) I have written a function to take some shorthand input and expand it into the full form, for example a-z0-9 becomes abcd...789. I added protection from buffer overflow exploits (knocks on wood), and have covered (or think I have) all and any edge cases that the user could throw at it.

Are there any ways in which my code could be improved (subjective and objective criticism welcome here)? Have I missed an edge case? are there any insecure sections of my code that might allow for a buffer overflow? Is my code written in a fairly concise and readable way?

// expand.c - function expand(s1, s2) takes shorthand input (i.e. a-z0-9) and expands it, placing the result in s2 (i.e. abcde...789)
#include <stdio.h>
#define MAXLEN 256
#define UP 1    //Included only to help improve readability
#define DOWN -1 //Ditto

void expand(char s1[], char s2[]);

int main()
{
    char output[MAXLEN], input[MAXLEN];
    printf("Enter string to expand: ");
    fgets(input, MAXLEN + 1, stdin);
    expand(input, output);
    printf("%s\n", output);
    return 0;
}

void expand(char s1[], char s2[])
{
    int i, j, k, count;
    for(i = j = 0; s1[i] != '\0' && i < MAXLEN && j < MAXLEN; i++) {
        if(s1[i + 1] == '-' && s1[i + 2] != '\0') {
            if(s1[i] < s1[i + 2])
                count = UP;
            else
                count = DOWN;
            for(k = 0; s1[i] + k - count != s1[i + 2] && j < MAXLEN; k += count, j++) {
                if(s1[i - 1] == '-' && k == 0)
                    j--;
                s2[j] = s1[i] + k;
            }
            s2[j] = '\0';
        }
    }
}

Here are some comments regarding your code:

Using s1[i+2] and s1[i-1] can possibly refer to memory locations outside of allocated memory
Inside of expand() you use the restriction of MAXLEN which is not conveyed in the parameters, which should be like char s1[MAXLEN] (which also introduces different strange aspect of the c-language). As your code stands now you can easily call it with char fails[2]; expand(fails, fails)
You should also test for null before accessing input parameters, currently you can call expand(input, 0) and it will happily try to do something which will fail
You are better of to always include braces around blocks within if-else statements to avoid erroneous indentation errors
Give s1 and s2 proper names to indicate what they are
You'll possibly get into problems or unwanted behaviour, with input like z-0, a-00--a, Z-0, --[
The range a-a will give count = DOWN, and behave somewhat strangely

Regarding the brace comment, consider the following code snippet:

int a = 13; int b = 5;
if (a < b)
   a = a + 10;
   if (a > b)
      a = 5;
else
   b = 6;

What will a and b hold now? Correct answer is a=5, b=5. If surrounding the blocks with braces the code would behave like you possibly read it, indicating the correct response of a=13, b=6. This is a constructed example, but it does happen way to often in real life that someone indents the code wrongly and strange things happen.

Here is the code example again, using starting braces on the same line, many prefer them to be on a new line. Choose a style, and stick to it.

int a = 13; int b = 5;
if (a < b) {
   a = a + 10;
   if (a > b) {
      a = 5;
   }
} else {
   b = 6;
}


PHP-generated table with colored cells How can I make my PHP script more easier? It seems quite difficult and long but I don´t know how can I make it shorter. This code is inserted in my HTML5 code on my PHP tutorial webpage, but I need it to be shorter.

<?php
$n=1;
$k=0;
for ($line=0;$line<1000;$line++)
{
  echo "
";
  if ($k==0)
{
  echo "$n"; echo"	";
  for ($p=0;$p<10-$n;$p++)
  {
    echo ""; echo"	";
  }
  $n++;
  if ($n==11)
  {
    $k=1;
    $n=9;
  }
  echo"
";
}
  if ($k==1)
  {
    echo "$n"; echo"	";
    for ($column=0;$column<10-$n;$column++)
    {
      echo ""; echo"	";
    }
    $n--;
    if ($n==1)
    {
      $k=0;
    }
    echo"
";
  }
}

?>

First of all you should use a deeper indent than one space. Changing it to three spaces makes the logic more readable:

if($k==0)
{
 $k=0;
}

vs:

if($k==0)
{
     $k=0;
}

The next thing is, that you named your variables in a way, that makes them not readable. $k and $n are not readable at all. If you rename $n to $colspan and $k to $countAsc they express their meaning in your logic, which improves your code style by a lot. Other programmers (like me) can  read way easier into your code then and have a faster understanding of your formulas.

Speaking of formulas, you should not put the calculations into the echo statement, but extract them, to separate the calculation of your values from its usage. That increases the overview of your variables by alot.

$red = $colspan * 25;
$green = (10 - $colspan) * 25;
/* ... */ "background:rgb(" . $red . "," . $green . ",0);" /* ... */

Speaking of the echo statements, you can separate the PHP from the HTML code. Since PHP 5.6 this is really easy due to the short echo tags <?= 'string to echo' ?>. This also enables you to indent the HTML tags as well, giving the reader an easier way to understand your overall structure.

<?php
$red = $colspan * 25;
$green = (10 - $colspan) * 25;
?>

<td style="background-color:rgb(<?= $red ?>, <?= $green ?>, 0);" colspan="<?= $colspan ?>">
    <?= $colspan ?>

As a next step you can safely extract the building of the 	 out of the if clauses. They are equal in both cases and therefore are always echoed, no matter the if conditions. This is also true for your 	 loop. Both inner for loops are exactly the same, given the difference, that the variables have a different names. In addition to that, the $countAsc variable only has two states: Being 1 or 0. Because of that it can safely be changed to a boolean variable.

$colspan = 1;
$countAsc = TRUE;
for ($line = 0; $line < 1000; $line++): ?>

        <?php
        $red = $colspan * 25;
        $green = (10 - $colspan) * 25;
        ?>

        <td style="background-color:rgb(<?= $red ?>, <?= $green ?>, 0);" colspan="<?= $colspan ?>">
            <?= $colspan ?>

        <?php
        for ($column = 0; $column < 10 - $colspan; $column++)
        {
            ?>

            <?php
        }

        if ($countAsc) ...

After extracting all these things you can see, that the if clause only determines, whether the iteration of the $colspan variable is rising or falling. Therefore the if clause should represent only that, to make the logic more readable:

if ($countAsc && $colspan >= 10)
{
    $countAsc = false;
}
elseif (!$countAsc && $colspan <= 1)
{
    $countAsc = true;
}

After this logic is extracted, the calculation for the $colspan is that simple, that it can be shortened to the short if syntax (condition ? true value : false value;) to be in a more readable manner:

$colspan = $countAsc ? $colspan + 1 : $colspan - 1;

Defining the conditions in a more fitting way reduces the necessarity to redefine the $colspan at all and therefore makes your logic less complex. Also if you have huge for loops, it is more readable to use the for(condition): ... endfor; syntax, because like this you can see first hand what type of "closing bracket" is displayed at the end of the loop, giving the code, again, a better readability.
The whole code is:

<?php
$colspan = 1;
$countAsc = TRUE;
for ($line = 0; $line < 1000; $line++): ?>

        <?php
        $red = $colspan * 25;
        $green = (10 - $colspan) * 25;
        ?>

        <td style="background-color:rgb(<?= $red ?>, <?= $green ?>, 0);" colspan="<?= $colspan ?>">
            <?= $colspan ?>

        <?php
        for ($column = 0; $column < 10 - $colspan; $column++)
        {
            ?>

            <?php
        }

        if ($countAsc && $colspan >= 10)
        {
            $countAsc = false;
        }
        elseif (!$countAsc && $colspan <= 1)
        {
            $countAsc = true;
        }

        $colspan = $countAsc ? $colspan + 1 : $colspan - 1;
        ?>

<?php endfor ?>


Inserting a role using PDO My question has to do with :

(a) Can this be made more secure?

(b) Are there best practices in relation to setting timezone?

(c) What best approach should be taken defining use of utf8?

        $my_success = 0;
        $my_message = '';
        $my_id = 0;

        try {

            # declare & set variables using $my_env
            list ($db_dbdriver, $db_hostname, $db_database, $db_username, $db_password) = Connection::dbConnect($my_env);

            # set default timezone
            date_default_timezone_set('America/Los_Angeles');

            # create & set connection
            $db_options = array(PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8');
            $conn = new PDO($db_dbdriver . ":host=" . $db_hostname . ";dbname=" . $db_database . ";charset=utf8", $db_username, $db_password, $db_options);
            $conn->exec("set names utf8");
            $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
            $conn->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

            # Prepare an SQL statement
            $my_sql = 'INSERT INTO Role (Role_Name, Role_CDate, Role_CBy, Role_EDate, Role_EBy)
                VALUES (:Role_Name, NOW(), :Role_CBy, NOW(), :Role_EBy)';
            $stmt = $conn->prepare($my_sql);

            # bind parameters to prevent SQL Injection
            $stmt->bindValue(':Role_Name', $role_name);
            # defaults to Now()...
            $stmt->bindValue(':Role_CBy', $role_cby);
            # defaults to Now()...
            $stmt->bindValue(':Role_EBy', $role_cby);

            # execute the SQL statement
            $stmt->execute();

            # closes the cursor, enabling the statement to be executed again
            $stmt->closeCursor();

            # grab ID value of the new record
            $my_id = $conn->lastInsertId();

            # close & unset the connection
            $stmt = null;
            unset($stmt);
            $conn = null;
            unset($conn);

            # Set success value to TRUE
            $my_success = 1;
            $my_message = 'success';

        } catch(PDOException $e) {

            # Catch error message
            $error_message = $e->getMessage();

            # Append error message to error log
            # ...

            # Set success value to FALSE & return an error message
            $my_success = -1;
            $my_message = $error_message;
            $my_id = 0;

        }

There are several points which can be improved, and by the style of your code I'm assuming that you are using this project to learn OOP..?

First off, your questions:
a) Since you are not outputting anything to the browser, or anything but an SQL server, this code has very few possible attack vectors. You are using prepared statements, which means that you have correctly protected yourself against SQL-injections.
That is, as long as you've ensured that the strings are indeed valid UTF-8, before sending them over to the DB server. This is especially important if these strings comes from a browser, and not the code itself.

b) Only two, really. Always set the timezone, and make sure it's set to the same timezone everywhere (and every time) you create a new time/date object. The timezone can/should be changed after creation, if it's necessary to change it.

c) You seem to have taken the appropriate steps, at least in the code posted above. I've found a site listing the general steps for UTF-8 support in PHP scripts, which might be useful for you.

That said, here are some points that could be improved in the code posted:

First off I'd change the line retrieving the DB variables, from your $my_env array(?). I would rather implement a DB connection factory here, or even a singleton if it's a small enough project. (Or to learn singletons.)
You've set the timezone using the old date_*() functions, which aren't really recommended any more. I strongly recommend looking into DateTime class family for date-time related needs.
Even though you've set the timezone, the code you've posted doesn't actually use it. You've used SQL's NOW() function instead, which uses the DBMS' timezone. This can be an issue if the PHP code and the database server are using different timezones, and you're later on using PHP to manipulate/compare timestamps.
You don't need to close the cursor, unset variables or anything like that. PHP normally does a perfectly good job of cleaning up afterwards, and it might even be detrimental to prematurely doing "cleanup". Normally this is only done when you have a specific use-case which requires you to clean up early.
You have what is essentially three return values, to communicate whether or not the query succeeded. You only need one, which should be either "0" for not OK, or the ID for OK. Any error messages or such should be added to an error log, by a dedicated function for this.
This one ties to the above, as the code should really be made into a function/method. This will allow you to maintain the code a lot easier, and make things quite a bit more flexible.
The try-catch block encompasses a lot more code than it needs to, which makes it a bit harder to follow the control-flow than necessary. One should only encompass the functions that actually can throw exceptions, if one doesn't just let them bubble up to a layer which can handle them in the proper location in the code-flow.
The connection class you have seems to have nothing to do with neither databases or connections to do, in fact it doesn't seem to following the OOP principles either. A method that simply takes input and returns output, without altering or depending upon the state of the object, isn't OOP. It just normal procedural programming, which is better done with pure functions. ;)

Here's a quick reorganization of your code, to give an example of the points I've listed:

function add_role ($name, $cby) {
    try {
        // Create or retrieve the database connection
        $conn = DB_Factory::create ($my_env);

        // Prepare an SQL statement
        $my_sql = 'INSERT INTO Role (Role_Name, Role_CDate, Role_CBy, Role_EDate, Role_EBy)
                    VALUES (:Role_Name, NOW(), :Role_CBy, NOW(), :Role_EBy)';
        $stmt = $conn->prepare ($my_sql);

        // bind parameters to prevent SQL Injection
        $stmt->bindValue(':Role_Name', $role_name);
        // defaults to Now()...
        $stmt->bindValue(':Role_CBy', $role_cby);
        // defaults to Now()...
        $stmt->bindValue(':Role_EBy', $role_cby);

        // execute the SQL statement
        $stmt->execute();
    } catch (PDOException $e) {
        // Catch error message
        $error_message = $e->getMessage();

        // Append error message to error log
        // ...

        // Set success value to FALSE & return an error message
        return 0;
    }

    // grab ID value of the new record
    return $conn->lastInsertId();
}


Can I somehow tidy up this (overuse?) of generics? I'm building a generic flat file reader which looks something like this.

 public class GenericReader<TComposite, THeader, TData, TTrailer>
    where TComposite : GenericComposite<THeader, TData, TTrailer>, new()
    where THeader : new()
    where TData : new()
    where TTrailer : new()
{
    public TComposite Read()
    {
        // read stuff, do parsing etc
        var composite = new TComposite();

        composite.Header = new THeader();
        composite.Data = new TData();
        composite.Trailer = new TTrailer();

        return composite;
    }
}

It could be consumed like so.

var reader = new GenericReader<Composite<Header, Data, Trailer>, Header, Data, Trailer> ();

var composite = reader.Read();
Console.WriteLine(composite.Data.SomeProperty);

Console.ReadLine();

Here are the classes used.

public class Composite<THeader, TData, TTrailer> : GenericComposite<THeader, TData, TTrailer>
{

}

public class GenericComposite<THeader, TData, TTrailer>
{
    public THeader Header { get; set; }

    public TData Data { get; set; }

    public TTrailer Trailer { get; set; }
}

public class Header {
    public string SomeProperty { get { return "SomeProperty"; } }
}

public class Data {
    public string SomeProperty { get { return "SomeProperty"; } }
}

public class Trailer {
    public string SomeProperty { get { return "SomeProperty"; } }
}

Is there a way how I could remove or encapsulate that generic type information in the GenericReader? I'm looking for an extra pair of eyes to show me something what I've been missing. We already did something with returning interfaces, and making the consumer do a cast, but that just moves the responsibility to the wrong location in my opinion, plus there is a small performance penalty.

Thanks.

Having so many new operations -- seems like you'd want to use some kind of a factory, injected in the constructor, or the instances themselves injected in the constructor, instead of constructing the classes inside the constructor.


Generic number class code I just made a generic number class:

From any ordered character list, make number class representing that ordered character list.
create_number_class("01") returns binary number class
create_number_class("0123456789") returns decimal number class
create_number_class("abcdefghij") return decimal number class but representing each digit as a alphabet.

Here is the generic number class definition. I think it is well-made class definition. Is there something needed for improvement in that class definition?

Example:

ABC_Class = create_number_class("abc")
x = ABC_Class("baa")
y = ABC_Class("bbb")
print(x+y)
#output digits: abc, v: cbb, decimal_v: 22

Here is the class definition:

def create_number_class(alphabet):
    class temp(object):
        digits = alphabet
        def __init__(self, v):
            self.v = v
            self.decimal_v = self.to_decimal(self)
        @staticmethod
        def to_decimal(self):
            r = 0
            for i in range(0, len(self.v)):
                r += len(temp.digits)**(len(self.v)-i-1)*(temp.digits.index(self.v[i]))
            return r
        @classmethod
        def from_decimal(cls, decimal_v):
            r = []
            mod = len(temp.digits)
            if decimal_v < mod:
                return cls(temp.digits[decimal_v])
            while True:
                remainder = decimal_v % mod
                r.append(remainder)
                decimal_v = int((decimal_v - remainder)/ mod)
                if decimal_v < mod:
                    r.append(decimal_v)
                    break
            r = "".join(list(reversed([temp.digits[x] for x in r])))
            #r = "".join(list(reversed([str(temp.digits.index(str(x))) for x in r])))
            return cls(r)
        def __add__(self, other):
            return temp.from_decimal(self.decimal_v+other.decimal_v)
        def __sub__(self, other):
            return temp.from_decimal(self.decimal_v-other.decimal_v)
        def __mul__(self, other):
            return temp.from_decimal(self.decimal_v*other.decimal_v)
        def __floordiv__(self, other):
            return temp.from_decimal(self.decimal_v//other.decimal_v)
        def __str__(self):
            return "digits: {}, v: {}, decimal_v: {}".format(temp.digits, self.v, self.decimal_v)
        def convert_to(self, new_class):
            return new_class.from_decimal(self.decimal_v)
    return temp

Here is an example:

BinClass = create_number_class("01")
DecimalClass = create_number_class("0123456789")

x = BinClass("111")
x = BinClass("1000")
y = BinClass("10")

HexClass = create_number_class('0123456789ABCDEF')
x = HexClass('1')
y = HexClass('AA')

print(x+y)
print(x-y)
print(x*y)
print(x//y)
print(x.convert_to(DecimalClass))
isinstance(x, BinClass)

Your code looks very good for me, what I found are just some minor improvements:

You do not need to inherit from object so you can just do class temp:
I would suggest to change temporary class name to something else since when you do print(type(x)) I would rather see something more informative than <class '__main__.create_number_class.<locals>.temp'>. I would suggest to add another argument for your method called className and set it dynamically as shown here
New lines after each method would be appreciated since it would make code more readable