Getting list of colours from image in lab format I need to write a bit of code that will get a list of the RGB colours in an image and then translate them into a NumPy array of lab colours. I have managed to do this, but I would like to learn how I can do it more efficiently.

from skimage.color import rgb2lab
from skimage.io import imread
import numpy as np
from PIL import Image  # @UnresolvedImport

def get_histogram (img):

    #histogram = plt.hist(img.flatten(), bins=100, facecolor='green', alpha=0.75)
    w, h = img.size
    colours = img.getcolors(w*h)  #Returns a list [(pixel_count, (R, G, B))]
    num, colours_rgb = zip(*colours)
    r,g,b = zip(*colours_rgb)

    num_of_colours = len(r)
    w2,h2 = 1,num_of_colours
    data = np.zeros( (w2,h2,3), dtype=np.uint8)
    print(data.shape)
    data[0,:,0] = r
    data[0,:,1] = g
    data[0,:,2] = b
    print(data)
    colours_lab = rgb2lab(data)

There's no docstring for get_histogram. What does this function do? What kind of object should I pass for the img argument?
You import skimage.io.imread and PIL.Image but you don't use either of them.
This comment doesn't seem relevant:

#histogram = plt.hist(img.flatten(), bins=100, facecolor='green', alpha=0.75)

There are two useless print statements. I presume that these are left over from a debugging session and you forgot to remove them. You might find it useful to learn to use the Python debugger which would avoid the need to add print statements to the code.
There are two pieces of functionality here: (i) extracting the colour data from an image into a Numpy array; (ii) converting colour data from RGB to the Lab colour space. It would make sense to split these up (especially as piece (ii) is so simple).
The function is poorly named: it does not actually get a histogram. (Because you discard the colour counts in the num array.) This makes me wonder what you are using this function for.
You use the spelling "colour" but the APIs you are calling use the spelling "color". Even if you prefer the "colour" spelling, it's better to be consistent. That way there's less chance of forgetting and making a mistake.
Creating a Numpy array from an a Python list is usually straightforward if you pass the list to the numpy.array function. There's no need to mess about with numpy.zeros and assigning column-wise.

So I'd write the following:

import numpy as np

def distinct_colors(img):
    """Return the distinct colors found in img.
    img must be an Image object (from the Python Imaging Library).
    The result is a Numpy array with three columns containing the
    red, green and blue values for each distinct color.

    """
    width, height = img.size
    colors = [rgb for _, rgb in img.getcolors(width * height)]
    return np.array(colors, dtype=np.uint8)

The conversion to Lab colour space is so simple that I don't think it needs a function.

Update

There's one minor difficulty: the skimage.color functions all demand an array with 3 or 4 dimensions, and so only support 2- and 3- dimensional images. Your array of distinct colours is a 1-dimensional image, so it's rejected. But you can easily use numpy.reshape to turn it into a 2-dimensional image whose first dimension is 1 before passing it to rgb2lab, like this:

colors = distinct_colors(img)
rgb2lab(colors.reshape((1, -1, 3)))

Or if you prefer the second dimension to be 1, use .reshape((-1, 1, 3)).

(Personally I think the skimage.color behaviour is absurd. Why does it care about the number of dimensions? There doesn't seem to be any obvious reason in the code. Maybe it would be worth filing a bug report?)


Deploying treasure in 4×4 board and finding them in Python Before starting I would like to say that I just made this program after studying python for 5 hours for the first time so I am totally new to this programming language.

This program tries to create 4X4 board denoting it with 0 everywhere. Now to deploy treasure, it randomly selects 9 position and deploy them on the board by denoting it with  '@'. Then tries to detect neighboring treasure which are there around vacant lands and counts them and then show them on those vacant lands area. It is just like Minesweeper game.

Since I am new to this python language so I would like to know that what changes do I have to make so that it will look like a Python program and what else I can do to make it work much better.

from random import randint

print "Welcome to Treasure Hunt \n"

q="@"     #For checking the treasure
board = []  #Board

for x in range(0,4):
    board.append(["0"] * 4)   # It creates 4x4 Board with 0 everywhere

treasure = 9
while treasure:                # Randomly spread treasure
 r = randint(0, len(board)-1)
 c = randint(0, len(board[0])-1)
 if board[r][c] != "@":
        board[r][c]="@"
        treasure -= 1

print "-------------------------------"
print "\n Treasures are here "
print "\n-------------------------------\n"
for row in board:
    print "     ".join(row)                  # Give output of board
    print "\n"

def hunt(a,b):              #Hunt function searches for treasure around the given land
    first = a-1
    second = b-1
    third = a+1
    fourth = b+1
    if(first < 0):
        first = first + 1;
    if(second < 0):
        second = second + 1;
    if(third == 4):
        third = third - 1;
    if(fourth == 4):
        fourth = fourth -1;

    h=0
    for i in range(first,third + 1):
        for j in range(second,fourth + 1):
            if(q==board[i][j]):
                h=h+1;
                board[a][b] = h

for k in range(len(board)):
    for l in range(len(row)):
        if(q!=board[k][l]):
            hunt(k,l)

for row in board:
    print "   ".join(map(str,row))

Styling

The Python style guide, PEP 8, has some good things to say.  I have found very few things in it of which I do not approve.  Certainly there are some that I don't agree with, but for the most part it makes your code easier to understand if you follow its guidelines.

These two rules from PEP 8 you aren't following:

  Always surround these binary operators with a single space on either side: assignment (=), augmented assignment (+=, -= etc.), comparisons (==, <, >, !=, <>, <=, >=, in, not in, is, is not), Booleans (and, or, not).

and

  Use 4 spaces per indentation level.

You break that last in only one place: in your while treasure: loop.  It might be just a typo, but even typos sometimes break your program.

Using first = a-1 is not against PEP 8, but I think it looks better to use first = a - 1.

Naming

You have many variables with names that are almost or completely meaningless.  For example, r = randint(0, len(board)-1).  If I look down a couple lines, I can gather that r stands for row; but what's wrong with naming it row in the first place?  Each line of code should be semi-readable without any other lines as context.  You use just about half of the letters in the alphabet as variable names, but you have only about five names that are actually descriptive.  You use first, second, third, and fourth; but what are they really?  Are they perhaps row_start, row_end, column_start, and column_end?

Logic

I'm not sure; maybe this should go under Styling, but I think you should use parentheses in your print statements.  Python3 has print as a function so they are required.  They are allowed in Python2, so I usually use them.  It can trip you up sometimes if you do print("Your score was", 45), say, because it would print ('Your score was', 45) instead of Your score was 45.  Since you aren't using comma-separated values in your print calls, everything should look exactly the same.  You may want to put from __future__ import print_function at the top so that if you add something later that uses comma-separated values, it will still print out all right.

q = "@"

Why do you use "@"?  I'm guessing that you use it because "@" is the default space-filler.  You use "@" in many places, so what if you decided to use "%" instead?  That would require a lot of modification.  What you have right now is called hard-coded values.  Instead of that, define a constant at the beginning of the file that defines which character it is, and use that constant instead of saying "@" explicitly.

board = []
for x in range(0, 4):
    board.append(["0"] * 4)

range() by default starts at 0, so you could use for x in range(4): instead.  Since you aren't using x, the standard name is _.  The easy way to define board would be to use a list comprehension:

board = [["0"] * 4 for _ in range(4)]

r = randint(0, len(board)-1)

There is another function in the random module that is closer to what you want: randrange().  Of course, you would need to change from random import randint to from random import randrange, but the code would be:

row = randrange(len(board))
column = randrange(len(board[row]))

first = a-1
second = b-1
third = a+1
fourth = b+1
if(first < 0):
    first = first + 1;
if(second < 0):
    second = second + 1;
if(third == 4):
    third = third - 1;
if(fourth == 4):
    fourth = fourth -1;

For one thing, those semicolons are redundant.  You could also shorten your code using max() and min():

first = max([a - 1, 0])
second = max([b - 1, 0])
third = min([a + 1, 4])
fourth = min([b + 1, 4])

But why do you use 4?  What's special about it?  Is that another hard-coded value?

All together, you have done very well for studying only five hours.  I haven't seen you on StackOverflow, so you appear to be quite good at researching what you need.  I believe that your main problem is that you are too focused on the project at hand.  You haven't made it easy to change little details about the program, and you haven't made it easy for other people to understand your code.  PEP 8 mentions that code is read much more often than it is written.  If you came back in five years, would you be able to explain the program to someone else?


Select highest and second highest values i've written a procedure to where it selects the first and second highest values not already selected (as designated by previous highlights of green and yellow). The first highest ignores anything already highlighted in green or yellow and selects the highest value. The second highest value ignores anything highlighted in green or yellow AND ignores the first highest highlighted in red. It then designates each selection with a 1 for 1st highest and 2 for second highest. It works but the problem is... it takes 1 minute 30 seconds to run, which isn't ideal. Is there another way I can write this? Please see below and thank you.

Sub Macro1()
' Select highest value not already selected (as designated by highlights green and yellow)

Sheets("sheet1").Select
Dim rng As Range, s As Range
Set rng = Range("C2:D500000")
Set s = Range("C2")
For Each r In rng
If r.Value > s And r.EntireRow.Interior.ColorIndex <> 4 And r.EntireRow.Interior.ColorIndex <> 6 Then Set s = r
Next r
For Each r In rng
If s.Value = r.Value And r.EntireRow.Interior.ColorIndex <> 4 And r.EntireRow.Interior.ColorIndex <> 6 Then r.EntireRow.Interior.ColorIndex = 22
Next r

' Select second highest value not already selected (as designated by highlights green, yellow, and red, which is first highest)

Dim rng2 As Range, q As Range
Set rng2 = Range("C2:D500000")
Set q = Range("C2")
For Each r In rng2
If r.Value > q And r.EntireRow.Interior.ColorIndex <> 22 And r.EntireRow.Interior.ColorIndex <> 4 And r.EntireRow.Interior.ColorIndex <> 6 Then Set q = r
Next r
For Each r In rng2
If q.Value = r.Value And r.EntireRow.Interior.ColorIndex <> 22 And r.EntireRow.Interior.ColorIndex <> 4 And r.EntireRow.Interior.ColorIndex <> 6 _
Then r.EntireRow.Interior.ColorIndex = 20
Next r

'Designates 1st highest with the number 1

Dim rg As Range

Set rg = Range("V2:V500000")
For Each cell In rg
If cell.EntireRow.Interior.ColorIndex = 22 Then
cell.Offset(0, 1) = "1"
End If
Next

'Designates second highest with the number 2

Set rg3 = Range("V2:V500000")
For Each cell In rg3
If cell.EntireRow.Interior.ColorIndex = 20 Then
cell.Offset(0, 1) = "2"
End If
Next
End Sub

I've done my best to refactor your code in such a way that you can use it to learn to improve your coding, but also in such a way that it all makes sense. I'll post it and then explain the process and some tips.

The Code

Option Explicit
Sub HighlightHighestValueRows()
    ' Select highest value not already selected (as designated by highlights green and yellow)

    Dim TargetSheet As Worksheet

    ' Use an Explicit reference to the sheet you are operating on
    ' This will find the sheet named "Sheet1" within the workbook that the code is run in.
    Set TargetSheet = ThisWorkbook.Sheets("Sheet1")

    ' Use more descriptive variable names.
    Dim SearchRange As Range
    Set SearchRange = TargetSheet.Range("C2:D500000")

    ' Because of the variable name, I have no clue what this is supposed to be.
    ' Keep this in mind when coding.
    Dim FoundCell As Range
    Set FoundCell = TargetSheet.Range("C2")

    ' r is undeclared which means its a Variant.
    ' I've declared it and given it a more proper name to prevent errors.

    ' Notice how the code makes sense as you read it with more descriptive naming conventions.
    ' Any reader will see this and know we are looping through single cells within
    ' a larger range (at least any reader with some VBA knowledge).

    Dim SearchCell As Range
    For Each SearchCell In SearchRange
        ' I prefer to avoid arrow code (which is what I turned your And's into) but it is more efficient overall
        ' to use nested If blocks versus using a longer AND statement. It also makes more sense to the reader.
        If SearchCell.Value > FirstCell.Value Then
            If SearchCell.EntireRow.Interior.ColorIndex <> 4 Then
                If SearchCell.EntireRow.Interior.ColorIndex <> 6 Then
                    Set FoundCell = SearchCell
                End If
            End If
        End If
    Next SearchCell

    ' Even after refactoring this loop I dont see what you're trying to accomplish. Are you highlighting all rows that match
    ' the value of the highest cell value? If so, why not just store the highest value in a variable? Something like:
    ' Dim Foo as Double
    ' Foo = FirstCell.Value
    ' Then much later
    ' If SearchCell.Value = Foo Then DoSomething

    For Each SearchCell In SearchRange
        If FoundCell.Value = SearchCell.Value Then
            If SearchCell.EntireRow.Interior.ColorIndex <> 4 Then
                If SearchCell.EntireRow.Interior.ColorIndex <> 6 Then
                    SearchCell.EntireRow.Interior.ColorIndex = 22
                End If
            End If
        End If
    Next SearchCell

    ' Select second highest value not already selected (as designated by
    ' highlights green, yellow, and red, which is first highest)

    ' No need to declare a new range variable since the first one doesnt need to be reused.

    Set SearchRange = TargetSheet.Range("C2:D500000")
    Set FoundCell = TargetSheetRange("C2")
    For Each SearchCell In SearchRange
        If r.Value > q Then
            If r.EntireRow.Interior.ColorIndex <> 22 Then
                If r.EntireRow.Interior.ColorIndex <> 4 Then
                    If r.EntireRow.Interior.ColorIndex <> 6 Then
                        Set FoundCell = SearchCell
                    End If
                End If
            End If
        End If
    Next SearchCell
    For Each SearchCell In SearchRange
        If FoundCell.Value = SearchCell.Value Then
            If SearchCell.EntireRow.Interior.ColorIndex <> 22 Then
                If SearchCell.EntireRow.Interior.ColorIndex <> 4 Then
                    If SearchCell.EntireRow.Interior.ColorIndex <> 6 Then
                        SearchCell.EntireRow.Interior.ColorIndex = 20
                    End If
                End If
            End If
        End If
    Next SearchCell

    'Designates 1st highest with the number 1
    Set SearchRange = TargetSheet.Range("V2:V500000")
    For Each SearchCell In SearchRange
        If SearchCell.EntireRow.Interior.ColorIndex = 22 Then
            ' Be sure to qualify your reference to the value of the cell
            SearchCell.Offset(0, 1).Value = "1"
        End If
    Next

    'Designates second highest with the number 2
    Set SearchRange = TargetSheet.Range("V2:V500000")
    For Each SearchCell In SearchRange
        If SearchCell.EntireRow.Interior.ColorIndex = 20 Then
            SearchCell.Offset(0, 1).Value = "2"
        End If
    Next
End Sub

Option Explicit

I cannot stress the importance of this being at the top of ALL of your code modules. What Option Explicit does is it causes a compile-time error when a variable is undeclared. This prevents run-time errors when you try to use variables that aren't set properly, and it will save you countless hours of debugging time.

Go To Tools > Options Editor > Require Variable Declaration. You'll thank yourself later.

Variable Naming

Not to be a jerk here, but your variable names are horrible and difficult to understand. You have rng, rng2, rg, q, s, r, cell and the worst part is that you don't need all of them. I declared four variables total (and one of those you didnt even have a variable for, but you did need).

Here's a tip, make sure your variables make sense. You should be able to look at them and have a pretty good idea of what they do. SearchRange is easy to understand: it is the range we are searching in. SearchCell is just as easy to understand: it is the cell we are currently searching in. Finally, FoundCell, you guessed it, is the cell we found using our criteria. It is pretty much bulletproof when you start to code like this (actually, not really. You'll learn that bulletproof coding takes a lot more work than a handful of good variables.) and it makes your code easy to maintain.

Additionally, there is no need to declare two variables if you need one. In other words, if SearchRange is used only for the first loop, and never needed after that, I dont have to make a NewSearchRange for the second loop. I can just use SearchRange.

General Names

Just as Variables need good names, so do subroutines. It is all too easy to take the Macro1 that the recorder gives us, and to leave it this way. When you have larger projects, Macro1 and Macro2 become undistinguishable, and god forbid you have a Module1 and Module2 as well. Name everything. That's pretty much my mantra. From Worksheets to Tables, and PivotTables, and Workbooks, to code modules, subs, functions, etc., give it a proper name. The sooner you get in the habit, the more time you'll save.

Implicit Object References

You'll likely hear this one a lot, so it is important to learn. Whenever you are working on an object, you must consider the scope of that object. While Range("A1") looks fine to the untrained eye, anyone worth their salt in the VBA community will point out that it is a unqualified Range reference. Even worse, is if we have Foo = Range("A1") and one step further is this nasty bit:

Dim Foo
Foo = Range("A1")

It looks harmless right? Wrong. Foo is Dim'ed but it isnt given a type so it is now the dangerous Type Variant which is a known problem-child. Variants are great when you need them, but they have a habit of rearing their ugly head for those knew to the language. But wait, there's more! Range("A1") is unqualified on both sides. So this means it is actually:

Foo = ActiveSheet.Range("A1").Value

Which is fine if it is what you meant but it may not be what you meant. The thing is, not only do you not know what the Activesheet is at the time of the code running (unless you Activate it, which is it's own problem) but you also don't have control over the type going into Foo. It could be a String, or an Integer, or a Double. If you use Set Foo it'll be a Range. Foo will happily take anything you give it, which is a problem.

To solve this, be smart about your code:

Dim Baz as Worksheet

' I use ActiveSheet, but you can use any sheet reference here.
' For example, 'ThisWorkbook.Sheets("Bar")' is quite common.
Set Baz = ActiveSheet

Dim Foo as String
Foo = Baz.Range("A1").Value

Know I know **exactly* what my code is doing, and I can debug it much faster if something breaks.

Indentation

This is a pet peeve of mine, and it is a pet peeve of the community as well. Always indent your code (properly) and never post code on SO that is not indented. We will be cursing your name as we indent it ourselves (or, even more frequently, we just refuse to help). A lack of indentation will make bad code even harder to read.

Shameless Plug For Rubberduck

Mat's Mug and his team have a tool called 'Rubberduck' that goes a long way towards enabling you to fix these problems. Everything from indentation, to variable naming and variable use, even to Function and Subroutine calls. I highly suggest checking it out: http://rubberduckvba.com/ .

In Closing

Whatever you do, don't simply copy and paste my code, throw it into your module, and just go on your merry way. I spent time on this to help you learn, and my hope is that six months from now you will be in my shoes. If you do decide to forego the learning opportunity here, you will miss out on all of the things VBA can really offer.

Best of luck!


Replacing birth year column values with calculated age value I have a stored procedure that is taking more than 2 hours to complete and I noticed the below SQL is taking close to 25 mins for updating a million+ values.  There are multiple steps like this and all added up takes around 2+ hours to finish.

All it is doing is taking a column value i.e. birth_year and replacing it with age i.e. getdate() - birth_year.

SELECT     id,
           birth_month,
           birth_year
INTO       #birth_date
FROM       table_name
WHERE      birth_year IS NOT NULL

UPDATE     table_name
SET        birth_year = ( YEAR(GETDATE()) - bd.birth_year )
FROM       #birth_date bd
INNER JOIN table_name ap ON ap.id= bd.id
WHERE      bd.birth_month <= MONTH(GETDATE())
           OR bd.birth_month IS NULL

UPDATE     table_name
SET        birth_year = ( YEAR(GETDATE()) - bd.birth_year - 1 )
FROM       #birth_date bd
INNER JOIN table_name ap ON ap.id= bd.id
WHERE      bd.birth_month > MONTH(GETDATE())

I replaced the above SQL with the below SQL and now instead of 25 mins, the below SQL takes less than a minute. I've verified the results and all seems good.  Is there anything that I am missing? Is there a better or alternate way?

UPDATE  table_name
SET     birth_year = case
                        when birth_month <= MONTH(GETDATE())  OR birth_month IS NULL then (YEAR(GETDATE()) - birth_year)
                        when birth_month > MONTH(GETDATE()) then ( YEAR(GETDATE()) - birth_year - 1 )
                        else null
                     end
FROM    table_name

First, as dnoeth commented, why are you doing this job in mass rather than calculating on the fly? You might have a good reason, but you never clarified.  Of course the best way to lighten the load would be not to do this task at all.

Second, as 200_success commented, you are damaging your database design by writing age data into a birth_year column.

If you MUST run this procedure and you MUST overwrite the birth_year, then I have some advice for your single-query approach:

You should consistently use all caps when writing mysql keywords and functions for readability.
You should try to minimize the number function calls and overall length of your query.
Because, I assume, you have covered all potential outcomes with your table data in your CASE conditions, there should be no logical requirement for the null outcome.  Furthermore, because there are only 2 outcomes, I would opt for an IF statement rather than the more verbose CASE statement.
Depending on the possibility of a null value in birth_year, you may need to include another IF or COALESCE call to mitigate that data challenge.

SQL (D.R.Y.):

UPDATE table_name
SET birth_year = YEAR(GETDATE()) - birth_year - IF(birth_month IS NULL OR birth_month <= MONTH(GETDATE()), 0, 1)
FROM table_name


Hardware interrupt handler in C++ I wrote this class to use a C++ function as hardware interrupt handler. The idea is to have a single entry point where I can switch stacks, and call a chain of std::function objects to handle the interrupt.

It works... but I think it's a pretty ugly hack. Is there anything I could improve here?

class __attribute__((packed)) interrupt_wrapper
{
    using function_ptr = void(*)(unsigned);
    unsigned int_vector;            // [eax-0x1C]
    selector ds;                    // [eax-0x18]
    selector es;                    // [eax-0x16]
    selector fs;                    // [eax-0x14]
    selector gs;                    // [eax-0x12]
    function_ptr entry_point;       // [eax-0x10]
    std::array<byte, 0x40> code;    // [eax-0x0C]

public:
    interrupt_wrapper(unsigned vec, function_ptr f) : int_vector(vec), entry_point(f)
    {
        byte* start;
        std::size_t size;
        asm volatile (
            ".intel_syntax noprefix;"
            "jmp interrupt_wrapper_end%=;"
            // --- \/\/\/\/\/\/ --- //
            "interrupt_wrapper_begin%=:;"   // On entry, the only known register is CS.
            "push ds; push es; push fs; push gs; pusha;"    // 7 bytes
            "call get_eip%=;"  // call near/relative (E8)   // 5 bytes
            "get_eip%=: pop eax;"           // Pop EIP into EAX and use it to find our variables
            "mov ds, cs:[eax-0x18];"        // Restore segment registers
            "mov es, cs:[eax-0x16];"
            "mov fs, cs:[eax-0x14];"
            "mov gs, cs:[eax-0x12];"
            "push cs:[eax-0x1C];"           // Pass our interrupt number along
            "call cs:[eax-0x10];"           // Call the entry point
            "add esp, 4;"
            "popa; pop gs; pop fs; pop es; pop ds;"
            "sti;"                          // IRET is not guaranteed to set the interrupt flag.
            "iret;"
            "interrupt_wrapper_end%=:;"
            // --- /\/\/\/\/\/\ --- //
            "mov %0, offset interrupt_wrapper_begin%=;"
            "mov %1, offset interrupt_wrapper_end%=;"
            "sub %1, %0;"                   // size = end - begin
            ".att_syntax prefix"
            : "=r" (start)
            , "=r" (size));
        assert(size <= code.size());

        auto* ptr = memory_descriptor(get_cs(), start).get_ptr<byte>(); // Get near pointer to cs:[start]
        std::copy_n(ptr, size, code.data());

        asm volatile (
            ".intel_syntax noprefix;"
            "mov ax, ds;"
            "mov bx, es;"
            "mov cx, fs;"
            "mov dx, gs;"
            ".att_syntax prefix"
            : "=a" (ds)
            , "=b" (es)
            , "=c" (fs)
            , "=d" (gs));
    }

    auto get_ptr(selector cs) { return far_ptr32 { cs, reinterpret_cast<std::size_t>(code.data()) }; }
};

We're missing some pieces, such as the memory_descriptor and far_ptr32 classes and get_cs function and I guessed at the definitions of selector and byte, but with that said, here's what I found that may help you improve your code.

Make the constructor noexcept

It's unlikely that an exception within the constructor would be a welcome event.  For that reason, your constructor should be declared noexcept.

Make function_ptr noexcept

For similar reasons, I'd suggest making function_ptr also noexcept:

using function_ptr = void(*)(unsigned) noexcept;

Be careful with interrupt flags

Instead of disabling all interrupts using sti, it might be better to restore the interrupt flag to whatever it had been.  Although the interrupt could not have been called by hardware if the interrupt flag had been reset (that is, interrupts disabled) but it's not uncommon to chain interrupts by having one hardware interrupt chain to another via explicit call.  In that circumstance, the hardware flag should not be re-enabled lest the interrupt chain be incorrectly interrupted.


Initializing a game in SDL I have recently learnt about RAII and exceptions and I have been trying to implement them into my code, but I have met an impasse.

With this code here what I am trying to do is manage everything that is being created so that there is nothing left over for when the program ends. I have deliberately set it so that media throws an exception. But before I continue on with more code, I am wondering if this general way of doing things is correct and if are there any other/better approaches to what I am doing.

The two classes that confuse me most are Media and Game. In their destructor, they have it stated that when they go out of scope. They should inform the console that they have gone out of scope, but it isn't currently doing that.

main.cpp

#include "Enemy.h"
#include "Media.h"
#include "Window.h"
#include "Game.h"

#include <SDL.h>
#include <iostream>

int main(int argc, char *args[])
{
    try
    {
        Window window;
        Media media;
        Game game;
    }
    catch (const std::exception &e)
    {
        std::cout << e.what();
        std::cout << "\nProgram failed.";
    }

    char c;
    std::cin >> c;
    return 0;
}

Window.h

#pragma once
#include <SDL.h>
#include <memory>
#include <string>
class Window
{
public:
    Window();
    ~Window();

    SDL_Window* getWindow() const { return m_window; }
    SDL_Surface* getWindowSurface() const { return m_windowSurface; }

    static constexpr int SCREEN_WIDTH = 800;
    static constexpr int SCREEN_HEIGHT = 800;

private:

    void createWindow();
    void CreateWindowSurface();
    SDL_Window* m_window = nullptr;
    SDL_Surface* m_windowSurface = nullptr;
};

Window.cpp

#include "Window.h"
#include <iostream>
Window::Window()
{
    createWindow();
    CreateWindowSurface();
}

Window::~Window()
{
    SDL_DestroyWindow(m_window);
    std::cout << "Window destroyed.\n";
}

void Window::createWindow()
{
    m_window = SDL_CreateWindow("Snake", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, SCREEN_WIDTH, SCREEN_HEIGHT, SDL_WINDOW_SHOWN);
    std::cout << "Window created.\n";
    if (m_window == nullptr) {
        throw(std::exception("Failed to initialize Window.\n"));
    }
}

void Window::CreateWindowSurface()
{
    m_windowSurface = SDL_GetWindowSurface(m_window);
    if (m_windowSurface == nullptr) {
        SDL_DestroyWindow(m_window);
        throw(std::exception("Failed to get window surface.\n!"));
    }
}

Media.h

#pragma once
#include <SDL.h>
class Media
{
public:
    Media();
    ~Media();

private:
    void loadSurface();

    SDL_Surface* m_playerSurface = nullptr;
};

Media.cpp

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

Media::Media()
{
    loadSurface();
    //Load all other textures that are associated with the game
}

Media::~Media()
{
    std::cerr << "Media destroyed.\n";
}

void Media::loadSurface()
{
    m_playerSurface = SDL_LoadBMP("Origrimmar.bmp");
    if (m_playerSurface == nullptr) {
        throw(std::exception("Cannot load player texture"));
    }
}

Game.h

#pragma once
#include <SDL.h>
class Game
{
public:
    Game();
    ~Game();

};

Game.cpp

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

Game::Game()
{
    std::cerr << "Game created.\n";
}

Game::~Game()
{
    std::cerr << "Game destroyed.\n";
}

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

Use all required #includes

The Media.cpp file uses a std::exception but is missing this line:

#include <exception>

Use std::runtime_error

The code in Media.cpp includes this line:

throw(std::exception("Cannot load player texture"));

However, that shouldn't compile because there is no constructor for std::exception that takes a const char * argument.  Instead, that should probably be this:

throw(std::runtime_error("Cannot load player texture"));

Don't forget SDL_Init

Any SDL2 program must call SDL_Init but yours doesn't.  To fix that, I'd recommend adding an SDL object as illustrated on the SDL Wiki.  It simply makes an object whose constructor calls SDL_Init and whose destructor calls SDL_Quit while translating SDL error into C++ exceptions.

Remember object scoping rules

Right now, your objects are all declared within the try block.  This also means that they only exist within that block because of C++ scoping rules.  In a real version of the program, you'd either need to have something happen inside that try block.


A PHP Toolkit of some functions Any major security risks? And please don't get angry over my novice log system.

<?php
   function makesalt($lg)
    {
    $chars = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@#$%^&*()-';
    $sz = strlen( $chars );
    $str = '';
    for( $i = 0; $i < $lg; $i++ ) {
        $str .= $chars[ rand( 0, $sz - 1 ) ];
    }
    return $str;
  }
  function duelsha($th)
  {
      $tem = hash('sha512', $th);
      $tem2 = md5($tem);
      return hash('sha256', $tem2);
  }
  function shasalt($ht)
  {
      $salt = makesalt(30);
      return duelsha($ht . $salt) . '|' . $salt;
  }
  function mysqlsan($ss)
  {
      if (get_magic_quotes_gpc()) $ss = stripslashes($ss);
      return mysql_real_escape_string($ss);
  }
  function htent($st)
  {
      return htmlentities($st);
  }
  function sqlhtml($sm)
  {
       return htent(mysqlsan($sm));
  }
  function logs($tl)
  {
    $fh = fopen("server.log", 'w') or die("File error");
    fwrite($fh, $tl) or die("File error");
    fclose($fh);
  }
?>

You home brewed security hashes and such are a big NO. Please, check here and here for a good read regarding that. Also, please do not hash a hash, that can lead to collisions and should be avoided! Using a method such as password_hash() creates a salt for you, therefore you shouldn't have to make one on your own.

You're using mysql_real_escape_string() which is not the way to go. If possible, move away from that and onto mysqli or PDO.

And then your function htent() is sort of redundant. You don't have anything else in the function, so it shouldn't be needed.


Data import "upsert" from a SSIS package I have the following "upsert" trigger in SQL Server 2005, which is handling a large data import from a SSIS package. It is taking a very long time to complete - I suspect due to the concatenation and CASTing of keys.

Is there a better, faster way of doing this?

CREATE TRIGGER [dbo].[MP_VOD_UPSERT]
ON  [dbo].[vwMP_VOD]
INSTEAD OF INSERT
AS
BEGIN

  SET NOCOUNT ON;

  UPDATE [dbo].[vwMP_VOD]
  SET
  [Org] = inserted.Org
  ,[id_vend] = inserted.id_vend
  ,[id_org_orig] = inserted.id_org_orig
  ,[id_vchr_nbr] = inserted.id_vchr_nbr
  ,[id_vchr_line] = inserted.id_vchr_line
  ,[id_vchr_line_dist] = inserted.id_vchr_line_dist
  ,[id_glac_exp] = inserted.id_glac_exp
  ,[text_ref_4] = inserted.text_ref_4
  ,[amt_exp_hcur] = inserted.amt_exp_hcur
  ,[amt_rem_hcur] = inserted.amt_rem_hcur
  ,[id_tax] = inserted.id_tax
  ,[code_type_chrg] = inserted.code_type_chrg
  ,[id_vchr_line_chrg] = inserted.id_vchr_line_chrg
  FROM inserted
  WHERE inserted.ORG + inserted.ID_VEND + inserted.ID_ORG_ORIG + CAST(inserted.ID_VCHR_NBR AS VARCHAR) + CAST(inserted.ID_VCHR_LINE AS VARCHAR) + CAST(inserted.ID_VCHR_LINE_DIST AS VARCHAR) + inserted.CODE_TYPE_CHRG + inserted.ID_VCHR_LINE_CHRG IN
  (
    SELECT ORG + ID_VEND + ID_ORG_ORIG + CAST(ID_VCHR_NBR AS VARCHAR) + CAST(ID_VCHR_LINE AS VARCHAR) + CAST(ID_VCHR_LINE_DIST AS VARCHAR) + CODE_TYPE_CHRG + ID_VCHR_LINE_CHRG
    FROM inserted
    INTERSECT
    SELECT ORG + ID_VEND + ID_ORG_ORIG + CAST(ID_VCHR_NBR AS VARCHAR) + CAST(ID_VCHR_LINE AS VARCHAR) + CAST(ID_VCHR_LINE_DIST AS VARCHAR) + CODE_TYPE_CHRG + ID_VCHR_LINE_CHRG
    FROM [dbo].[vwMP_VOD]
  )

  INSERT INTO [dbo].[vwMP_VOD]
  SELECT * FROM inserted
  WHERE ORG + ID_VEND + ID_ORG_ORIG + CAST(ID_VCHR_NBR AS VARCHAR) + CAST(ID_VCHR_LINE AS VARCHAR) + CAST(ID_VCHR_LINE_DIST AS VARCHAR) + CODE_TYPE_CHRG + ID_VCHR_LINE_CHRG IN
  (
    SELECT ORG + ID_VEND + ID_ORG_ORIG + CAST(ID_VCHR_NBR AS VARCHAR) + CAST(ID_VCHR_LINE AS VARCHAR) + CAST(ID_VCHR_LINE_DIST AS VARCHAR) + CODE_TYPE_CHRG + ID_VCHR_LINE_CHRG
    FROM inserted
    EXCEPT
    SELECT ORG + ID_VEND + ID_ORG_ORIG + CAST(ID_VCHR_NBR AS VARCHAR) + CAST(ID_VCHR_LINE AS VARCHAR) + CAST(ID_VCHR_LINE_DIST AS VARCHAR) + CODE_TYPE_CHRG + ID_VCHR_LINE_CHRG
    FROM [dbo].[vwMP_VOD]
  )

END

I can pretty much guarantee it's the concatenation that's the problem.  Use the columns as columns; you don't want to handcuff the optimizer when it puts together your query plan.  Here's a pretty typical "upsert" pattern:

  UPDATE [dbo].[vwMP_VOD]
  SET [Org] = inserted.Org
  ,[id_vend] = inserted.id_vend
  ,[id_org_orig] = inserted.id_org_orig
  ,[id_vchr_nbr] = inserted.id_vchr_nbr
  ,[id_vchr_line] = inserted.id_vchr_line
  ,[id_vchr_line_dist] = inserted.id_vchr_line_dist
  ,[id_glac_exp] = inserted.id_glac_exp
  ,[text_ref_4] = inserted.text_ref_4
  ,[amt_exp_hcur] = inserted.amt_exp_hcur
  ,[amt_rem_hcur] = inserted.amt_rem_hcur
  ,[id_tax] = inserted.id_tax
  ,[code_type_chrg] = inserted.code_type_chrg
  ,[id_vchr_line_chrg] = inserted.id_vchr_line_chrg
  FROM inserted
      INNER JOIN (
         SELECT ORG, ID_VEND, ID_ORG_ORIG, ID_VCHR_NBR, ID_VCHR_LINE, ID_VCHR_LINE_DIST, CODE_TYPE_CHRG, ID_VCHR_LINE_CHRG
         FROM [dbo].[vwMP_VOD]
    ) AS V
      ON inserted.Org = V.Org
        AND inserted.ID_VEND = V.ID_VEND
        AND inserted.ID_ORG_ORIG = V.ID_ORG_ORIG
        AND inserted.ID_VCHR_NBR = V.ID_VCHR_NBR
        AND inserted.ID_VCHR_LINE = V.ID_VCHR_LINE
        AND inserted.ID_VCHR_LINE_DIST = V.ID_VCHR_LINE_DIST
        AND inserted.CODE_TYPE_CHRG = V.CODE_TYPE_CHRG
        AND inserted.ID_VCHR_LINE_CHRG = V.ID_VCHR_LINE_CHRG;

INSERT INTO [dbo].[vwMP_VOD]
SELECT inserted.*
FROM inserted
    LEFT JOIN (
         SELECT ORG, ID_VEND, ID_ORG_ORIG, ID_VCHR_NBR, ID_VCHR_LINE, ID_VCHR_LINE_DIST, CODE_TYPE_CHRG, ID_VCHR_LINE_CHRG
         FROM [dbo].[vwMP_VOD]
    ) AS V
      ON inserted.Org = V.Org
        AND inserted.ID_VEND = V.ID_VEND
        AND inserted.ID_ORG_ORIG = V.ID_ORG_ORIG
        AND inserted.ID_VCHR_NBR = V.ID_VCHR_NBR
        AND inserted.ID_VCHR_LINE = V.ID_VCHR_LINE
        AND inserted.ID_VCHR_LINE_DIST = V.ID_VCHR_LINE_DIST
        AND inserted.CODE_TYPE_CHRG = V.CODE_TYPE_CHRG
        AND inserted.ID_VCHR_LINE_CHRG = V.ID_VCHR_LINE_CHRG
WHERE V.Org IS NULL;

What I've done here is to take the your WHERE IN condition and replicate it with a join instead.

In the "update" case, you wanted all rows in the INSERTED table that are also in the [dbo].[vwMP_VOD] table.  Textbook inner join.  Joining on the key columns should be pretty quick.  If not, make sure your table has a PRIMARY KEY defined on these columns; if you have a surrogate key (ID column) instead, then you can still add a UNIQUE INDEX on the columns used to identify the rows in this upsert.

In the "insert" case, you want all rows in the INSERTED table that are not in [dbo].[vwMP_VOD].  To avoid concatenating all the key columns together, I simulated the effects of the EXCEPT operation by using a LEFT JOIN from INSERTED to [dbo].[vwMP_VOD], joining on all key columns, and adding a WHERE condition that checks that a non-nullable column from [dbo].[vwMP_VOD] is null.  That reliably indicates that there was no row present in the right-hand table ([dbo].[vwMP_VOD]).


Creating an array of objects for dictionaries I wrote a piece of code that creates an array of objects based on the array of dictionaries. The problem is that my solution uses multiple loops imperatively.

Let's consider the following case. There is a class called Drug:

class Drug {

var name:String
var dosage:[(dose:String,time:String)]

init(name:String){

    self.name = name
    self.dosage = []
    }
}

I obtain the data as an array of dictionaries:

var array = [["drugName":"Amotaks", "time":"17.00", "dose":"5"], ["drugName":"Amotaks", "time":"18.00", "dose":"5"], ["drugName":"Amotaks", "time":"18.00", "dose":"5"], ["drugName":"Amotaks", "time":"17.00", "dose":"5"], ["drugName":"Claritin", "time":"17.00", "dose":"5"], ["drugName":"Claritin", "time":"18.00", "dose":"5"]]

Now is the tricky part. What is the best way to turn this data into an array of Drug objects? I am not happy with my solution because it uses nested loops and additional helper arrays thus it's very inefficient.

The purpose is to create objects without repeating their names and provide each object with an dosage array which consist of (dose, time) tuples):

var DrugArray:[Drug] = []
    var drugs:[String] = []

    for i in array

        {
        if contains(drugs, i["drugName"]! as String){
            continue
            }
        else{

            drugs.append(i["drugName"]!)
            DrugArray.append(Drug(name: i["drugName"]!))

            }

        }

    for a in DrugArray{

        for i in array {
            if i["drugName"]! == a.name
            {
                a.dosage.append(dose: i["dose"]!, time: i["time"]!)

            }

        }}

I believe there is a better way to do that, probably in a few lines of code. I tried to figure out something with basic FP functions such as map or filter but I end up with nothing.

How can I perform the operation in an elegant way (probably using more advanced functional programming)?

var DrugArray:[Drug] = []
var drugs:[String] = []

for i in array {
    if contains(drugs, i["drugName"]! as String) {
        continue
    } else {
        drugs.append(i["drugName"]!)
        DrugArray.append(Drug(name: i["drugName"]!))
    }
}

for a in DrugArray {
    for i in array {
        if i["drugName"]! == a.name {
            a.dosage.append(dose: i["dose"]!, time: i["time"]!)
        }
    }
}

So, now that we've cleaned up the code and made it more readable, let's talk about some refactoring we can do.

The first thing I notice is that your nested loop at the bottom is entirely unnecessary.  We can do all of that logic in the top loop.

We're also doing a lot of forced unwrapped... which you really don't want to do.  All this is going to do is result in a crash in your app eventually.

We're also using single letter variable names which invokes no meaning to the reader.  These loops are short... for now.  If & when the contents of the loop grows (the top loop is already close to being at the point), you'll start to see the value in having meaningful variable names.  It helps you remember exactly what the variable represents.  Better variable names is just one part of making your code self-documenting.

And ultimately, as Mario Zahone's answer points out, what we clearly need is a dictionary rather than an array (and a separate array to keep track of what's already been tracked).

Finally, repeatedly appending to a collection almost always results in poor performance.  If there's not enough space in the collection's current memory location for one more item, the entire collection has to be copied into a new memory location, which is an expensive task.  In a loop with lots of iterations, this is likely to happen multiple times.  So, when we can guess at a good minimum size, we want to go ahead and make sure our collection is allocated in a large enough memory space.

So, taking all of these things into account, the final implementation of our code probably looks something like this:

var drugs = [String:Drug](minimumCapacity: array.count)

for drugDictionary in array {
    if let
        drugName = drugDictionary["drugName"],
        drugDose = drugDictionary["dose"],
        drugTime = drugDictionary["time"] {

        drugs[drugName] = drugs[drugName] ?? Drug(name: drugName)
        drugs[drugName]?.dosage.append(dose: drugDose, time: drugTime)
    }
}

If we really want an array, we can always fetch one from the dictionary:

let drugArray = drugs.values


Moving all .xlsx files from the last five days (but not today) My code works, but it looks very bad.

I need to move .xlsx files from the previous days from one folder to another, but I should'nt move the files with today's date, just the files from previous days.

In my current code, I am moving the files by specifying each one of them with a Minus X days, but I think the code is not in the best 'shape' nor the best performance/quality.

Any one can help me improve it?

import datetime,os, glob, shutil
from pathlib import Path

dir_carteiras = r"C:\Users\GuilhermeMachado\Documents\Carteiras"
test = os.listdir(dir_carteiras)

for item in test:
    if item.endswith(".jpg"):
        os.remove( os.path.join(dir_carteiras, item))

#get todays  date and a calculation for today -1 day
today=datetime.date.today()
minus_one_day=datetime.timedelta(days=-1)

#use the calculation to get previous 5 days strings
d_N1=today+minus_one_day
d_N1_ = d_N1.strftime('%Y.%m.%d')
d_N2=d_N1+minus_one_day
d_N3=d_N2+minus_one_day
d_N4=d_N3+minus_one_day
d_N5=d_N4+minus_one_day

#format dates for the files formats
Date_Carteira_N1 = d_N1.strftime('%d_%m_%Y')
Date_Carteira__0 = today.strftime('%d_%m_%Y')
Date_Carteira_N2 =d_N2.strftime('%d_%m_%Y')
Date_Carteira_N3 =d_N3.strftime('%d_%m_%Y')
Date_Carteira_N4 =d_N4.strftime('%d_%m_%Y')
Date_Carteira_N5 =d_N5.strftime('%d_%m_%Y')
Alocacao_0 = today.strftime('%Y%m%d')
Alocacao_N1 = d_N1.strftime('%Y%m%d')
Alocacao_N2 = d_N2.strftime('%Y%m%d')
Alocacao_N3 = d_N3.strftime('%Y%m%d')
Alocacao_N4 = d_N4.strftime('%Y%m%d')
Alocacao_N5 = d_N5.strftime('%Y%m%d')

#folder destination
NewFolderPath = dir_carteiras+"\Historico"
Path(NewFolderPath).mkdir(parents=True, exist_ok=True)

#Move the "Carteira" files from previous 5 days
Carteiras_move_N1 = glob.glob(r'C:\Users\GuilhermeMachado\Documents\Carteiras\*'+Date_Carteira_N1+'.xlsx', recursive=True)
for f in Carteiras_move_N1:
        shutil.move(f,NewFolderPath)
Carteiras_move_N2 = glob.glob(r'C:\Users\GuilhermeMachado\Documents\Carteiras\*'+Date_Carteira_N2+'.xlsx', recursive=True)
for f in Carteiras_move_N2:
        shutil.move(f,NewFolderPath)
Carteiras_move_N3 = glob.glob(r'C:\Users\GuilhermeMachado\Documents\Carteiras\*'+Date_Carteira_N3+'.xlsx', recursive=True)
for f in Carteiras_move_N3:
        shutil.move(f,NewFolderPath)
Carteiras_move_N4 = glob.glob(r'C:\Users\GuilhermeMachado\Documents\Carteiras\*'+Date_Carteira_N4+'.xlsx', recursive=True)
for f in Carteiras_move_N4:
        shutil.move(f,NewFolderPath)
Carteiras_move_N5 = glob.glob(r'C:\Users\GuilhermeMachado\Documents\Carteiras\*'+Date_Carteira_N5+'.xlsx', recursive=True)
for f in Carteiras_move_N5:
        shutil.move(f,NewFolderPath)

#Move the "Alocacao" files from the previous 5 days
Alocacao_move_N1 = glob.glob(r'C:\Users\GuilhermeMachado\Documents\Carteiras\*'+Alocacao_N1+'.xlsx', recursive=True)
for f in Alocacao_move_N1:
        shutil.move(f,NewFolderPath)
Alocacao_move_N2 = glob.glob(r'C:\Users\GuilhermeMachado\Documents\Carteiras\*'+Alocacao_N2+'.xlsx', recursive=True)
for f in Alocacao_move_N2:
        shutil.move(f,NewFolderPath)
Alocacao_move_N3 = glob.glob(r'C:\Users\GuilhermeMachado\Documents\Carteiras\*'+Alocacao_N3+'.xlsx', recursive=True)
for f in Alocacao_move_N3:
        shutil.move(f,NewFolderPath)
Alocacao_move_N4 = glob.glob(r'C:\Users\GuilhermeMachado\Documents\Carteiras\*'+Alocacao_N4+'.xlsx', recursive=True)
for f in Alocacao_move_N4:
        shutil.move(f,NewFolderPath)
Alocacao_move_N5 = glob.glob(r'C:\Users\GuilhermeMachado\Documents\Carteiras\*'+Alocacao_N5+'.xlsx', recursive=True)
for f in Alocacao_move_N5:
        shutil.move(f,NewFolderPath)

First, if you are using Python 3, you can use pathlib to make your life regarding paths a lot easier (especially if you want your scripts to work on both Windows and Linux). If you are not using Python 3 by now, well, you should.

While your approach works (and could be made a bit shorter maybe using a complex RegEx), you might want to look into different methods of doing this. Instead of manually using the format of your file names, you can use the time the file was last modified (or accessed) and just filter on that:

from datetime import datetime, timedelta
from pathlib import Path
import shutil
from typing import Iterable

def old_files(dir: Path, pattern: str, start: int = -5, end: int = -1) -> Iterable[Path]:
    """List all files in `dir` matching `pattern` modified between
    `start` and `end` (both inclusive) days from today.
    """
    today = datetime.today()
    start = (today.replace(hour=0, minute=0, second=0, microsecond=0)
             + timedelta(days=start)).timestamp()
    end = (today.replace(hour=23, minute=59, second=59, microsecond=999999)
           + timedelta(days=end)).timestamp()
    return (file for file in dir.glob(pattern)
            if file.is_file() and start <= file.stat().st_mtime <= end)

Which you can use like this:

if __name__ == "__main__":
    dir = Path(r"C:\Users\GuilhermeMachado\Documents\Carteiras")
    new_dir = dir / "Historico/"
    new_dir.mkdir(exist_ok=True)
    for file in old_files(dir, "*.xlsx", start=-5, end=-1)
        shutil.move(str(file), new_dir)

Note that I used the fact that when you divide a path by a string (`dir / "Historico") it just joins the path (with the correct path separator depending on your OS).

I also added a if __name__ == "__main__": guard to allow importing from this script without the code running, made the number of days to select parameters of a function to which I also added typing hints and a docstring and followed Python's official style-guide, PEP8, by having spaces around the = for assignments.

Of course this assumes that you have no other Excel spreadsheets modified during the last five days in that folder. In that case you might be able to adapt the pattern to only match the files you want, or you will have to use a different method for filtering the files. But that would just mean modifying the function, not your whole script. This way the changes are localized, which makes it easier to track.


Detecting the total amount of RAM installed on a hardware from a Linux system using a POSIX shell script Today, I gave myself a small task of detecting the total amount of RAM installed on a hardware from a Linux system using a POSIX shell script. I ended up using dmidecode utility and a rather complex looking code for such a simple task.

Though I found countless simple pieces of code on various places, they are all doing not just that exactly.

I don't want free RAM or total available RAM to the kernel.

I want the pure number of MiBs, which are installed in the hardware (memory sticks).

Reasons are multiple, to name one, for example, there could be less available RAM to the system due to various things, like integrated graphics shared memory, etc.

Performance is not an issue. I know it is slow. I just want to know if there is a way to perhaps simplify the code.

#!/bin/sh

is_number()
{
    [ "${1}" -eq "${1}" ] 2> /dev/null
}

ram_entries=$(sudo dmidecode --type 17 | grep Size: | awk '{ print $2 }')
ram_entries_count=$(( $(printf '%s\n' "${ram_entries}" | wc -l) - 1 ))

i=0
ram_entry=x
total_ram_size_mb=0

while [ "${i}" -le "${ram_entries_count}" ]
do
    is_number "${ram_entry}" && total_ram_size_mb=$(( total_ram_size_mb + ram_entry ))
    i=$(( i + 1 ))
    ram_entry=$(echo "${ram_entries}" | sed --posix --quiet "${i}{p;q}")
done

if [ "${total_ram_size_mb}" -ge 1024 ]
then
    echo "Total RAM installed in Gigabytes =" $(( total_ram_size_mb / 1024 ))
else
    echo "Total RAM installed in Megabytes = ${total_ram_size_mb}"
fi

Using sudo can be problematic within scripts.  It's really designed to be used interactively, and may request a password (which won't work very well if its standard input isn't a terminal).  The alternative is to require the script to be run as a suitably privileged user, and I can see that you want to minimise the amount of privileged code.

I couldn't initially find an unprivileged way to do what dmidecode - it inspects /sys/firmware/dmi/tables/DMI which is unreadable by ordinary users and I don't see the same information anywhere else in sysfs.  However, lshw can produce a total RAM summary without privilege (but can't see the individual DIMMs).  After a bit of tracing, it seems that it reads from /sys/devices/system/memory - multiply the contents of block_size_bytes (in hexadecimal) by the total of */online to get the total RAM size in bytes:

printf '%c' /sys/devices/system/memory/memory*/online | wc -c |
    dc -e '16i' -f /sys/devices/system/memory/block_size_bytes -e 'Ai ? *p'

If coreutils is available (almost all Linux systems, I think; certainly as likely as sed or awk and much more so than dmidecode), then we can use the numfmt utility to convert that number to a friendly human form:

numfmt --to=iec-i --format='%.1f%f' --suffix B

(see the man page to understand the options, and adjust to taste).

Full working alternative

#!/bin/sh

printf 'Total RAM installed = '
printf '%c' /sys/devices/system/memory/memory*/online | wc -c |
    dc -e '?16i' -f /sys/devices/system/memory/block_size_bytes -e '*p' |
    numfmt --to=iec-i --suffix B

As a bonus, this is much faster than using dmidecode (which seems to gather all its information even when printing only a small subset).

Other points:

I recommend setting -e and -u in all shell scripts (both are specified in POSIX):

set -eu

Don't pipe through grep and awk - since the latter does regexp matching, just add a pattern to the awk program:

dmidecode --type 17 | awk '/^[[:space:]]*Size: [0-9]/ { print $2 }'

Including a digit in the pattern eliminates the empty slots neatly at that stage, so obviating is_number.
Counting using sprintf|wc is a little safer if you use %c or %.1s conversion; that uses just the first letter, so you can count using wc -c and avoid double-counting strings with embedded newlines (for example, see my worked code).  You won't get the problem lines here, but it's a technique worth knowing.
Instead of iterating through the indices of $ram_entries and repeatedly invoking sed to extract individual lines, it's better to check that it's safe to expand without quotes (we know this to be true, since we've ensured it contains only decimal numbers), and then simply for ram_entry in $ram_entries.

Even better, instead of capturing and assigning to a variable, we could pipe into a while read loop to process each line.  Be careful to make the loop be in the main shell, not a sub-shell, though, or make it communicate the result out.


Clean up Rails Routes Is there any way I can DRY up this Rails 4 routes.rb code?

  match '/oauth/apps/' => 'oauth/applications#index', :as => :oauth_applications, :via => :get
  match '/oauth/apps/new/' => 'oauth/applications#new', :as => :new_oauth_application, :via => :get
  match '/oauth/apps/new/' => 'oauth/applications#create', :via => :post
  match '/oauth/apps/:id/' => 'oauth/applications#show', :as => :oauth_application, :via => :get
  match '/oauth/apps/:id/edit/' => 'oauth/applications#edit', :as => :edit_oauth_application, :via => :get
  match '/oauth/apps/:id/edit/' => 'oauth/applications#update', :via => :post
  match '/oauth/apps/:id/destroy/' => 'oauth/applications#destroy', :as => :destroy_oauth_application, :via => :delete

Yes, use namespace and resources:

namespace :oauth do
  resources :apps, controller: "applications", as: :applications
end

This is not 100% identical (the names of some of your routes have changed) but it is the way you should be building your routes, and you should change the rest of your app to reflect the change.

The primary difference is your names for the create and destroy routes have gone away; these routes shouldn't have names anyways. When you want to create/destroy a model, you should be using oath_applications_path, with  method: :post or method: :delete, not oath_apps_new_path/destroy_oauth_application_path.

My way also uses the correct verb (PUT) for updates.

Your rake routes:

       oauth_applications GET    /oauth/apps(.:format)             oauth/applications#index
    new_oauth_application GET    /oauth/apps/new(.:format)         oauth/applications#new
           oauth_apps_new POST   /oauth/apps/new(.:format)         oauth/applications#create
        oauth_application GET    /oauth/apps/:id(.:format)         oauth/applications#show
   edit_oauth_application GET    /oauth/apps/:id/edit(.:format)    oauth/applications#edit
                          POST   /oauth/apps/:id/edit(.:format)    oauth/applications#update
destroy_oauth_application DELETE /oauth/apps/:id/destroy(.:format) oauth/applications#destroy

My rake routes (reordered to match yours):

    oauth_applications GET    /oauth/apps(.:format)          oauth/applications#index
 new_oauth_application GET    /oauth/apps/new(.:format)      oauth/applications#new
                       POST   /oauth/apps(.:format)          oauth/applications#create
     oauth_application GET    /oauth/apps/:id(.:format)      oauth/applications#show
edit_oauth_application GET    /oauth/apps/:id/edit(.:format) oauth/applications#edit
                       PUT    /oauth/apps/:id(.:format)      oauth/applications#update
                       DELETE /oauth/apps/:id(.:format)      oauth/applications#destroy


Multiple regex matches to correct OCR output Using OCR, my program reads the current selection for each of these categories: format, invert color? and storage. The OCR result will always have inconsistent error here and there, which is why I need a Regex match and return the correct wording for each selection:

//format
private Regex bmp8 = new Regex(".bmp.8....");
private Regex bmp24 = new Regex(".bmp.24....");
private Regex png24 = new Regex(".png.24....");
private Regex raw = new Regex("raw");
private Regex ts = new Regex("....setup");
private Regex csv = new Regex(".csvd...");

//invert color?
private Regex yes = new Regex("y.s");
private Regex no = new Regex("n.");

//storage
private Regex intrnl = new Regex(".nte...");
private Regex erase = new Regex("e.ase");
private Regex transfer = new Regex("t.ansf.");
private Regex usbdev = new Regex("usbdev.");

private string CheckStorage(string input)
{
    if (intrnl.IsMatch(input))
    {
        return "Internal";
    }

    else if (erase.IsMatch(input))
    {
        return "Erase";
    }

    else if (transfer.IsMatch(input))
    {
        return "Transfer";
    }

    else
    {
        return "USB Device";
    }
}

private string CheckInvertColor(string input)
{
    if (yes.IsMatch(input))
    {
        return "Yes";
    }

    else
    {
        return "No";
    }
}

private string CheckFormat(string input)
{
    if (bmp8.IsMatch(input))
    {
        return "bmp(8-bit)";
    }

    else if (bmp24.IsMatch(input))
    {
        return "bmp(24-bit)";
    }

    else if (png24.IsMatch(input))
    {
        return "png(24-bit)";
    }

    else if (raw.IsMatch(input))
    {
        return "Raw";
    }

    else if (ts.IsMatch(input))
    {
        return "Trace & Setup";
    }

    else
    {
        return "csv Data";
    }
}

I am sure there is a better way to tidy up this code, perhaps using List<>?

I see something that bugs me in your code, that being default return values and some unused regex expressions declared, for example in the following function:

private string CheckInvertColor(string input)
{
    if (yes.IsMatch(input))
    {
        return "Yes";
    }

    else
    {
        return "No";
    }
}

Why do you only check if the yes regex is a match, what if the input is different from the one that matches the no answer? What do you expect the code to do in this case? You return a "No" in any case that the yes regex does not match, then why have the no regex declared at all?

This also applies to the "USB device" and "csv data" of their respecting functions, you are not actually checking those values yet have them declared still.

I stress this because in my opinion, you should prevent unexpected inputs from returning a value for which you are expecting a specific input for. Instead the code should throw an exception or a value which then you can manage elsewhere to notify that an unexpected input has been fed to the function.

In regards to tiding up your code, there are some options you can use but the one that suits your needs is the following one:

Dictionaris

Although you may already know, a dictionary is a class that returns a value if the key supplied exists in said dictionary, with it your regex declarations would look like in the following:

private Dictionary<Regex, string> formatDictionary = new Dictionary<Regex, string>()
{
    {new Regex(".bmp.8...."),"bmp(8-bit)"},
    {new Regex(".bmp.24...."),"bmp(24-bit)"},
    {new Regex(".png.24...."),"png(24-bit)"},
    {new Regex("raw"),"Raw"},
    {new Regex("....setup"),"Trace & setup"},
    {new Regex(".csvd...."),"csv data"},
}

private Dictionary<Regex, string> inversionDictionary = new Dictionary<Regex, string>()
{
    {new Regex("y.s"),"Yes"},
    {new Regex("n."),"No"}
}

private Dictionary<Regex, string> storageDictionary = new Dictionary<Regex, string>()
{
    {new Regex(".nte..."),"Internal"},
    {new Regex("e.ase"),"Erase"},
    {new Regex("t.ansf."),"Transfer"},
    {new Regex("usbdev."),"USB Device"}
}

This way, with a LINQ query, you can check the validity of the input:

if (!storageDictionary.Any(x => x.Key.IsMatch(input)))
    throw new Exception("the input is not valid");

This way you can ensure that the input matches at least one of the Regex you defined, now you can recover the corresponding value:

return storageDictionary[storageDictionary.Where(x => x.Key.IsMatch(input)).First().Key];

Your functions would end up looking like this:

private string CheckInvertColor(string input)
{
    if (!storageDictionary.Any(x => x.Key.IsMatch(input)))
        throw new Exception("the input is not valid");

    return storageDictionary[storageDictionary.Where(x => x.Key.IsMatch(input)).First().Key];
}


pythonic way to split bytes I have incoming data from an absolute rotary encoder. It is 24 bits wide. 12 bits are the number of turns and 12 bits are the angle.

I'm reading the data with an Arduino and sending it to the computer over a serial port. The most natural thing to do would be to combine all of the bits and send it as 3 bytes.

Splitting up the 3 bytes into 12 bits/12 bits at the computer in Python 3 feels unpythonic to me. Here is what I have so far:

import struct

# fake message, H is header last 3 bytes are data
msg = b'H\x90\xfc\xf9'
header, bytes3 = struct.unpack('>c3s', msg)
print('bytes3 ', bytes3)
val = int.from_bytes(bytes3, 'big')
print('val ', val)
print('bin(val) ', bin(val))

upper12 = val >> 12
print('bin(upper12)', bin(upper12))
lower12 = val & 0b000000000000111111111111
print('bin(lower12)', bin(lower12))

which gives console output of

bytes3  b'\x90\xfc\xf9'
val  9501945
bin(val)  0b100100001111110011111001
bin(upper12) 0b100100001111
bin(lower12) 0b110011111001

This code seems to work fine, but bitshifting (val >> 12) and bitwise anding (val & 0b000...) are a little wonky. Also it feels funny to specify Big Endian twice: first in the struct.unpack format string and second in int.from_bytes.

Is there a more elegant way to achieve this?

Update: timing

I'm still on the fence on which technique is most Pythonic. But I did time 3 techniques:

original technique above
technique by AlexV with overlapping unsigned 16 bit integers
change comms protocol to send a padded 4 byte unsigned integer that struct can convert directly to integer

import  struct
import time
import random
import timeit

def gen_random_msg(nbyte = 3):
    # make a n byte message with header 'H' prepended
    data = random.randint(0, 2**12-1)
    data = data.to_bytes(nbyte, 'big')
    msg = b'H' + data
    return msg

def original_recipe():
    msg = gen_random_msg(3)
    header, bytes3 = struct.unpack('>c3s', msg)
    val = int.from_bytes(bytes3, 'big')

    upper12 = val >> 12
    lower12 = val & 4095 # 4095 = 2**12-1

def overlap16bits():
    msg = gen_random_msg(3)
    header, val = struct.unpack('>cH', msg[0:-1])
    upper12 = val >> 4
    lower12 = struct.unpack('>H', msg[2:])[0] & 0xfff

def fourbyte():
    msg = gen_random_msg(4)
    header, val = struct.unpack('>cI', msg)
    upper12 = val >> 12
    lower12 = val & 4095 # 4095 = 2**12-1

niter = int(1e6)
t0 = time.time()
for i in range(niter): gen_random_msg(3)
t1 = time.time()
for i in range(niter): gen_random_msg(4)
t2 = time.time()
for i in range(niter): original_recipe()
t3 = time.time()
for i in range(niter): overlap16bits()
t4 = time.time()
for i in range(niter): fourbyte()
t5 = time.time()

gentime3 = t1-t0
gentime4 = t2-t1
original_time = t3-t2
overlap_time = t4-t3
fourbyte_time = t5-t4

print('gentime3: ', gentime3, '. gentime4: ', gentime4)
print ('original recipe: ', original_time - gentime3)
print ('overlap 16 bits: ', overlap_time - gentime3)
print ('four bytes: ', fourbyte_time - gentime4)

this has console output:

gentime3:  3.478888988494873 . gentime4:  3.4476888179779053
original recipe:  1.3416340351104736
overlap 16 bits:  1.435237169265747
four bytes:  0.7956202030181885

It takes more time to generate 1M dummy msg than it does to process the bytes. The fastest technique was to change the comms spec and pad the 3 bytes to make a 4 byte unsigned integer. The speed up was good (approx 2x) for "fourbytes", but requires changing the communications specification. At this point I think it comes down to personal preference as to which algorithm is the best.

You could unpack them twice into (overlapping) unsigned shorts of 16bit and shift/mask them accordingly.

upper12 = struct.unpack(">H", msg[1:-1])[0] >> 4
lower12 = struct.unpack(">H", msg[2:])[0] & 0xFFF

Telling Python to interpret them as short integers helps you to get rid of int.from_bytes.


How to put a lot of information in the class? I have such a problem:

I'm parsing a lot of HTML files with Simple HTML DOM Parser, that's why I need to use three foreachs to parse the necessary information from it -> I am getting a lot of information and now I have three arrays which keep it. I think that it's dumb to keep the code like that and I want to keep the information in the class.

How can I do it in a right way? I really need your advice to have a good code in the future

Here is the code with three arrays:

if(count($infos->find('.products-list-item')))
{
    foreach ($infos->find('.products-list-item product-label, .products-list-item .products-list-item__brand, .products-list-item .products-list-item__type,.products-list-item .price') as $item_info)
    {
        $item_info_arr[] = $item_info->plaintext;
        $i=0;
    }

    foreach ($infos->find('.products-list-item__img') as $img)
    {
        $src_arr[] = $img->src;
    }

    foreach ($infos->find('.products-list-item .products-list-item__sizes') as $sizes_info)
    {
        while($size = $sizes_info->children($k++))
        {
            $sizes_arr[] = $size->plaintext;
        }

        $k=0;
    }

} else {echo "not found";}

But what can I do, then?

Please provide some more information, like the example contents of $infos. It is hard to find any other solution if we are not able to understand your current solution in the first place.

However, there already are some issues with your code:

avoid using variable names like $i and $k. They should be named according to their meaning (see example for $k)
$i seems not to be read in your code
$k and all arrays seem to be used uninitialized

Your while loop seems a bit too clever to me. All you want to do is iterate $k so you should instead use a for loop:

for($childIndex = 0; $size = $sizes_info->children($childIndex); ++$childIndex)
    $sizes_arr[] = $size->plaintext;

The foreach loops look as if they could be better implemented by array_map:

$item_infos = $infos->find('.products-list-item product-label, .products-list-item .products-list-item__brand, .products-list-item .products-list-item__type,.products-list-item .price');
$get_plaintext = function($item_info) { return $item_info->plaintext; };
$item_info_arr = array_map($get_plaintext, $item_infos);

A foreach loop could be as readable however then you should at least use named variables (like $item_infos but with better names (I have no idea what this find returns)) to keep the head of the loop short and readable.


A C++17 `std::allocator` implementation This is an implementation of C++17 std::allocator under the name my_std::allocator, with deprecated stuff omitted for simplicity.

// A implementation of C++17 std::allocator
// with deprecated members omitted

#include <cstddef>
#include <new>
#include <type_traits>

namespace my_std {

  template <typename T>
  class allocator {
  public:
    using value_type = T;
    using propagate_on_container_move_assignment = std::true_type;
    using is_always_equal = std::true_type;

    allocator() noexcept = default;
    allocator(const allocator&) noexcept = default;
    ~allocator() = default;

    template <class U>
    allocator(const allocator<U>&) noexcept
    {
    }

    T* allocate(std::size_t n)
    {
      if constexpr (alignof(T) > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
        return static_cast<T*>(
          ::operator new(n * sizeof(T), static_cast<std::align_val_t>(alignof(T)))
        );
      else
        return static_cast<T*>(
          ::operator new(n * sizeof(T))
        );
    }

    void deallocate(T* p, std::size_t n)
    {
      if constexpr (alignof(T) > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
        return ::operator delete(p, n * sizeof(T),
                                 static_cast<std::align_val_t>(alignof(T)));
      else
        return ::operator delete(p, n * sizeof(T));
    }
  };

  template <class T, class U>
  bool operator==(const allocator<T>&, const allocator<U>&) noexcept
  {
    return true;
  }

  template <class T, class U>
  bool operator!=(const allocator<T>&, const allocator<U>&) noexcept
  {
    return false;
  }

}

I have tested it like this:

int main()
{
  std::set<std::string, std::less<std::string>,
       my_std::allocator<std::string>> words;

  for (std::string w; std::cin >> w;)
    words.insert(std::move(w));

  for (const auto& w : words)
    std::cout << w << "\n";
}

And currently no bug is discovered.

noexcept on the explicitly defaulted default-ctor and copy-ctor is already implied.
Copy-ctor and dtor would be implicitly declared if left out.
.allocate() (and also .deallocate(), but that doesn't matter) fails to check n * sizeof(T) for wrap-around.
Possible Solutions:

Start with a check. This has the advantage of simplicity and broadest conformity.

if (n > std::size_t(-1) / sizeof(T))
    throw std::bad_array_new_length();

Use saturating math, and rely on ::operator new being unable to allocate std::size_t(-1) bytes on (nearly?) all implementations. This is generally more efficient for the common case.

auto bytes = n > std::size_t(-1) / sizeof(T) ? std::size_t(-1) : n * sizeof(T);

Because the casts are so simple and obviously right, I would eschew the verbosity of static_cast. Guidelines may prohibit it though...
I suggest importing the following additional refinements from C++20:

Mark the templated ctor constexpr.
Mark .allocate()'s return-value [[nodiscard]].
Mark both operator== and operator!= constexpr.


Codeforces problem python code optimization I have solved one problem on Codeforces and I guess the output is quite correct. However, it is slow for input in range of thousands. Since, I am learning Python now, it would be great help if one could help me optimise this code. Any notes, tips or suggestions on the sidelines are always welcome.

The question is a practice question and hence does not violate any contest rules.

import sys
import time

_no_of_ants, _no_of_queries = (input().split())
no_of_ants = int(_no_of_ants)
no_of_queries = int(_no_of_queries)

class ants_cord():
    def __init__(self,point,no_of_ants):
        self.co_ant = {point:no_of_ants}

    def ants_at_point(self,point):
        return self.co_ant[point]

    def move_ants(self,point):
        x1,y1 = point
        if (x1+1,y1) in self.co_ant.keys():
            self.co_ant[((x1+1),y1)] += 1
        else:
            self.co_ant[((x1+1),y1)] = 1
        if (x1,y1+1) in self.co_ant.keys():
            self.co_ant[((x1),y1+1)] += 1
        else:
            self.co_ant[((x1),y1+1)] = 1
        if (x1,y1-1) in self.co_ant.keys():
            self.co_ant[((x1),y1-1)] += 1
        else:
            self.co_ant[((x1),y1-1)] = 1
        if (x1-1,y1) in self.co_ant.keys():
            self.co_ant[((x1-1),y1)] += 1
        else:
            self.co_ant[((x1-1),y1)] = 1
        self.co_ant[point] -= 4

    def check_status(self):
        for key in self.co_ant.keys():
            if k.co_ant[key] >= 4:
                return key
        else: return False
start = time.time()
k = ants_cord((0,0),no_of_ants)
point = k.check_status()
while point:
    k.move_ants(point)
    point = k.check_status()
end = time.time()
print("Sec:{}".format(end-start))
while no_of_queries:
    _x,_y = input().split()
    x = int(_x)
    y = int(_y)
    if (x,y) in k.co_ant:
        print(k.co_ant[(x,y)])
    else:
        print('0')
    no_of_queries -= 1

EDIT: I updated the code according to suggestion. But I still get a TLE error in test case 4. Should I employ other algorithm then?

A few things:

You should probably put the start = time.time() as early as possible if you want it to make sense.
As your class consists on a simple dict, there is no or little value in defining it. (In a "real life" scenario, it would be considered as a good practice).

Things got 25% quicker on average starting with 1000 ants.

You could make your code more concise by using structure to isolate the parts that change.

move_ants() would look like this :

def move_ants(g,point):
    x,y = point
    for a,b in [(x+1,y),(x,y+1),(x,y-1),(x-1,y)]: # or you could define x generxtor
        if (a,b) in g.keys():
            g[a,b] += 1
        else:
            g[a,b] = 1
    g[point] -= 4

(Not performance impact)

In check_status(), instead of iterating on keys and then retrieve the corresponding value, you could use iteritems() :

check_status() would be like this :

def check_status(g):
    for k,v in g.iteritems():
        if v >= 4:
            return k
    else: return False

(This does improve performances)

To get the values from your dict with default values, you could use get(). You'd have something like print(g.get((x,y),0)) and g[a,b] = 1 + g.get((a,b),0). (I really didn't expect so but this made the script more than twice faster).

At the end, my script is like this (I've changed other details but it's mostly personal preferences) :

#!/usr/bin/python

import sys
import time

start = time.time()

##_no_of_ants, _no_of_queries = (input().split())
##no_of_ants = int(_no_of_ants)
##no_of_queries = int(_no_of_queries)

no_of_ants = 1000
no_of_queries = 1

def move_ants(g,point):
    x,y = point
    for a,b in [(x+1,y),(x,y+1),(x,y-1),(x-1,y)]: # or you could define x generxtor
        g[a,b] = 1 + g.get((a,b),0)
    g[point] -= 4

def check_status(g):
    for k,v in g.iteritems():
        if v >= 4:
            return k
    else: return False

g = {(0,0) : no_of_ants} # or you could use default dict
point = check_status(g)
while point:
    move_ants(g,point)
    point = check_status(g)
print("Sec:{}".format(time.time()-start))
for q in xrange(no_of_queries):
    ###_x,_y = input().split()
    ###x = int(_x)
    ###y = int(_y)
    x,y=1,1
    print(g.get((x,y),0))


Method for String Building in java I have written a custom method to use StringBuilder in java. My Requirement is to create a method which accepts any number of arguments of any type and return String by concatenating the arguments.

eg. msgBuilder("Hello ",0," how"," are"," you ",2.5) return "Hello 0 how are you 2.5"

Here is my java code. Please Someone review my code and suggest If I did anything is wrong or Can I use this further in my projects.

        public class Test {
            public static void main(String[] args ){
                String msg = msgBuilder("Hello ",5," How ","are ","you");
                System.out.println(msg);
            }

            private static String msgBuilder(Object... params){
                StringBuilder sb = new StringBuilder();
                for(Object obj:params){
                    sb.append(obj);
                }
                return sb.toString();
            }
        }

Your approach

As pointed out by the other answers, you are practically hard-coding whitespaces in your arguments, which can get non-standard easily, even in your example. Otherwise, the use of StringBuilder is sound, although it wouldn't deal nicely with a single null value - the for-each loop will fail as it implicitly tries to get an Iterator for it.

Java 8

An alternative is to rely on the stream-based processing that Java 8 offers:

private static String format(Object... values) {
    if (values == null) {
        return "";
    }
    return Arrays.stream(values)
                    .map(Objects::toString)
                    .collect(Collectors.joining(" "));
}

Construct a Stream<Object> from values using Arrays.stream(T...).
map() each value using Objects.toString(Object) as a method reference.
collect() the stream into a String by joining(" ") them together.


Manipulating model elements in angular All,
I barely have a week of angular behind me and I am still struggling with some of the concepts. Below is a piece of code I came up with. The idea is to let the user add/delete entries in a model (in practice, the model is read from a json and is more complex than what is presented). While this code seems to work, I'm wondering whether it's "proper" angular:

Is it OK to leave the add_item function in the controller ? I understand that any DOM manipulation should be put in a directive, but it's only manipulating the model, right?
Shouldn't the del_item be in the controller too, then?
Using the index to control which item to delete look a bit dangerous, what would be a better alternative?

Thanks for any input.

   {{items}}

     <button type="button" ng-click="add_item()">
       New item
     </button>

       <display-item idx={{$index}}></display-item>

Right, you're only manipulating the model, updating the model about a change (thus) updating it in the controller is perfectly fine, and it's what the controller does.
del_item should be in the controller too, you are correct. That's where it belongs.
Using the index to deleting the item is dangerous, what I'd do is pass this as a function and then use an indexOf to locate the element you're deleting and delete that.

If I may ask, why are your display items in a directive in the first place and not in the DOM? You could use an ng-repeat in the DOM to repeat the items instead and drop the directive altogether.

Directives are useful for code reuse, are you reusing this spefici component in a lot of other places through out your code? Behaviors affecting your models and not just your presentation logic should probably not be in the directive to begin with (like del_item in your example), a directive should encompass presentational behavior and not business logic.


How to optimize C# console application i'm wondering is there any way to optimize this code?
It must be able to work with big files, for example, formatting raw 140kb .txt file (12.5k words) takes 2 seconds (measured with Stopwatch class).
Text example http://pastebin.com/2p88v8EN
Maybe i used here some bad techniques or there is some part to simplify? Maybe multithreading? I'm not familiar with this yet.
Would be grateful for help!

Code below:

class TextManipulations
{
    public string[] wordsDist;                                                  // main array, contains words in alphabetic order and output lines

    public void TextFormat(string sourcePath)                                   // creating method that will format our source text according to task
    {
        string textInput = System.IO.File.ReadAllText(sourcePath).ToLower();    // reading text from file, lowercased at start for precise search
        MatchCollection m = Regex.Matches(textInput, @"\b[\w']+\b");            // exact search of all alphanumeric "words" including words with apostrophe
        List<string> words = new List<string>();                                // creating List<T> for containing unknown amount of words

        foreach (Match match in m)                                              // assigning all matches to List<string>
        {
            words.Add(match.ToString());
        }
        words.Sort();                                                           // sorting words in alphabetic order
        wordsDist = words.Distinct().ToArray();                                 // assigning words to main array without duplicates
        System.IO.File.WriteAllLines(@"D:\output.txt", wordsDist);      // writing words into txt file to edit in setLineNumbers method
    }

    public void setLineNumbers(string sourcePath)                               // creating method for adding line numbers
    {
        string[] linesOutput = new string[wordsDist.Count()];                   // creating array that will contain line numbers
        string[] lines = System.IO.File.ReadAllLines(sourcePath);               // assigning source text by lines

        for (int j = 0; j < wordsDist.Count(); j++)                             // main cycle checking each word for presence in each line
        {
            for (int i = 0; i < lines.Count(); i++)
            {
                if (Regex.IsMatch(lines[i].ToLower(), "\\b" + wordsDist[j] + "\\b"))   // using ToLower() here, because we can't use it in line 33
                {
                    linesOutput[j] += (i + 1).ToString() + ", ";                // adding line numbers according to word
                }

            }
        }

        for (int i = 0; i < wordsDist.Count(); i++)                             // connection of two relative arrays
        {
            wordsDist[i] += "_______________________________" + linesOutput[i];
            wordsDist[i] = wordsDist[i].Remove(wordsDist[i].Length - 2);        // removing last ',' char
        }

        System.IO.File.WriteAllLines(@"D:\output.txt", wordsDist);      // writing final output result into txt file

    }

}

Rewritten code:

class TextManipulations
{
private const string AlphanumericWords = @"\b[\w']+\b";
public string[] WordsDist;

public void TextFormat(string sourcePath, string output)
{
    var textInput = File.ReadAllText(sourcePath).ToLower();
    var m = Regex.Matches(textInput, AlphanumericWords);
    var words = new SortedSet<string>();

    foreach (Match match in m)
    {
        words.Add(match.ToString());
    }

    WordsDist = words.ToArray();

    File.WriteAllLines(output, WordsDist);
}

public void SetLineNumbers(string sourcePath, string output)
{
    var lines = File.ReadAllLines(sourcePath).AsParallel().Select(x => x.ToLowerInvariant()).ToArray();

    var regExs = WordsDist.Select(word => new Regex("\\b" + word + "\\b")).ToArray();

    var wordsOut = new string[regExs.Length];

    Parallel.For(0, regExs.Length, j =>
        {
            var sb = new List<int>(15);
            var regEx = regExs[j];
            for (var i = 0; i < lines.Length; i++)
            {
                if (regEx.IsMatch(lines[i]))
                {
                    sb.Add(i + 1);
                }
            }

            wordsOut[j] = WordsDist[j] + "_______________________________" + string.Join(", ", sb);
        });

    File.WriteAllLines(output, wordsOut);
}

}

TextFormat

Using a SortedSet is better becouse we don't need additional Sort() or Distinct() calls becouse it will handle all these stuff by it's self.

SetLineNumbers

The main problem was is that the testing Regex was always created inside the second for loop instead of creating once and then using it in all the iterations. The parallel stuff isn't necessary but we can speed up the code a little bit with that.

Results

Your code was executed on my machine a little bit above 2 secs and my code is finishing around 0.25 secs.

Other things

Do not ever write comments at the line endings becouse they are really useless and  annoying.
I've added an additional parameter to each method which can be used to specify the output file. (But the code still isn't really reuseable or testable.)


Did not pass the interview about an RPN Calculator I recently received a simple interview assignment as to implement an RPN calculator in Ruby. The program takes user input and quits when the user presses 'q'.

I did it in under 30 minutes and was quite confident about my solution, but the interviewers did not like it. I would love to know the flaws it has so I can avoid being overconfident next time and learn better coding practices!

#!usr/bin/env ruby

# A Ruby RPN Calculator
#Note: decimal numbers are rounded to nearest tenth
=begin
example input/ouput
> 2
2
> 9
9
>3
3
> +
12
> * 24
=end

class RPNCalculator

  attr_reader :operators
  def initialize(operators)
    @operators=operators
  end

  def start
    print '> '
    n = gets.strip
    stack = []

    while n!= 'q'
      if n =~ /\d/
        stack.push(n.to_f)
        puts n
      elsif self.operators.include? n
        operands = stack.pop(2)
        res = operands.reduce(n.to_sym)
        (res.to_i == res) ? (puts res.to_i) : (puts "%0.1f" % res)
        stack.push(res)
      else
        puts "Invalid input"
      end
      print '> '
      n = gets.strip
    end
  end
end

rpn = RPNCalculator.new(["+","-","*","/"])
rpn.start

I don't know how your assignment looked exactly, so some of the below may be misguided criticisms. I've just tried to poke as many holes in the code as possible; fair or otherwise. Something will probably overlap with your interviewer's thoughts. But of course, I don't know what that might be, so there's no real order to these points - I just wrote them as I thought of them.

Separate the logic from the REPL user interface. It'd be nicer if you could push input to the calculator from anywhere. And it'd be much easier to test the code.
Speaking of: Tests would be nice to see. If the place you interviewed is really big on TDD/BDD, maybe that's what they wanted to see more than anything. Interview assignments (when done well) are as much about the candidate's process as it is the result.
#start is a rather long method and it pretty much does, well, everything. If you call it, it starts a loop you won't be able to exit from code. That's kinda scary. The method just does too much.
Again, a different structure will help. Also, Ruby's case statements are pretty powerful. Might come in handy instead of if...elsif...else.
The regex used to match numbers is very loose. It'll accept anything as long as there's a digit somewhere in it. So "u wot m8" is technically a number in the eyes of your code.
The (res.to_i == res) ? (puts res.to_i) : (puts "%0.1f" % res) line is several kinds of nasty. It'd be much cleaner if you put the puts first, and used the ternary for its arguments instead. Right now it's "if A then puts, otherwise if B then... well, then also puts". Also you don't want to use a ternary's branches for anything except the simplest things. You'll want to use them to decide between values, not actions. So calling to_i on stuff or doing simple arithmetic is fine, but don't go too far. If you want to do more, use a proper if..else.
Something like this would make more sense:

puts res.to_i == res ? res : "%0.1f" % res

Or you could use %g instead, which'll ignore decimals for integers:

puts "%g" % res.round(1)

However, it'll truncate large numbers so it's not a clear winner.
Your code accepts a list of operators, but it assumes:

that any string you pass is actually valid (otherwise the reduce call will break)
that all the operators have an arity of 2.

So the code is halfway configurable, and halfway not. Suppose you want to support a ! operator for factorial: For one, ! is not a method on Float, so reduce(n.to_sym) will fail. For another, the factorial operator only has one operand, not two.
Frankly, I wouldn't bother making this configurable unless I was asked to. Or I'd ask if I was unsure, rather than make assumptions.
Speaking of operator arity: The script crashes if the stack's empty and you enter an operator. You assumed that all operators would always have two operands, but you also assumed that there always are two operands.
Not sure what the last example input (* 24) is supposed to do. Push 24 to the stack and multiply? Complain about the input? (Edit: As pointed out in the comments, it's probably just missing linebreak. That would make sense. Even though I should've figured it out myself, I'll just turn that around and say you should proof-read your code comments and documentation :)
Regardless of its intended meaning, it's interesting as test input: It prints * 24 back to you, because you print the raw input, rather than the parsed value. So you don't know what the script actually made of your input. What it did, though, was push 0.0 to the stack, because the code thinks it's a number, but "* 24".to_f just returns zero.
Speaking of what you print, echoing the user's input right back is sort of pointless. I mean, the user just wrote it. Printing the stack (or at least the input after interpretation) would likely be more useful.
Naming. The variable n is too short to mean much. I suppose it means "number", but that only makes sense if the input is indeed a number. I'd just call it input. Also, res is unnecessarily shortened. It's not that bad to write result instead. There's really no reason to abbreviate.
Otherwise, naming is fine - I like that operators and operands are both full words. Still, as mentioned, #start is not a great name for a method that does everything including reading, evaluating, printing and (ironically) stopping. But that's less a question of changing the name; the method itself should be refactored.
If you want to be fancy, you could make a Stack class instead, and implement #*, #+, etc. as methods on that class.
Small fry, but ["+","-","*","/"] could be written more simply as %w(+ - * /). But again, I'd skip making it configurable.

Now, as mentioned I don't know what your exact exercise was. You built a REPL calculator, but perhaps they wanted something that could parse a full expression like "3 4 * 2 /" => 6? Or a calculator "engine" that's callable from code? If that's the case, you kinda went the wrong way to begin with :)

Edit: I should add that I don't intend for this to be harsh. As 200_success points out, it's not at all bad for < 30 mins of coding while under pressure.


Concurrent blocking stack I just want to train before interview. I googled concurrency interview questions and one of the question was write concurrent ready stack.

public interface ConcurrentStack<T> {
    void put(T t) throws InterruptedException;

    T poll() throws InterruptedException;
}

class BlockingConcurrentStack<T> implements ConcurrentStack<T> {

    private List<T> list = new ArrayList<>();
    private int maxSize;

    public BlockingConcurrentStack(int maxSize) {
        this.maxSize = maxSize;
    }

    @Override
    public synchronized void put(T t) throws InterruptedException {
        while (maxSize <= list.size()) {
            wait();
        }
        list.add(t);
        notify();

    }

    @Override
    public synchronized T poll() throws InterruptedException {
        while (list.size() == 0) {
            wait();
        }
        T t = list.get(list.size() - 1);
        list.remove(t);
        notify();
        return t;
    }
}

There is only one concern for me: you should not allow to instantiate the object with max elements below 0 (IllegalArgumentException ). Therefore a proper handling for this situation in your methods has to be made to avoid a neverending blocking. I suggest to return null.

Expecting a timout for your poll-method may be beneficial in several situations. Concurrent implementations of the Java API do define both: a non-argument poll-method and one with a timeout. But I do not see this as mandatory.

Some developers will suggest to make synchronization intransparent to the user of your class introducing an internal lock object and mention thread-safety in JavaDoc. They argue with "defensive programming" to prevent deadlocks. My personal opinion to that: You send the wrong message to the using developer.

Don't get me wrong: Preventing deadlocks is important. But it is more important to know what to do when. That is known as competence. Preventing others to make mistakes is ok. That can be done in several ways. But others to allow a lack of competence is exactly the message you would send. You would try to solve a potential future errors that are not your business. On the other side you allow misusage of objects without habing the other force to correct thhis. My way is: If someone is doing strange things with objects he doesn't own he either should know exactly what he is doing or handle occuring errors and deadlocks. You should know that this is how the majority of developers will NOT argue.