Loading Bitmaps Efficiently Suppose there is a huge bitmap, huge_bitmap, in the drawable-nodpi folder. I want to create a custom view with huge_bitmap as its background:

public class CustomView extends View {

    private Bitmap mBackground;

    public CustomView(Context context) {
        super(context);
    }

    public CustomView(Context context, AttributeSet attrs) {
        super(context, attrs);
    }

    public CustomView(Context context, AttributeSet attrs, int defStyleAttr) {
        super(context, attrs, defStyleAttr);
    }

    @Override
    protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
        super.onMeasure(widthMeasureSpec, heightMeasureSpec);
        mBackground = decodeAndResizeFromResource(getResources(),
            R.drawable.huge_bitmap, getMeasuredWidth(), getMeasuredHeight());
    }

    @Override
    protected void onDraw(Canvas canvas) {
        canvas.drawBitmap(mBackground, 0, 0, mPaint);
        super.onDraw(canvas);
    }

}

where decodeAndResizeFromResource method subsamples the image and loads a smaller, scaled version into memory.

This works fine. However, onMeasure will often be invoked more than once. Lint warns about this:

  Avoid object allocations during draw/layout operations (preallocate and reuse instead)

Allocating objects in methods such as onMeasure, onLayout, onSizeChanged and onDraw should therefore be avoided.

How do I correctly fix the example above?

I obviously don’t want to preallocate the bitmap (assume it will throw an OutOfMemoryError). Since I need the view’s measured width and height in order to subsample/scale accordingly, I need to load the bitmap after measuring.

Here are two possible workarounds:

@Override
protected void onDraw(Canvas canvas) {
    if (mBackground == null) {
        mBackground = decodeAndResizeFromResource(getResources(),
            R.drawable.huge_bitmap, getMeasuredWidth(), getMeasuredHeight());
    }
    canvas.drawBitmap(mBackground, 0, 0, mPaint);
    super.onDraw(canvas);
}

or maybe

private int mWidth, mHeight;

@Override
protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
    super.onMeasure(widthMeasureSpec, heightMeasureSpec);
    final int w = getMeasuredWidth();
    final int h = getMeasuredHeight();
    if (mWidth != w || mHeight != h) {
        mBackground = decodeAndResizeFromResource(getResources(),
            R.drawable.huge_bitmap, w, h);
        mWidth = w;
        mHeight = h;
    }
}

but both feel clumsy. What would you recommend?

To achieve that the first thing for me would be to move your logic away from the view in a MVP way.

With that in place you could use a observer/observable pattern to wrap that logic in a different class that would be triggered when that event happen.

The problem with this is when your onMeasure is triggered before the addObserver is ran, this of course would end up with the decodeAndResizeFromResource() not running on that first call. A way to fix that, if it is an issue, could be using CustomPresenter constructor to make the values manually or use some default values and make that first call.

Something like this:

import java.util.Observable;
import java.util.Observer;

public class CustomPresenter extends View implements Observable {

    private Bitmap mBackground;
    private List observers = new ArrayList();

    public CustomPresenter (Context context) {
        super(context);
    }

    public CustomPresenter (Context context, AttributeSet attrs) {
        super(context, attrs);
    }

    public CustomPresenter (Context context, AttributeSet attrs, int defStyleAttr) {
        super(context, attrs, defStyleAttr);
    }

    @Override
    protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
        super.onMeasure(widthMeasureSpec, heightMeasureSpec);
        hasChanged();
        notifyObservers();
    }

    @Override
    protected void onDraw(Canvas canvas) {
        canvas.drawBitmap(mBackground, 0, 0, mPaint);
        super.onDraw(canvas);
    }

    public Map getHugeBitmap(){
        return this.drawable.huge_bitmap;
    }

    public void registerObserver(Observer observer) {
        observers.add(observer);
    }

    public void removeObserver(Observer observer) {
        observers.remove(observer);
    }

    public void notifyObservers() {
        for (Observer ob : observers) {
            ob.update(this);
        }
    }
}

Then the "View"

    public class CustomResizerView implements Observer{
       public void update(Object obj){
           CustomPresenter view = (CustomPresenter) obj;
           mBackground = decodeAndResizeFromResource(view.getResources(),
                   view.getHugeBitmap, view.getMeasuredWidth(), view.getMeasuredHeight());
       }
}

Then the test

public class Test {
    public static void main(String args[]) {
        CustomPresenter view = new CustomPresenter();
        CustomResizerView resizer = new CustomResizerView();
        view.registerObserver(resizer);
    }
}


Sliding window calculation using R I have the following function count.ones that takes in a data frame data.l in which the observations have been subdivided into bins defined by indices, a vector of consecutive indices x, and the length of the sliding window width over consecutive indices. The goal of this function is to slide over the indices and count the number of observations labeled as ones.

count.ones = function(x,data.l,width){
  labvec = data.l$lab
  indvec = data.l$index
  return(sapply(1:(length(x)-(width-1)),
                       function(j) sum(labvec[indvec%in%x[j:(j+(width-1))]])))
}

My data is as follows:

library(ks)
library(zoo)
library(plyr)
library(BBmisc)
library(ggplot2)
library(rje)

set.seed(1234)
n1 = n2 = 50000
X1 = rnorm.mixt(n=n1,mus=c(0.4,0.8),sigmas=c(0.04,0.04),props=c(0.97,0.03))
X2 = rnorm.mixt(n=n2,mus=c(0.4,0.8),sigmas=c(0.04,0.04),props=c(0.98,0.02))

n1 = length(X1)
n2 = length(X2)

#pooled observations - label X1's as 1 and X2's as 0
dat = data.frame(X=c(X1,X2),lab=c(rep(1,n1),rep(0,n2)))

N = n1+n2

#number in each bin
n = N^(1/3)

dat$quant = with(dat, cut_number(dat$X, n = round(N/n)))

#number of bins
M = length(levels(dat$quant))
dat$index = factor(dat$quant, levels = levels(dat$quant),
                   labels = 1:length(levels(dat$quant)))

Example function call:

indx.contig = seq(M)
count.ones(x=indx.contig,data.l=dat,width=1) #can vary width for any integer >= 1

This function seems to be slower than expected as it's only performing the simple task of counting the number of 1's in each sliding window. This would become problematic as n1 and n2 increase into the millions (which is ultimately what I want to use). Is there a way to speed this up? I initially looked into rollapply in the zoo package, but it is not as flexible as my code.

It is important for me to look at consecutive indices. If, for example, I had:

indx.vec = c(1:10,14:20)

I would first create a list with two elements: one containing 1:10 and the other containing 14:20. I would then run count.ones on each of the elements using sapply.

Your code is inefficient because each iteration within the sapply has to search through the 100k rows of data to find the relevant ones. Without changing your code at all, you could get a significant speed improvement by first pre-aggregating your input data:

dat <- aggregate(lab ~ index, data = dat, FUN = sum)

This way, each iteration will only have to search through $M = 2154$ rows instead of the 100k. But it is not fixing everything and I prefer rewriting your code with your more general goal in mind, where

indx.vec <- c(1:10, 14:20)

First, aggregate your data using tapply:

agg.lab <- with(dat, tapply(lab, list(index), FUN = sum))
length(agg.lab)
# [1] 2154
head(agg.lab)
#  1  2  3  4  5  6
# 25 23 26 21 22 21

It returns a vector containing sum(lab) for each distinct value of index. In the event that the data has no value for a given index, you could overwrite the output-ed NA with a 0 so as to not affect the upcoming roll sums:

agg.lab[is.na(agg.lab)] <- 0

Next, we subset by only keeping the indices in indx.vec

agg.lab   <- agg.lab[indx.vec]
#  1  2  3  4  5  6  7  8  9 10 14 15 16 17 18 19 20
# 25 23 26 21 22 21 19 21 24 28 21 18 24 22 23 21 20

Next, we map each value of indx.vec to the "group number" it belongs to (here, 1:14 belong to group #1 then 14:20 belong to group #2):

indx.grp  <- cumsum(c(TRUE, diff(indx.vec) != 1))
# [1] 1 1 1 1 1 1 1 1 1 1 2 2 2 2 2 2 2

Using this vector of group indices, we break agg.lab into a list of vectors, one for each group number:

agg.split <- split(agg.lab, indx.grp)
# $`1`
#  1  2  3  4  5  6  7  8  9 10
# 25 23 26 21 22 21 19 21 24 28
#
# $`2`
# 14 15 16 17 18 19 20
# 21 18 24 22 23 21 20

Finally, we rollsum within each vector (where I used width = 4 for illustration)

out <- lapply(agg.split, zoo::rollsum, k = width)
# $`1`
#  2  3  4  5  6  7  8
# 95 92 90 83 83 85 92
#
# $`2`
# 15 16 17 18
# 85 87 90 86

With indx.vec <- 1:M, this code runs in 0.015 seconds on my machine, versus 12+ seconds for your count.ones. It also scales pretty well as it
takes less than 0.4 sec for n1 = n2 = 1e6.


A child ID getting all parent IDs Is there any MySQL function that will optimize this code?

function get_parents() {

$ids = array();

while($id) :
    $query = "SELECT placement_id FROM referrals WHERE user_id = $id";
    $query = $this->db->query($query);
    $result = $query->row();

    if(!isset($result->placement_id)) :
        break;
    elseif(isset($result->placement_id) && $result->placement_id == 2) :
        break;
    endif;

    $id = $result->placement_id;
    array_push($ids, $id);

    if($result) :
        continue;
    endif;

    break;
endwhile;

return $ids;

}

The code above will return all parent ID of given user_id, and this will stop if nothing is found. I found this code too slow and heavy load.

My table:

relations table
| id  |   user_id |   placement_id    |
| 1   |       2   |       NULL        |
| 2   |       3   |       2           |
| 3   |       4   |       2           |
| 4   |       5   |       3           |
| 5   |       6   |       4           |
| 6   |       7   |       3           |
| 7   |       8   |       3           |
| 8   |       9   |       3           |
| 9   |       10  |       6           |
| 10  |       11  |       5           |
| 11  |       12  |       6           |
| 12  |       13  |       4           |
| 13  |       14  |       3           |
| 14  |       15  |       9           |
| 15  |       16  |       10          |

user_id is the child and parent is placement_id.

MySQL and caching

From the manual (emphasis added)

  The query cache stores the text of a SELECT statement together with the corresponding result that was sent to the client. If an identical statement is received later, the server retrieves the results from the query cache rather than parsing and executing the statement again.

To test out, I recreated the table you posted in MySQL Workbench:

use test;
drop table if exists referrals;
create table referrals(
    id int not null primary key,
    user_id int,
    placement_id int
);
insert into referrals(id, user_id, placement_id)
values
(1,          2,         NULL),
(2,          3,          2),
(3,          4,          2),
(4,          5,          3),
(5,          6,          4),
(6,          7,          3),
(7,          8,          3),
(8,          9,          3),
(9,          10,         6),
(10,         11,         5),
(11,         12,         6),
(12,         13,         4),
(13,         14,         3),
(14,         15,         9),
(15,         16,        10);

I ran the following query 3 times:

SELECT placement_id FROM referrals WHERE user_id = 3;
-- 1st time 0.002 sec / 0.000 sec
-- 2nd time 0.000 sec / 0.000 sec
-- 3rd time 0.000 sec / 0.000 sec

As you can see, each time you run it with the same user_id it will get it from the cache. But if it's not the same user_id it will start a new query.

Stored statement

Assuming your actual table is much larger than the example provided, you would likely benefit from creating a stored statement, which will retain the execution plan along with the query. To create the statement, just pass this to MySQL once, either from Workbench or from PHP:

delimiter $$
    create procedure FindPlacementId (in param_user_id int)
    begin
    SELECT placement_id FROM referrals WHERE user_id = param_user_id;
    end$$
delimiter ;

Then this section:

$query = "SELECT placement_id FROM referrals WHERE user_id = $id";

Becomes:

$query = "CALL FindPlacementId ($id);";
-- a few tests:
-- 0.001 sec / 0.000 sec
-- 0.000 sec / 0.000 sec
-- 0.000 sec / 0.000 sec


Search a value across 2 array I want to find the corresponding hash in an array from a string that contains a criterion defined in a hash of the array.

I do something like that :

types = [
 {key: 'type_1', criteria: ['type_1a', 'type_1b']},
 {key: 'type_2', criteria: ['type_2a', 'type_2b']},
 ...
]

def find_type(str)
  types.each do |type|
    type[:criteria].each do |criterion|
      return type if str =~ /#{criterion}/i
    end
  end
  nil
end

I'm sure it could be more ruby but don't find how...

The orthodox (and functional) approach in Ruby is:

def find_type(types, str)
  types.detect do |type|
    type[:criteria].any? do |criterion|
      str =~ /#{criterion}/i # or Regexp.new(criterion, "i")
    end
  end
end


Performance optimization for Linq To SQL I have been working with C# for quite some time but relatively new to the concepts of lambdas and Linq. I was working with a Linq To SQL example and trying to write a generic solution for executing a search against a collection of entities (eg. list of Customer objects) where the search parameter will be specified by passing a partially filled object of the entity class itself. I was doing this just to ensure that a person does not have to go on specifying different overloads for different searches based on the entity class attributes and trying to write a generic solution which will cater to any entity class.

Hence I used reflection and an array of predicates to successively apply the Where clauses to the collection.

Client App

using (CustomerManager oCustomerManager = new CustomerManager())
{
   IEnumerable<Customer> customers = oCustomerManager.Load();

   Customer oSearchCustomer = new Customer();
   oSearchCustomer.City = "London";
   oSearchCustomer.ContactName = "Thomas Hardy";

   IEnumerable<Customer> customerList = oCustomerManager.Search(oSearchCustomer);

   foreach (Customer customer in customerList)
   {
      Console.WriteLine(customer.ToString());
   }
}

Manager Class

public IEnumerable<Customer> Search(Customer searchObject)
{
    IEnumerable<Customer> customers = DataContext.Customers;
    List<Func<Customer, bool>> result =

    DataContext.Customers.GenerateFilterClause<Customer>(searchObject);

    foreach (var item in result)
    {
        customers = customers.Where(item);
    }

    return customers;
 }

Extension Method Utility Class

public static class UtilityExtensions
{
   public static List<Func<T, bool>> GenerateFilterClause<T>(this IEnumerable<T>
          collection, T searchEntity)
   {
        List<Func<T, bool>> whereFilterList = new List<Func<T, bool>>();
        Func<T, bool> predicate = null;

        var propertyList = searchEntity.GetType().GetProperties();

        foreach (PropertyInfo p in propertyList)
        {
            if (p.GetCustomAttributes(false).OfType<ColumnAttribute>().Count() > 0)
            {
               string propName = p.Name;
               var searchVal =

               searchEntity.GetType().GetProperty(propName).GetValue(searchEntity,null);
               if (searchVal != null)
               {
                  predicate = new Func<T, bool>(entity => propertyWhereClause(entity,
                         searchEntity, propName, searchVal));
                  whereFilterList.Add(predicate);
               }
            }
         }

         return whereFilterList;
     }

      private static bool propertyWhereClause<T>(T obj, T searchEntity, string    propertyName, object searchVal)
     {
           return obj.GetType().GetProperty(propertyName).GetValue(obj,
                                   null).Equals(searchVal);
      }
  }

While the solution is working, I have two specific questions:

What are the drawbacks of this solution? I was hoping to have inputs regarding performance and cleaner way to rewrite the code.
Can anyone suggest a more efficient and better solution? That would help me to understand the proper way to utilize the power of lambdas and Linq.

How much did you look into LINQ to SQL? Unless you have a reason not to expose the DataContext, you can just perform queries on it without using the 'search object'.

from c in DataContext.Customers
where c.City == "London" && c.ContactName == "Thomas Hardy"
select c

I haven't used LINQ to SQL myself yet, so I can't really tell much about the best practices and whether or not exposing DataContext is a good idea. Probably it's best to use an existing ORM framework. DataObjects.NET looks really nice.

A nice ORM overview can be found on ORMbattle.


Updating all members of a struct in database This function is part of our company's project code.
The following function updates six members of struct CpGSMTimeDelayQuery -- t14_cp, T14_au, Tdl_fib_f, Tul_fib_f, Tdl_cp_f, Tul_cp_f -- in the database.

It contains six similar blocks. These blocks do the same thing: read a specific register, calculate the member value using a specific formula, and then set database.

typedef unsigned int    Uint32;

void FpgaAdapter::UpdateCpGsmDelayTime()
{
    Uint32 regvalue(0);
    Uint32 result(0);
    float tmp;

    if (ReadReg(0x98, regvalue)) {
        result = (float)regvalue * 6.51;
        SetStructElementToDB("CpGSMTimeDelayQuery", "t14_cp", result);
    }

    if (ReadReg(0x99, regvalue)) {
        result = (float)regvalue * 6.51;
        SetStructElementToDB("CpGSMTimeDelayQuery", "T14_au", result);
    }

    if (ReadReg(0x9c, regvalue)) {
        result = (float)regvalue * 6.51;
        SetStructElementToDB("CpGSMTimeDelayQuery", "Tdl_fib_f", result);
    }

    if (ReadReg(0x9d, regvalue)) {
        result = (float)regvalue * 6.51;
        SetStructElementToDB("CpGSMTimeDelayQuery", "Tul_fib_f", result);
    }

    if (ReadReg(0xa0, regvalue)){
        tmp = (0xff & regvalue) * 32.55;
        result = ( 0xff & (regvalue>>8)) * 1000 + (Uint32)tmp;
        SetStructElementToDB("CpGSMTimeDelayQuery", "Tdl_cp_f", result);
    }

    if (ReadReg(0xa1, regvalue)){
        tmp = (0xff & regvalue) * 32.55;
        result = (0xff & (regvalue>>8)) * 1000 + (Uint32)tmp;
        SetStructElementToDB("CpGSMTimeDelayQuery", "Tul_cp_f", result);
    }
}

How can I refactor this function so that I can use a for loop to update all members of the struct instead of using six similar blocks?

My thought:
Create a map which key is member name, and value is register address and calculation formula. Then loop through the map to update all members in the database.

But, I don't know how to do that because I am not familiar with C++.

One way you can simplify this code and also make it more forward compatible with future addition of new fields is by using a simple array that stores the register keys, database field names and a pointer to a function that transforms the register value in the adequate way. You should also be able to remove all duplicate code in the process.

The following example uses auto and C++11 lambdas to keep everything close together, but you can easily replace that by plain functions if you don't have access to a C++11 compiler. Also note that I don't have enough context to pick more meaningful names other than commands and xform, but you should be able to find better ones.

void FpgaAdapter::UpdateCpGsmDelayTime()
{
    auto xform1 = [](Uint32 regValue) {
        return static_cast<Uint32>(regValue * 6.51);
    };

    auto xform2 = [](Uint32 regValue) {
        const float tmp = (0xff & regValue) * 32.55;
        return (0xff & (regvalue >> 8)) * 1000 + static_cast<Uint32>(tmp);
    };

    static const struct {
        Uint32 regKey;
        const char * dbField;
        Uint32 (*transform)(Uint32);
    } commands[] = {
        { 0x98, "t14_cp",    xform1 },
        { 0x99, "T14_au",    xform1 },
        { 0x9c, "Tdl_fib_f", xform1 },
        { 0x9d, "Tul_fib_f", xform1 },
        { 0xa0, "Tdl_cp_f",  xform2 },
        { 0xa1, "Tul_cp_f",  xform2 }
    };

    for (int i = 0; i < sizeof(commands) / sizeof(commands[0]); ++i) {
        Uint32 regValue = 0;
        if (ReadReg(commands[i].regKey, regValue)) {
            const Uint32 result = commands[i].transform(regValue);
            SetStructElementToDB("CpGSMTimeDelayQuery", commands[i].dbField, result);
        }
    }
}


Hex to ASCII conversion in Haskell I wrote a simple program in Haskell which takes Hex input from stdin, converts the input to ascii and writes it to stdout. An example use:

$ echo -n '48656c6c6f2c2043522e534521' | unhex
Hello, CR.SE!

And here the source code:

import qualified Data.ByteString as B
import Data.Bits
import Data.Word

unhex :: [Word8] -> [Word8]
unhex [] = []
unhex (x:[]) = error "invalid input length"
unhex (x:y:xs) = [(shiftL (hex2val x) 4) .|. (hex2val y)] ++ unhex xs
    where hex2val x =  hex2val' $ (x .|. 0x20) -- lowercase char
          hex2val' x
        | x >= 48 && x < 48 + 10 =  0 + x - 48 -- 48 is '0' in ascii
        | x >= 97 && x < 97 +  6 = 10 + x - 97 -- 97 is 'a' in ascii
        | otherwise              = error "invalid hex input"

main = do
    input <- B.getContents
    B.putStrLn $ B.pack $ unhex $ B.unpack input

This is one of my first Haskell programs I wrote, would be interesting to get some feedback on how to accomplish this task more the "Haskell" way. I'd like to know if this kind of error reporting is appropriate.

Calling error is fine for a small program like this.

For more advanced error handling see the Failure page on HaskellWiki.

Some comments about your code:

unhex (x:[]) = error "invalid input length"

x is not used here, use this pattern instead: (_:[])

unhex (x:y:xs) = [(shiftL (hex2val x) 4) .|. (hex2val y)] ++ unhex xs

[x] ++ xs is usually written x : xs

hex2val' $ (x .|. 0x20)

$ is useless here.


Parameterized FizzBuzz This is a FizzBuzz program, designed to take parameter inputs for start and ending numbers, as well as a "fizz" number and a "buzz" number. The program then outputs the numbers one per line. If the number is divisible by the "fizz" number (normally 3), "fizz" is printed instead, the same with the "buzz" number. If the number is divisible by both, "fizzbuzz" is the output.

I am primarily looking for advice with my choice of logic, syntax, and use of methods with regards to complexity and efficient code. However, all advice is appreciated.

using System;

class FizzBuzzProgram
{
static void Main()
{
    //initialize menu variables
    bool menuvalidinput = false;
    string menuchoice;

    /*get decision if user wants to fizzbuzz, looping
    until a valid input is provided*/
    do {
        Console.WriteLine("Would you like to fizzbuzz? [Y]/[N]");
        menuchoice = Console.ReadLine();
        menuchoice = menuchoice.ToLower();  //convert to lowercase
        if (menuchoice == "y" | menuchoice == "n"){
            menuvalidinput = true;          //accept input
        } else {
            Console.WriteLine("Sorry, input was not valid.");
        }
    } while (menuvalidinput == false);
    //end of input loop

    //interpret the result of either "y" or "n"
    if (menuchoice == "y"){
        FizzBuzzWithInput();
    } else {
        Console.WriteLine("That's ok, have a nice day!");
    }
}

static void FizzBuzzWithInput()
{
    //initialize fizzbuzz input (FBinput*) variables
    int FBinputS, FBinputE, FBinputF, FBinputB;
    bool FBvalidinput = false;

    /*take input for start and end values,
    if start is larger than end ask again*/
    do {
        Console.WriteLine("What number would you like to start from?");
        FBinputS = Convert.ToInt32(Console.ReadLine());
        Console.WriteLine("What number would you like to end at?");
        FBinputE = Convert.ToInt32(Console.ReadLine());

        if (FBinputS > FBinputE){   //check of start and end values
            Console.WriteLine("You chose a starting number larger than your ending number;");
            Console.WriteLine("please input new values.");
        } else {
            FBvalidinput = true;    //accept input
        }
    } while (FBvalidinput == false);
    //end validation loop

    //take fizz and buzz inputs, classically 3 and 5 respectively
    Console.WriteLine("What number would you like to fizz?");
    FBinputF = Convert.ToInt32(Console.ReadLine());
    Console.WriteLine("What number would you like to buzz?");
    FBinputB = Convert.ToInt32(Console.ReadLine());

    //call the actual FizzBuzz method
    RunFizzBuzz(FBinputS, FBinputE, FBinputF, FBinputB);
}

static void RunFizzBuzz(int FBstart, int FBend, int FBfizz, int FBbuzz)
{
    //initialize the display string
    string c;

    Console.WriteLine("\nFizzbuzz time!\n");

    //fizzbuzz loop
    for (int i = FBstart; i <= FBend; i++){
        //set c to blank string
        c = "";

        if(i%FBfizz == 0){
            c = c + "fizz";
        }
        if(i%FBbuzz == 0){
            c = c + "buzz";
        }
        if(c.Length == 0){  /*"if the number is not fizz or buzz"
                            aka: if(i%FBfizz!=0 & i%FBbuzz!=0)*/
            c = Convert.ToString(i);
        }
        //print display string of number i
        Console.WriteLine(c);
    }
    //end fizzbuzz loop

    Console.WriteLine("\nFizzbuzz complete!");
    Console.ReadLine();
}
}

Some quick remarks

variables should be named using camelCase casing
avoid declaring multiple variables on the same line to improve readability.
don't trust the users of your app! Never trust them! If you think, hey they can read and follow instructions you will get into great pain. Always check that inputs are valid. That beeing said you should use int.TryParse() instead of Convert.ToInt32()
method arguments should be named using camelCase casing as well.
let your variables have some room to breathe (see the % operator)
don't use abbreviations for naming stuff. In a few months you won't know what they are about.
comments should explain why something is done in the way it is. Let the code speak for itself what is done by using names as descriptive as possible.


Frequency form to case form - unexpected values in table I'm taking count data on voting preferences by race and entering it into a table in frequency form, then expanding out to case form and using CrossTable to look at adjusted residuals. Here's my script:

### create table in frequency form
vnames <- c("Race", "Party", "freq")
a <- c("White","Dem", 871)
b <- c("White","Ind", 444)
c <- c("White","Rep", 873)
d <- c("Black","Dem", 302)
e <- c("Black","Ind", 80)
f <- c("Black","Rep", 43)

rtable <- rbind(a,b,c,d,e,f)
colnames(rtable) <- vnames

###check table - looks fine
head(rtable)

###expand to case form
rtablecase <- expand.dft(rtable, freq = "freq")

###check table - *here's where it gets weird*.
head(rtablecase)

The first two rows hold the value of "1" in the race column rather than "White". Everything else is as expected. Why? I eventually just manually replaced the values (see below) so I could run CrossTable and it looks fine, but can someone please tell me where these two 1s are coming from?*

### replace 1s in Race column
rtablecase[1,1] <- "White"
rtablecase[2,1] <- "White"

###convert to data frame
rtcasedf <- data.frame(rtablecase)
head(rtcasedf)

###Crosstabulation with adjusted residuals
CrossTable(rtcasedf$Race, rtcasedf$Party, format="SPSS", asresid=TRUE, chisq=TRUE)

The behavior seems to be due to a bug in the expand.dft function. I created a simple function myfun that allows you to create a matrix similar to the one created by expand.dft.

myfun <- function(x, freq = "Freq") {
  # repeat the row indices according to `freq`
  idx <- rep(seq(nrow(x)), as.integer(x[, freq]))
  # expand matrix and ignore `freq` column
  x[idx, colnames(x) != freq]
}

Note that is function is not meant to replace expand.dft but works for simple cases only.

Try it with your data set rtable:

rtablecase <- myfun(rtable, freq = "freq")

The result:

head(rtablecase)
#   Race    Party
# a "White" "Dem"
# a "White" "Dem"
# a "White" "Dem"
# a "White" "Dem"
# a "White" "Dem"
# a "White" "Dem"


Generating probability table of 2-character combinations from text file in Java I have written a probability table generator in Java to help me generate randomized girl's names for the game I am making.
It reads from a text file and creates a HashMap<String, Double> full of two-character combinations and their rate of incidence.
I don't have a lot of experience with either probability tables or Java in general.
I would like help making it run faster
Example text from input file:
Each line has ten names. The real text file has 462 lines, and 4620 names.

Marva Kandy Debbi Louisa Edris Orpha Lynn Louise Reyna Jacki Evangeline
Ray Sienna Alecia Elin Debby Noreen Alishia Michael Evangelina Eliz
Usha Ladawn Dominga Cynthia Marty Shayla Sanora Betsey Ira Jodee
Qiana Elia Elke Torie Amirah Marta Lavonna Ozie Lavonne Teofila
Latoria Catharine Epifania Susy Keli Delpha Isa Marth Rosina Marti
Jordan Ella Theda Tyesha Rana Francesca Shayna Letha Shayne Marry
Rea Joellen Fidela Kandi Celeste Salina Kamryn Jenelle Celena Celesta
Kenyatta Ada Shelia Maribel Edith Lorilee Jazmyn Myrtice Kena Laurence

My code
public class MCVE {

    public static void main(String[] args) throws Exception {
        // create the file-path
        String address = "./res/text_files/Human_FirstNames";
        // will hold letter combinations and their frequency
        HashMap<String, Double> frequencies = new HashMap<>();

        FileReader fr = new FileReader(address);
        BufferedReader br = new BufferedReader(fr);

        // Iterate through file
        // get rid of new-line characters and concat to wholeString
        String aLine = "";
        String wholeString = "";
        while((aLine = br.readLine()) != null) {
            wholeString += aLine.replaceAll(System.lineSeparator(), " ");
        }

        // convert names to lowercase and into char[]
        char[] charArr = wholeString.toLowerCase().toCharArray();

        // iterate through char[] charArr and
        // add the number of times each character-combination is shown
        // to the value for each character-combination key.
        int length = charArr.length;
        String tempString = "";
        for(int i = 0; i < length; i++) {

            if(i + 1 == length)
                break;

            if(charArr[i] != ' ' && charArr[i + 1] != ' ') {
                tempString = Character.toString(charArr[i]) + Character.toString(charArr[i + 1]);
                if (frequencies.containsKey(tempString))
                    frequencies.replace(tempString, frequencies.get(tempString) + 1.0);
                else // if it doesn't contain this key yet, create it
                    frequencies.put(tempString, 1.0);
            }
        }

        // iterate through HashMap frequencies and
        // replace the values with the product of
        // (original_value / size_of_HashMap)
        double mapLength = frequencies.size();
        Iterator iter = frequencies.entrySet().iterator();
        while(iter.hasNext()) {
            HashMap.Entry pair = (HashMap.Entry)iter.next();

            frequencies.replace((String) pair.getKey(), (Double) pair.getValue() / mapLength);

            System.out.println(pair.getKey() + " " + pair.getValue());
            iter.remove();
        }
    }
}

I have looked through your code. Here are a few things I notice

StringBuilder instead of using += on String, for more efficient string concatenation.

Creating a new instance of StringBuilder and using that would work better than using the + operator on Strings. Something like this:

StringBuilder builder = new StringBuilder();
// inside the loop call the following
builder.append("some string to append");
// ...
// later on when you need the string
builder.toString();

More descriptive variable names

Try to explain what your variable represents instead of just calling it wholeString. To you, you may very well remember the significance of each variable and what it represents, but this is a good coding practice to get into because code will be read more than written and it would be easier for anyone else reading your code to immediately tell what it's doing without having to in many cases even read comments. This is called "self-documenting code".

JDK 8+ Files API

After JDK 8 you can call lines() using the Files API. Although your approach is correct using BufferedReader and FileReader, you may want to consider switching to using the lines() method and operating on the Stream returned because it is a bit less verbose which makes reading it easier.

Succinct method chaining using Stream over iterative approach

If you switch to using Stream for the file, you can easily adopt a more functional approach of calling filter or other stream related functions on it. This is a bit cleaner to read than the iternative while loop to remove the newlines in my view.

Multiline comments using multiline comment syntax

For in-depth explanations and long comments that span to multiple lines, you can use the following notation:

/*

This comment

spans

multiple lines

*/

However, with that being said - I think the comments in this code are a bit much. Using self documenting variables as mentioned above, comments can be lessened and the code would be more readable too.

Hope that helps, let me know if you have any questions on the feedback I've given.


Can this iterating javascript code be optimized? Say I have this type of code I'd like to execute. There's some data encapsulated around a big Javascript object containing methods & properties that I'd like to extract based on comparing some other data in said object. The object includes associative (hash?) arrays where I don't know the array IDs at runtime so I have to do some iteration, this is the working code structure I came up with. But it's bloated and a bit messy to read. Was wondering it it can be optimized for efficiency and readability in general, to minimize iteration and conside the length of the object referencing.

I execute this code from Firebug console (or via Selenium WebDriver's execute javascript command, in which case console.log become "return" to return the desired value).

for(x in MainObj.aaa.bbb[MainObj.someGetIdMethod()].someObjArry){
    if (MainObj.aaa.bbb[MainObj.someGetIdMethod()].someObjArry[x].someObj.displayName == 'some value')
        console.log(MainObj.aaa.bbb[MainObj.someGetIdMethod()].someObjArry[x].someObj.someId);
}

You should cache all methods calls - JS is interpreting language and
there is no optimizers to help you, therefore, each time you call
method, get element from array etc, you are actually asking JS
engine to DO this for you.
Concluding that someObjArry is an array, should avoid using loops with "in" statements - use standard loops with indexes instead. "in" statement walks through all object's properties, not collection's elements, so it's just future maintenance "watch-out".
Property "displayName" of object "someObj" seems like of a string type - try using strong type comparisons if possible (not an optimization, just a good practice)
Cache as much as you can

var someArray = MainObj.aaa.bbb[MainObj.someGetIdMethod()].someObjArry,
    arrayElement;

for(var i=0, $length = someArray.length; i< $length; i++){
    arrayElement = someArray[i].someObj;

    if (arrayElement.displayName === 'some value'){
        console.log(arrayElement.someId);
    }
}


LINQ to SQL Joining Entities I have two tables, one for jobs, and one for the names of industries (e.g. automotive, IT, etc).

In SQL I would just do:

SELECT industryName, count(*)
FROM jobs
JOIN industry
ON jobs.industryId = industry.id
GROUP BY industryName

In LINQ I have the following, but it's three separate statements and I'm pretty sure this would be doable in one.

var allIndustries =
    from j in dbConnection.jobs
    join i in dbConnection.industries on j.industryId equals i.id
    select i.industryName;
var industriesWithCount =
    from i in allIndustries
    group i by i into iGrouped
    select new { Industry = iGrouped.Key, Count = iGrouped.Count() };
var industries = new Dictionary<string, int>();
foreach (var ic in industriesWithCount)
{
    industries.Add(ic.Industry, ic.Count);
}

Is there a way to make this simpler or shorter?

Your j and i variables are pretty plain and won't make this code fun to maintain (especially if it were a big block of code)

You should use more descriptive variables.

you should create a collection that groups the Jobs by their Industry and then select from that collection the industry name and the count of jobs in the collection with that name.

from job in dbConnection.jobs
join industry in dbConnection.industries on job.industryId equals industry.id
group new {industry, job} by industry.Name
into jobsByIndustry

select new {
    industryName = jobsByIndustry.Key.Name,
    jobscount = jobsByIndustry.Count()
}


Project Euler 12 (triangle numbers), solved using functools.reduce() and looping I have solved problem 12 on Project Euler website, which reads:

  The sequence of triangle numbers is generated by adding the natural
  numbers. So the 7th triangle number would be 1 + 2 + 3 + 4 + 5 + 6 + 7
  = 28. The first ten terms would be:

  1, 3, 6, 10, 15, 21, 28, 36, 45, 55, ...

  Let us list the factors of the first seven triangle numbers:

  1: 1
  3: 1,3
  6: 1,2,3,6
  10: 1,2,5,10
  15: 1,3,5,15
  21: 1,3,7,21
  28: 1,2,4,7,14,28

  We can see that 28 is the first triangle number to have over five
  divisors.

  What is the value of the first triangle number to have over five
  hundred divisors?

And I've solved this in two ways:

1.functools.reduce()

import math
from functools import reduce
import time

def primeFactors(n):
    i = 2
    factors = {}
    num = 0
    while i ** 2 <= n:
        if n % i:
            i += 1
        else:
            n //= i
            num = factors.get(i, None)
            if num is None:
                factors[i] = 1
            else:
                factors[i] = num+1
    if n > 1:
        num = factors.get(n, None)
        if num is None:
            factors[n] = 1
        else:
            factors[n] = num+1
    return factors

start_time = time.time()
numOfDivisors = 500
n = 2

while True:
    t = int(n*(n+1)/2)
    factors = primeFactors(t).values()

    if reduce(lambda x, y: x*y, [t+1 for t in factors]) >= numOfDivisors:
        print(t)
        break
    else:
        n += 1

print("----%s seconds ----" % (time.time() - start_time))

2.for-loop

import math
import time

def primeFactors(n):
    i = 2
    factors = {}
    num = 0
    while i ** 2 <= n:
        if n % i:
            i += 1
        else:
            n //= i
            num = factors.get(i, None)
            if num is None:
                factors[i] = 1
            else:
                factors[i] = num+1
    if n > 1:
        num = factors.get(n, None)
        if num is None:
            factors[n] = 1
        else:
            factors[n] = num+1
    return factors

start_time = time.time()
numOfDivisors = 500
n = 2

while True:
    t = int(n*(n+1)/2)
    factors = primeFactors(t).values()

    k = 1
    for i in factors:
        k *= (i+1)

    if k >= numOfDivisors:
        print(t)
        break
    else:
        n += 1

print("----%s seconds ----" % (time.time() - start_time))

I tested out which one was faster, found that there was a fine line only:

1.670 secs and 1.678 secs

I attempted to calculate the average computing time by making the given number bigger from 500 to 1000, repeating this process 10 times.

But both solutions were done almost simultaneously:

functools.reduce

[13.616845607757568, 13.67394757270813, 13.623621225357056, 13.596383094787598, 13.657264471054077, 13.694176197052002, 13.669324398040771, 13.65349006652832, 13.66620421409607, 13.57088851928711, 13.686619901657104]
for-loop

[13.56181812286377, 13.686477422714233, 13.548126935958862, 13.565587759017944, 13.562162637710571, 13.556873798370361, 13.562631845474243, 13.572312593460083, 13.57419729232788, 13.567578315734863, 13.611796617507935]

Question: Are there any insignificant or significant differences between the two from a practical view and in a normal case, Or is this just a matter of behavior of two functions to use?

Here's an in-depth view from BDFL.

reduce should be faster, especially if you supply it with function that is written in C . Try replacing your multiplication lambda with operator.mul and see if anything changes.

There is a matter of style which of two functions to prefer, because more complex tasks would require more complex lambdas and the whole thing would be less comprehensible than for-loop. Thus, use of reduce was discouraged and it was moved into functools.

On the other hand, for less complicated tasks there are such things as sum, all and any. Latter two also short-circuit, whereas reduce does not.


Counting the frequency of certain entries in a list of JSON objects I have a list of JSON objects that looks like this:

[{
  url: '/home',
  ...
}, {
  url: '/analytics',
  ...
}]

I'm using the following function to count the frequencies of the URLs in order to pass it to a c3.js chart.

var getSectionFrequencyData = function(data) {
  var frequencies = {};
  data.map(function(entry) {
    var matches = /[a-z]+/g.exec(entry.url);
    var url = 'home';
    if (matches) {
      url = matches[0];
    }
    frequencies[url] = frequencies[url] ? frequencies[url] + 1 : 1;
  });
  var items = Object.keys(frequencies).map(function(section) {
    return [section, frequencies[section]];
  }).sort(function(a, b) {
    return b[1] - a[1];
  }).slice(0, 10);
  return [
    ['sections'].concat(items.map(item => item[0])),
    ['frequency'].concat(items.map(item => item[1]))
  ];
};

This code works, but the frequency counting, sorting, and grouping seems extremely overengineered and clunky. Best practices for this?

The approach in your code is straightforward and hence easy to understand.

Nits:

use more of ES2015+ if you use some parts of it anyway - for example Map is handy here;
don't mix function and => notations - unless the former is absolutely required;
don't use array .map if you don't need the output array - to avoid garbage collection lags;

const getSectionFrequencyData = data => {
  const getSection = entry => /[a-z]+|$/.exec(entry.url || '')[0];

  const frequencies = new Map();
  for (const entry of data) {
    const section = getSection(entry) || 'home';
    frequencies.set(section, (frequencies.get(section) || 0) + 1);
  }

  const sorted10 = new Map(
    [...frequencies.entries()]
      .sort((a, b) => b[1] - a[1])
      .slice(0, 10)
  );

  return [
    ['sections', ...sorted10.keys()],
    ['frequency', ...sorted10.values()],
  ];
};


Karatsuba multiplication in Rust This is an implementation of the Karatsuba algorithm for multiplication:

use std::cmp::max;

/// Multiplies two numbers using the Karatsuba algorithm
fn karatsuba(a: isize, b: isize) -> isize {
    // Single digit multiplication: no need for Karatsuba
    if a < 10 && b < 10 {
        a * b
    } else {
        let nr_of_digits = max(get_nr_of_digits(a), get_nr_of_digits(b));

        let half_nr_of_digits = nr_of_digits / 2;

        let (p, q) = split_at(half_nr_of_digits, a);
        let (r, s) = split_at(half_nr_of_digits, b);

        let u = karatsuba(p, r);
        let w = karatsuba(q, s);
        let v = karatsuba(p + q, r + s);

        // Since we used integer division for half_nr_of_digits,
        // half_nr_of_digits * 2 is not always equal to nr_of_digits.
        // For example when nr_of_digits is 9.
        let raised_u = u * 10_isize.pow(half_nr_of_digits * 2);

        let raised_v_w_u = (v - w - u) * 10_isize.pow(half_nr_of_digits);

        // That's the product of a and b
        raised_u + raised_v_w_u + w
    }
}

/// Gets the number of digits in a number. For example:
/// get_nr_of_digits(12345) == 5
fn get_nr_of_digits(x: isize) -> u32 {
    let mut nr_of_digits = 1;
    let mut copy = x;
    while copy > 9 {
        copy /= 10;
        nr_of_digits += 1;
    }

    nr_of_digits
}

/// Splits a number at a position. For example:
/// split_at(2, 1234) == (12, 34)
fn split_at(pos: u32, x: isize) -> (isize, isize) {

    let power = 10_isize.pow(pos);

    let high = x / power;
    let low = x % power;
    (high, low)
}

#[test]
fn split_at_works() {
    assert_eq!(split_at(2, 1234), (12, 34));
    assert_eq!(split_at(1, 67), (6, 7));
    assert_eq!(split_at(2, 67), (0, 67));
    assert_eq!(split_at(2, 674), (6,74));
    assert_eq!(split_at(2, 67461), (674, 61));
    assert_eq!(split_at(3, 674610), (674, 610));
}

#[test]
fn karatsuba_works() {
    // Positive numbers
    assert_eq!(karatsuba(12, 34), 12 * 34);
    assert_eq!(karatsuba(3, 4), 3 * 4);
    assert_eq!(karatsuba(5678, 4321), 5678 * 4321);
    assert_eq!(karatsuba(678, 4321), 678 * 4321);
    assert_eq!(karatsuba(67, 65432), 67 * 65432);
    assert_eq!(karatsuba(671, 654), 671 * 654);
    assert_eq!(karatsuba(6781001, 6542001), 6781001 * 6542001);
    assert_eq!(karatsuba(671, 654), 671 * 654);
    assert_eq!(karatsuba(67, 654321), 67 * 654321);
    assert_eq!(karatsuba(678032, 432132012), 678032 * 432132012);

    // Negative numbers
    assert_eq!(karatsuba(-678, 432), -678 * 432);
    assert_eq!(karatsuba(678032, -232132012), 678032 * -232132012);
    assert_eq!(karatsuba(571, -654), 571 * -654);
}

#[test]
fn get_nr_of_digits_works() {
    assert_eq!(get_nr_of_digits(0), 1);
    assert_eq!(get_nr_of_digits(10), 2);
    assert_eq!(get_nr_of_digits(12345), 5);
    assert_eq!(get_nr_of_digits(87654321), 8);
}

I'd love to know how to make this faster and rustier.

I can only think of two good reasons for using base 10:

The purpose of the code is to teach, and the person (could be yourself) or people being taught are more comfortable with base 10 than base 2.
The underlying representation of the integers is in base 10.

Note that case 2 is extremely unlikely. And since you're asking about speed, I assume that case 1 doesn't apply either (although see below). Therefore you should probably be using a base which is a power of 2. The only case in which this isn't true is where profiling showed that the bottleneck was converting your final output to base 10 for printing and so you chose to write a custom big integer implementation which uses a base which is a power of 10: I have personally encountered this situation, and chose to work base 1000000000 so that the base would fit in a 32-bit integer and the product of two digits fit within a 64-bit integer.

Note that working with a base which is a power of 2 means that all of the power operations can be done with bitshifts, or if you're able to access the internal data structure of a big integer then by index adjustments.

Note also the reference to big integers above. Working (isize, isize) -> isize, there's no point using Karatsuba except for teaching purposes. It would be faster to use *.


Snippet menu using jQuery and PHP I have been trying to manage users using jQuery & PHP instead using just PHP because I think it is more comfortable to use jQuery & PHP but using just PHP is easier at my personal point though more organized.

Since I'm not an expert using jQuery I would like to know if the way I'm managing my users is optimal or should I return to full PHP?

Let's take an example of how I work with jQuery+PHP.

I have 3 kind of users:

Guests
Users
Admins

I will start with my Login.php which code I will skip because I think is irrelevant. Login.php will start my sessions.

Session_handler.php

This helps me to focus which type of users is logged at the moment:

session_start();
if(isset($_SESSION['Usuario']))
{
    if( $_SESSION['estado'] == 'User'){
    $send = array('ID' => $_SESSION['ID'], 'user' => $_SESSION['Usuario'], 'estado' => 'usuario' );
    echo json_encode($send);
    }
    if( $_SESSION['estado'] == 'Admin'){
    $send = array('ID' => $_SESSION['ID'], 'user' => $_SESSION['Usuario'], 'estado' => 'admin' );
    echo json_encode($send);
    }

}
else{
    $send = array('estado' => 'invitado' );
    echo json_encode($send);
}

Sessions.js

Now I will manage my users menu depending on which type of user is logged in:

$(document).ready(function(){
    $.ajax({
        url:'js/Session_handler.php',
        success:function(data) {
            var result = $.parseJSON(data);
            var i = 0;
            var carrito = '

';
            var admin = '
[Admin] '+result.user+'
';
            var cuenta = '
'+result.user+'
';
            var navegacion = '
Entrar
';
            var registrase = '
Registrarse
';
            var salir = '
Salir
';
            if(result.estado == 'invitado'){
                $('#snipuser').append(navegacion,registrase);
            }
            if(result.estado == 'usuario'){
                $('#snipuser').append(carrito,cuenta,salir);
            }
            if(result.estado == 'admin'){
                $('#snipuser').append(carrito,admin,salir);
            }
        }
    });
});

Then I just need to call my id to show the menu depending on which user is logged in:

Index.html

OK, now I will ask again in case my doubt was not clear.

Is it more effective to work this way (JQuery+PHP) than working with full PHP? What does it mean to work with full PHP? I will show an example.

Login.php that will start my sessions.

Now instead working with Index.html I will be bound change my file extension to Index.php and work my menus like this:

<?php
  session_start();
   if(isset($_SESSION['Usuario']) and $_SESSION['estado'] == 'Autenticado')                     { ?>
     <?php echo $_SESSION['Usuario'];?>

    Salir
 <?php }
    else {    ?>
        Entrar
        Registrarse
<?php   }    ?>

jQuery+PHP or full PHP?

Reasons not to substitute Javascript for PHP

If you use Javascript (including jQuery), then your site won't work if Javascript is disabled.

To use Javascript, you have to give unauthenticated users the same information as authenticated users and even admins.  Someone can save the HTML and edit it to show the full information.  This doesn't matter much with a menu, but with data...

Formatting

<?php
  session_start();
   if(isset($_SESSION['Usuario']) and $_SESSION['estado'] == 'Autenticado')                     { ?>
     <?php echo $_SESSION['Usuario'];?>

    Salir
 <?php }
    else {    ?>
        Entrar
        Registrarse
<?php   }    ?>

I find your use of beginning and ending tags confusing.  Consider

<?php
  session_start();
  if (isset($_SESSION['Usuario']) && $_SESSION['estado'] == 'Autenticado') {
?>
     <?php echo $_SESSION['Usuario']; ?>

    Salir
<?php
  }
  else {
?>
        Entrar
        Registrarse
<?php
  }
?>

By always putting tags at the left margin, it is easier to see where code blocks begin and end.  Only the embedded output has the tags inline as well.  This will look more familiar to most people who might edit your code later.


K&R 4-12 recursive converter int to string The exercise is:

  Adapt the ideas of printd to write a recursive version of itoa; that
  is, convert an integer into a string by calling a recursive routine.

I wonder if I have correctly answered the question.

# include <stdio.h>
# define MAX 100

int itoa (char s[], int n)
{
    static int i = 0;

    if (n < 0) {
        s[i++] = '-';
        n = -n;
    }
    if (n / 10)
        itoa (s, n / 10);

    if (i < MAX-1)
        s[i++] = n % 10 + '0';
    else {
        s[i] = '\0';
        return 0;
    }
    return 1;
}

int main ()
{
    int number = -123456789;
    char string[MAX] = { 0 };

    if (itoa(string, number))
        printf("The string is : %s\n", string);
    else
        printf("Error : array limit reached\n");
}

I presented it in a way it's not directly a converter but mostly. I'm a beginner so I don't know if I write some strange things. Could you please review my code?

No, this is not a correct solution.  Since i is static, the function will fail the second time it is used.

It also fails to handle INT_MIN correctly, since -INT_MIN will overflow an int.

You should never omit the "optional" braces for if, else, for, and while blocks.  By doing so, you are contributing to future coding accidents.  See this recent Apple security vulnerability and this quote from the author of jQuery:

  I really dis-liked having unnecessary braces. This… unfortunate… style preference plagued us for quite a while and caused all sorts of avoidable logic errors.


Functionally retrieving rows from a database in Scala Consider the following piece of code:

var result = List[Measurement]()

val dbResult = conn.createStatement.executeQuery(unsentMeasurements)

while(dbResult.next) {
  result ::= Measurement(dbResult.getString("stuff"), dbResult.getString("morestuff"))
}

result.reverse

Versus this:

val dbResult = conn.createStatement.executeQuery(unsentMeasurements)

Stream.continually(dbResult).takeWhile(_.next) map { r =>
  Measurement(r.getString("stuff"), r.getString("morestuff"))
}

Any flaws in my rationale? Any improvements?

Of course this is not purely functional, although it represents an improvement. What would a purely functional solution look like?

trait Sql[A] { self =>
  def unsafePerformIO(ds: javax.sql.DataSource): A = unsafePerformIO(ds.createConnection)

  def unsafePerformIO(conn: java.sql.Connection): A //abstract

  def map(f: A => B): Sql[B] = new Sql[B] {
    def unsafePerformIO(conn: Connection) = f(self.unsafePerformIO(conn))
  }
  def flatMap(f: A => Sql[B]): Sql[B] = new Sql[B] {
    def unsafePerformIO(conn: Connection)
      = f(self.unsafePerformIO(conn)).unsafePerformIO(conn)
  }

}

I imagine you might say "huh?" at this point. Let me introduce the companion:

object Sql {
  def apply[A](comp: Connection => A) = new Sql[A] {
    def unsafePerformIO(conn: Connection) = comp(conn)
  }
}

The idea (of course) is that you delay side-effects until the end of the world, encapsulating the values returned by the database in a type that represents value returned from DB call, in the same way Option represents value which may not exist.

The usage becomes something like this:

Sql { conn => process( conn.prepareStatement.executeQuery ) }

Where the process method might look like your code (above) perhaps.

I imagine this may raise a few questions:

What the? Huh? I mean, what the?

Well, it allows you to do something like this:

def getStuff1: Sql[Int]
def getStuff2: Sql[Int]
def useStuff(i: Int): Sql[String]

Then you can compose using for-comprehensions

val result =
  for {
    i <- getStuff1
    j <- getStuff2
    s <- useStuff(i + j)
  }
  yield s

What is the type of this expression? Well, it's an Sql[String] of course! To get hold of the String:

result.unsafePerformIO(myDataSource)

This creates a single connection which is then threaded through the computation.

What is missing? Well, resource management (i.e. cleaning up the connections of course). I understand this will all be available in scalaz7


Cracking the code interview: Problem 1 .6 basic string compression I created a GitHub repository containing my code solutions for some of the exercises proposed in the book. I updated my solution for the problem 1.6 here.

Description

  A function that compress the original string and store the compressed
  string in compress ex. aabcccccaaa will become a2blc5a3. If the
  compressed string is longer than the original it returns the
  original string.

Here's the code:

#include <stdio.h>
#include <string.h>

const char *compression(const char *original, char *compress) {
    size_t len, i, j, nc;

    /*memorize the first char of original in compress*/
    char previous = original[0];
    len = strlen(original);
    nc = 1;

    for (i = 1, j = 0; i < len; ++i) {
        if (original[i] == previous) {
            ++nc;
        } else {
            compress[j++] = previous;
            compress[j++] = nc + '0';
            nc = 1;
            previous = original[i];

        }

    }
    /* there are characters still in the stream */
    compress[j++] = previous;
    compress[j++] = nc + '0';
    compress[j] = '\0';

    if (strlen(compress) >= len) return original;
    return compress;
}

int main() {
    char buffer[20] = {0};
    printf("%s\n", compression("aabcccccaaa", buffer));
    return 0;
}

I'm looking for possible improvement of the code or better algorithm to solve the problem.

A call to strlen(compress) is unnecessary. You already know the length of the compressed string; it is j.
You may (in fact, you should) terminate the loop as soon as j reaches the length of the original. Once it happened, the compressed string will for sure be longer than original.
nc + '0' assumes that nc is a single digit. The problem statement doesn't say anything about that.
The size of buffer[20] is pretty arbitrary. Since the function provides guarantees about the size of the compressed string, consider allocating strlen(original) bytes for compressed.


Invocation list implementation (signals/slots) This class is mostly an educational exercise for me using some C++11 constructs. I wanted to create something similar to an "invocation list" in C#, i.e., a list of zero or more function objects which should be called when the invocation list as a whole is invoked. I'm certain there's room for improvement here and I'm hoping I didn't mess anything up TOO badly.

Some key implementation notes:

Variadic templates are used to specify the parameters passed to the target(s) of the invocation list.
The subscribe() function returns an "opaque" subscription object which can be used to unsubscribe() from the invocation list. This is done because std::function is not equality-comparable (so passing in a std::function to unsubscribe is problematic).
The subscription object that gets returned can be copied, and is designed so that instances of it are still well-behaved in a "detached" state when the original invocation_list instance has been destroyed; this is accomplished by sharing a pointer to the underlying target list using std::shared_ptr to do the reference counting on that pointer. The pointer is nulled when the invocation list is destroyed, detaching any associated lingering subscription instances.
No attempt has been or will be made to make this thread-safe.

#pragma once

#include <functional>
#include <list>
#include <memory>

template <class... Args>
class invocation_list
{
    private:

        typedef std::function<void(Args...)> func_type;
        typedef std::list<func_type> list_type;

    public:

        class subscription
        {
            public:

                void unsubscribe()
                {
                    if (_targets && *_targets)
                    {
                        (*_targets)->erase(_it);
                        _targets.reset();
                    }
                }

            private:

                subscription(
                        std::shared_ptr<list_type *> targets,
                        typename list_type::iterator it)
                    : _targets(targets), _it(it) { }

                std::shared_ptr<list_type *> _targets;
                typename list_type::iterator _it;

            friend class invocation_list;
        };

    public:

        invocation_list() : _token(new list_type *)
        {
            *_token = &_targets;
        }

        ~invocation_list()
        {
            *_token = nullptr;
        }

        invocation_list(const invocation_list&) = delete;
        invocation_list& operator=(const invocation_list&) = delete;

        subscription subscribe(func_type func)
        {
            _targets.push_back(func);
            return subscription(_token, --_targets.end());
        }

        void operator()(Args... args) const
        {
            for (auto it = _targets.begin(); it != _targets.end(); ++it)
                (*it)(args...);
        }

    private:

        list_type _targets;
        std::shared_ptr<list_type *> _token;
};

This would be used like:

invocation_list<int> foo;
auto a = foo.subscribe([](int x) { std::cout << "hello: " << x << std::endl; });
auto b = foo.subscribe([](int x) { std::cout << "world: " << x << std::endl; });
foo(42); // prints "hello: 42" "world: 42"
a.unsubscribe();
foo(54); // prints only "world: 54"

Future changes I've considered and am interested in feedback on:

Overload operators for subscribing and unsubscribing instead of an explicit subscribe() function?
Extending the subscription type to optionally operate in an RAII mode where the target is automatically unsubscribed when the subscription object goes out of scope?

I mentioned in the comments the following improvement to operator() using range-based loops:

for (auto target : _targets) target(args...);

I've also identified another potential issue: with the code as originally posted, the user must not destroy the invocation_list instance during invocation (i.e., inside a target function). This situation could easily arise if, for example, the invocation list is used to implement a "close" event, and inside the "close" event "handler" the user wants to delete the object that "triggered" the event.

To allow this, the subscribe() function must first make a copy of the std::list of targets before iterating over it. This allows the invocation list itself to be safely destroyed (per the "suicide" rules):

void operator()(Args... args) const
{
    list_type targets(_targets);
    for (auto target : targets) target(args...);
}

This reduces performance since the list of targets has to be copied for every invocation, but allows more robust usage. If anyone knows any alternatives that don't require copying the list but still allow this usage pattern, I'm all ears! Thanks!


Solution for swapping ArrayList data members with another Two possible solutions but which one? Or is there a Better?

I have a class MP3Track with members: Track number (int), track name (String), artist (String), in faourites (boolean) etc. My catalogue class contains two ArrayLists, one containing all tracks (vectorMain) and the other containing favourites (vectorFav) which are copies of the tracks in the main catalogue.

I want the user to have the option of swapping two track numbers (which are unique in both arrays).

Sample printout of the main catalogue below:

Both samples get the desired result but:

(Solution 1) deleted as didn't work in reverse.
(Solution 2) Below Seems rather cumbersome!
(Solution 3) Below Seems more elegant but very long winded!

getMainIndex() - returns the index position in the ArrayList for a given track number or -1.

Here is Solution 2 (overcame problem of solution 1 - but ugly)

Here is start of method:

public void swapTrack(){
    int t1 = -1, t2 = -1;
    System.out.println(face.getSwapTrackMenu());
    System.out.print("1) Please enter first track number to swap: ");
    t1 = scan.readInt();
    System.out.print("2) Please enter second track number to swap: ");
    t2 = scan.readInt();
    MP3Track mp31 = null;
    MP3Track mp32 = null;

Now solution 2

if((getMainIndex(t1)!=-1)&&(getMainIndex(t2)!=-1)){
    String s1 = null;
    String s2 = null;
    for(MP3Track track : vectorMain){
        if(track.getTrackNo() == t1){
            s1 = track.getTitle();
        }
        if(track.getTrackNo() == t2){
            s2 = track.getTitle();
        }
    }

    for(MP3Track track : vectorMain){
        if(track.getTitle().equals(s1)){
            track.setTrackNo(t2);
        }
        if(track.getTitle().equals(s2)){
            track.setTrackNo(t1);
        }
    }

    for(MP3Track track : vectorFav){
        if(track.getTitle().equals(s1)){
            track.setTrackNo(t2);
        }
        if(track.getTitle().equals(s2)){
            track.setTrackNo(t1);
        }
    }
}

Here is Solution 3

    //Loop thorugh main (ArrayList) looking for both track numbers
    for(MP3Track track : vectorMain){
        if(track.getTrackNo() == t1){
            //If track 1 found assign it to temp var
            mp31 = vectorMain.get(getMainIndex(t1));
        }
        if(track.getTrackNo() == t2){
            //If track 2 found assign it to temp var
            mp32 = vectorMain.get(getMainIndex(t2));
        }
    }

    //Check if we found track 1
    if(vectorMain.contains(mp31)){
        //Set temp var new track number
        mp31.setTrackNo(t2);
    }
    //Check if we found track 2
    if(vectorMain.contains(mp32)){
        //Set temp var new track number
        mp32.setTrackNo(t1);
    }

    mp31 = null;                //Remove pointer reference
    mp32 = null;                //Remove pointer reference

    //loop thorugh Fav (ArrayList) looking for track numbers
    for(MP3Track track : vectorFav){
        if(track.getTrackNo() == t1){
            //Assign it to temp var
            mp31 = vectorFav.get(getFavIndex(t1));
        }
        if(track.getTrackNo() == t2){
            //Assign it to temp var
            mp32 = vectorFav.get(getFavIndex(t2));
        }
    }
    //Check if we found it first
    if(vectorFav.contains(mp31)){
        mp31.setTrackNo(t2);
    }
    //Check if we found it first
    if(vectorFav.contains(mp32)){
        mp32.setTrackNo(t1);
    }

Is there something I'm missing here? I've hit a wall as to a better solution!!

I'm new to Java so any suggestions, improvements & comments appreciated.

Many thanks

In Solution 3, assuming you initialize mp31 and mp32 to null, you can replace this

//Check if we found track 1
if(vectorMain.contains(mp31)){
    //Set temp var new track number
    mp31.setTrackNo(t2);
}

With:

//Check if we found track 1
if(mp31 != null) {
    //Set temp var new track number
    mp31.setTrackNo(t2);
}

There is really no need to check if the arraylist contains the object since you used the get(...) method on the arraylist to get it in the first place. Therefore, you know that if the object has been initialized (is not null), the arraylist contains the object.

Also, I would change the method signature for getMainIndex to also take the list to search in as an argument to the method. In that case, you don't need to have two methods (with probably a lot of duplicated code) getMainIndex and getFavIndex can be the same method, getIndexInList(int trackNumber, List<MP3Track> list).

I am not sure how your getMainIndex method works at the moment, but I assume the code mp31 = vectorMain.get(getMainIndex(t1)); within the loop is the same thing as mp31 = track;. Therefore, you can replace this entire loop:

for(MP3Track track : vectorMain){
    if(track.getTrackNo() == t1){
        //If track 1 found assign it to temp var
        mp31 = vectorMain.get(getMainIndex(t1));
    }
    if(track.getTrackNo() == t2){
        //If track 2 found assign it to temp var
        mp32 = vectorMain.get(getMainIndex(t2));
    }
}

With either:

for(MP3Track track : vectorMain){
    if(track.getTrackNo() == t1){
        //If track 1 found assign it to temp var
        mp31 = track;
    }
    if(track.getTrackNo() == t2){
        //If track 2 found assign it to temp var
        mp32 = track;
    }
}

Or:

int index = getMainIndex(t1);
if (index != -1)
    mp31 = vectorMain.get(index);
index = getMainIndex(t2);
if (index != -1)
    mp32 = vectorMain.get(index);

As for Solution 2, this code here does not guarantee uniqueness of trackNo if two songs have the same title:

for(MP3Track track : vectorMain){
    if(track.getTrackNo() == t1){
        s1 = track.getTitle();
    }
    if(track.getTrackNo() == t2){
        s2 = track.getTitle();
    }
}

for(MP3Track track : vectorMain){
    if(track.getTitle().equals(s1)){
        track.setTrackNo(t2);
    }
    if(track.getTitle().equals(s2)){
        track.setTrackNo(t1);
    }
}

What you are essentially doing here is that you first scan through the list to get the title of the matching songs, and then you search for their title to change their track number. Why not change their track number within the first loop? You don't need all the three loops you have in Solution 2. Instead, search through vectorMain once and change the track numbers there, then search through vectorFav and change it there.

Some other comments:

I am not sure about how exactly you are using these two vectors and your MP3Track objects, or your trackNumber values. It seems to me as if whenever you swap track numbers, you do it in both of the lists. Therefore, perhaps you should use the same object references in both of the list. Don't create new MP3Track objects to place in vectorFav, you can re-use the same objects. That way, it will be enough to search only the vectorMain for the tracks, because it's the same object it will also update in vectorFav. As I said, I don't know exactly how you are using the trackNumber values so this might fit for you at the moment, but if it doesn't fit then you might want to change your design of the project.

As far as I have seen of the usage of getMainIndex and getFavIndex, you only use those methods to first get the index in the list, and then get the MP3Track object itself. Consider returning an MP3Track (the object itself) instead of an int (the index of the object in the list). Then you should also rename the method to reflect this change in behavior.


Python script to scrape titles of public Youtube playlist Just started in Python; wrote a script to get the names of all the titles in a public Youtube playlist given as input, but it got messier than it might have to be.

I looked around online and found HTMLParser, which I used to extract the titles, but it had problems with encoding which might have to do with there being foreign characters in the playlist HTML, so I messed around with encodes and decodes until it worked. But is there a prettier way to fix the problem?

import urllib.request
from html.parser import HTMLParser

playlistUrl = input("gib nem: ")

with urllib.request.urlopen(playlistUrl) as response:
  playlist = response.read()

html = playlist.decode("utf-8").encode('cp1252','replace').decode('cp1252')

titles = ""
class MyHTMLParser(HTMLParser):
  def handle_starttag(self, tag, attrs):
    for attr in attrs:
      if attr[0] == "data-title":
        global titles
        titles += attr[1] + "\n"

parser = MyHTMLParser()
parser.feed(html)
print(titles)

with open("playlistNames.txt", "w") as f:
  f.write(titles)

Well, how you handle the output of the titles can be improved. You don't need to fall back to using global variables here. They are very rarely really needed. Here it would be easier to make handle_starttag a generator, which is then consumed by str.join:

class MyHTMLParser(HTMLParser):

    def handle_starttag(self, tag, attrs):
        for attr in attrs:
            if attr[0] == "data-title":
                yield attr[1]

parser = MyHTMLParser()
titles = '\n'.join(parser.feed(html))
print(titles)

This assumes that HTMLParser.feed does not return any other values except from within the handle_starttag method (and that it actually returns the output of handle_starttag).

Note that I increased the number of spaces to 4 per indentation level, as recommended by Python's official style-guide, PEP8.

You also might want to add an early exit if the tag is not the correct tag.

If those assumptions above about feed are wrong, you might want to look for a different tool. Most parsing is done with BeautifulSoup, as far as I can tell. It offers strainers, with which you can reduce the amount of HTML to parse to only those tags you care about and CSS selectors which would let you directly select all of those tags with the right attribute.


Is it possible to make a re-usable icon component that doesn't create duplicate TemplateRefs I have attempted to create an IconComponent that shows a different SVG icon depending on the string that is passed in.
Example:
<app-icon icon="tick"></app-icon>

Would show an SVG of a tick.
Small StackBlitz example
I believe this approach, however, creates a new and unique TemplateRef for each instance of IconComponent, which would create useless duplicates of data in memory.
This problem concerns me when there may be thousands of icons on a page, such as on a table/grid.
Is there a better approach?
import {
  ChangeDetectorRef,
  Component,
  Input,
  OnChanges,
  SimpleChanges,
  TemplateRef,
  ViewChild
} from "@angular/core";

@Component({
  selector: "app-icon",
  templateUrl: "./icon.component.html",
  styleUrls: ["./icon.component.css"]
})
export class IconComponent implements OnChanges {
  @Input("icon")
  public icon?: string;

  @ViewChild("chevronTemplate", { static: true })
  public chevronTemplate?: TemplateRef<undefined>;

  @ViewChild("tickTemplate", { static: true })
  public tickTemplate?: TemplateRef<undefined>;

  public iconTemplate?: TemplateRef<undefined>;

  public constructor(private readonly changeDetectorRef: ChangeDetectorRef) {}

  public ngOnChanges(changes: SimpleChanges): void {
    let hasChanges = false;

    if (changes.icon !== undefined) {
      this.iconTemplate = this.getIconTemplate();

      hasChanges = true;
    }

    if (hasChanges) {
      this.changeDetectorRef.detectChanges();
    }
  }

  private getIconTemplate(): TemplateRef<undefined> {
    switch (this.icon) {
      case "chevron":
        return this.chevronTemplate;

      case "tick":
        return this.tickTemplate;

      default:
        throw new Error(`Unknown Icon Type '${this.icon}'.`);
    }
  }
}

Indeed, although working, this approach looks a bit heavy, because for each new SVG template the icon.component.html file will need to be changed and a respective TemplateRef field has to be created in the component.
It is not clear from the example, but if in your task all the pieces of SVG are static (not generated through the app logic), it can be solved differently. Here are some hints:

for each SVG template, create a file under the application resources: chevron.svg, tick.svg etc, with the respective contents.

in icon.component.ts, add a getter returning the path to the SVG file, for example:

public get svgResourcePath(): string {
  return `/path/to/resource/${this.icon}.svg`;
}

change the icon.component.html, so that it uses a standard HTML image tag, with the value of the src attribute bound to the path:

<img [src]="svgResourcePath"/>

Update

One point I didn't put in my question is that I also need to bind a color to the SVG through an @Input().

As I've mentioned, it was not clear from the initial question. But still there is a solution for that, although it looks a bit longer than my previous suggestion.
I've played a bit and implemented it here.
The ideas behind the approach:

Since SVG code can be included directly in a component template, we can create separate component definitions for all necessary cases, e.g. for tick:

@Component({
  selector: "tick-icon", // does not matter, because we do not intend to insert it through a template
  template: `
    <svg>
      <path
        [attr.fill]="fillColor"
        d="M23.9 3.7L9.7 17.9l-6.8-6.8-2.8 2.6 9.7 9.6L26.7 6.5z"
      />
    </svg>
  `
})
export class TickComponent implements StyleConfigurable {
  private _fillColor: string;

  @Input()
  public set fillColor(fc: string) {
    this._fillColor = fc;
  }

  public get fillColor(): string {
    return this._fillColor;
  }
}

The StyleConfigurable interface here is an interface that defines the SVG styling properties that we want to customize:
export interface StyleConfigurable {
  fillColor: string;
}

Angular components can be instantiated dynamically, so we adjust the algo from the doc for our use case:

export class IconComponent implements OnInit {
  private _icon: string;

  private _fillColor: string;

  // inject the necessary objects for the dynamic instantiation:
  public constructor(
    private _resolver: ComponentFactoryResolver,
    private _containerRef: ViewContainerRef
  ) {}

  public ngOnInit(): void {
    // trigger the SVG icons loading when this component is initialized:
    this.loadIcon();
  }

  // dynamically load the icon:
  private loadIcon(): void {
    const cmpFactory = this._resolver.resolveComponentFactory(this.getTypeForIcon());
    const cmpRef = this._containerRef.createComponent<StyleConfigurable>(
      cmpFactory
    );
    // note this call: instance.fillColor is available through StyleConfigurable
    cmpRef.instance.fillColor = this._fillColor;
  }

  // for each supported value of the 'icon' property, return the corresponding component type:
  private getTypeForIcon(): Type<StyleConfigurable> {
    switch (this._icon) {
      case "tick":
        return TickComponent;
      case "chevron":
        return ChevronComponent;
      default:
        throw Error(`Unrecognized icon: ${this._icon}`);
    }
  }
}

Now the IconComponent can be used as follows:
<app-icon icon="chevron" fillColor="rgb(0, 255, 0)"></app-icon>
<app-icon icon="tick" fillColor="rgb(255, 0, 0)"></app-icon>


Interactive Nim Game This my first java program. I wrote this as a Nim Game so that I could learn java while writing it. I would love to see how could this be made better.
Sample Game:

Code:
import java.util.Scanner;

public class Nim {

  public static boolean isArrayZero(int[] arr){
    for(int i: arr){
      if(i > 0)
      return false;
    }
    return true;
  }

  public static int getPile(char pile){
    switch (pile) {
      case 'A': return 0;
      case 'B': return 1;
      case 'C': return 2;
      default:  return -1;

    }
  }
  public static String playerTurn(String pl1, String pl2, int nbOfTurns){
    if(nbOfTurns % 2 == 0)
       return pl1;
       return pl2;
  }
  public static void main(String[] args) {
    Scanner input = new Scanner(System.in);
    String player1, player2, player;
    int[] pile = {3,5,7};
    int pileNb, howMany = 0, chosenPile = 0, nbOfTurns = 0;
    char pileName;
    //Get names of players
    System.out.print("Player1 name: ");
    player1 = input.next();

    System.out.print("Player2 name: ");
    player2 = input.next();

    do {
      System.out.println(String.format("\nA:%s\tB:%s\tC: %s", pile[0], pile[1], pile[2]) );
      player = playerTurn(player1, player2, nbOfTurns);
      do{
          System.out.print(String.format("\n%s,Choose a pile: ", player));
          pileName = Character.toUpperCase(input.next().charAt(0));
          pileNb = getPile(pileName);
          if(pileNb == -1)
            continue;
          chosenPile = pile[pileNb];
          if(chosenPile == 0)
            continue;
          System.out.println(String.format("How many to remove from pile %s", pileName));
          howMany = input.nextInt();
      }while((howMany > chosenPile) || (howMany <= 0));
      pile[pileNb] = chosenPile - howMany;
      nbOfTurns += 1;
    } while (!isArrayZero(pile));
    System.out.println(String.format("%s, You loose", player) );
  }
}

Checking arrays

Since you are in Java 8, you can use IntStream to help you out:

public static boolean isArrayZero(int[] arr){
    return Arrays.stream(arr).allMatch(i -> i <= 0);
}

What that means is to ensure all matches the IntPredicate (lambda) i <= 0.

Input validation

Your input validation can probably be better done as a method:

private static int getPile(Scanner scanner) {
    int result = -1;
    while (result = -1) {
        result = getPile(Character.toUpperCase(input.next().charAt(0)));
    }
    return result;
}

And the same goes for asking the number to remove from the pile. These can be done to eliminate the need to continue from the loop.

Modeling

Using an int[] array is perfectly fine, but you may also want to explore other forms of modeling, such as using an enum:

enum Pile {
    A, B, C;

    public static Pile getPile(char input) {
        try {
            return valueOf(String.valueOf(input));
        } catch (IllegalArgumentException e) {
            return null;
        }
    }
}

Instead of dealing with array indices, you then deal with enum values, which depending on your perspective may look a little better.

try-with-resources

The usage of try-with-resources on the Scanner instance is recommended to manage its underlying I/O resource cleanly.

Potential bug

The screenshot you currently have does not agree with your code, since it looks like Mark managed to remove 8 from 7 in pile 'C'. The same prompts confused me here... Maybe use a prompt that indicates an invalid input was encountered will be a bit better.


Best way to manage a device information class This is probably a really simple question.

An embedded device I'm working with will store the device information in an xml file.

In my code, I want to read this xml file and freely reference to it wherever I need to, so I made a simple helper class:

public class DeviceInformation {
    public static final String XML_DIRECTORY_ON_DEVICE = "/xml/";
    public static final String ROOT_DEVICE_XML_FILE = "rootdevice.xml";

    public static String DEVICE_ID;
    public static String DEVICE_TYPE;
    public static String HOME_ID;

    public DeviceInformation() {
        File xmlFile = new File(XML_DIRECTORY_ON_DEVICE + ROOT_DEVICE_XML_FILE);
        if (!xmlFile.exists())
            return;
        RootDeviceXMLReader rootDeviceXMLReader = new RootDeviceXMLReader(xmlFile);
        DeviceInfoStruct deviceInfoStruct = rootDeviceXMLReader.getDeviceInfoStruct();
        DEVICE_ID = deviceInfoStruct.getDeviceID();
        DEVICE_TYPE = deviceInfoStruct.getDeviceType();
        HOME_ID = deviceInfoStruct.getHomeID();
    }
}

And then I'd just get my device id by DeviceInformation.DEVICE_ID. Same goes for type and home_id.

But this approach just doesn't seem elegant. Turn the initializer into a static one? Still, I feel like there's something wrong with this whole approach and I just cannot articulate it. How can I improve this class?

I think you should have :

A constructor taking device_id, device_type and home as an argument.
a static method doing the file handling and calling the constructor previously defined.

This would make your code more modular, easier to test and would put different independent things (device creation and file handling) in different places.

I'd also like to point out that at the moment, your constructor does create a non-properly-initialised device when the file does not exist. It would probably be much better not to create it at all (and return null ?) if we don't have the required information to do so.

Also, and this is not relevant to your question but there's not real need for 2 variables XML_DIRECTORY_ON_DEVICE and ROOT_DEVICE_XML_FILE if you are always going to call them together. It might be just as easy to have "XML_PATH".


Smallest number in BST which is greater than or equal to N Given a Binary Search Tree and a number N, the task is to find the
  smallest number in the binary search tree that is greater than or
  equal to N. Print the value of the element if it exists otherwise
  print -1.

Please review the code, the unit tests are just for the demo. please comment about performance thanks.

using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace TreeQuestions
{
    /// <summary>
    /// https://www.geeksforgeeks.org/smallest-number-in-bst-which-is-greater-than-or-equal-to-n/
    /// </summary>
    [TestClass]
    public class SamllestNumberBstGreatherThanOrEqualToN
    {
        [TestMethod]
        public void SmallestNumberBstGreatherThanOrEqualToNTest()
        {
            BstNode root = new BstNode(19 );
            BstNode left = new BstNode(7);
            BstNode right = new BstNode(21);
            root.Left = left;
            root.Right = right;
            BstNode leftLeft = new BstNode(3);
            BstNode leftRight = new BstNode(11);
            BstNode leftRightLeft = new BstNode(9);
            BstNode leftRightRight = new BstNode(14);
            root.Left.Left = leftLeft;
            root.Left.Right = leftRight;
            root.Left.Right.Left = leftRightLeft;
            root.Left.Right.Right = leftRightRight;
            int result = SmallestNumberGratherThanOrEqual(root, 20);
            Assert.AreEqual(21, result);
        }

        [TestMethod]
        public void SmallestNumberBstGreatherThanOrEqualToNTest2()
        {
            /*    19
        /    \
       7     21
     /   \
    3     11
         /   \
         9    14
          */

            BstNode root = new BstNode(19);
            root = root.Add(root, 7);
            root = root.Add(root, 3);
            root = root.Add(root, 11);
            root = root.Add(root, 9);
            root = root.Add(root, 13);
            root = root.Add(root, 21);
            int result = SmallestNumberGratherThanOrEqual(root, 18);
            Assert.AreEqual(19, result);
        }

        private int SmallestNumberGratherThanOrEqual(BstNode root, int num)
        {
            int smallest = -1;
            while (root != null)
            {
                if (root.Value >= num)
                {
                    smallest = root.Value;
                    root = root.Left;
                }
                else
                {
                    root = root.Right;
                }
            }
            return smallest;
        }
    }
}

here is also the BST class

public class BstNode
{
    public int Value;
    public BstNode Left;
    public BstNode Right;
    public BstNode Parent;

    public BstNode(int keyt)
    {
        Value = keyt;
    }

    public BstNode Add(BstNode node , int key)
    {
        if (node == null)
        {
            return new BstNode(key);
        }
        if (node.Value < key)
        {
            node.Right = Add(node.Right, key);
        }
        else
        {
            node.Left = Add(node.Left, key);
        }
        return node;
    }

}

SmallestNumberGratherThanOrEqual

This looks mostly fine to me. I can't think how else to implement it, since you can't really separate the 'finding smallest' bit from the 'finding where num would go' bit.

The method should probably be public and static.
The spec does say a number N... but oh well. root is fine as the parameter name, but it quickly becomes misleading, as it ceases to be the root node pretty quickly.
It's iterative, which makes me happy.

Since I have so little to say about the code in question...

BstNode.Add(BstNode, int)

Why does Add take a BstNode as a parameter?!?!?!?1 Either it should be an instance method and operate on itself, or it should be static and operate on the parameter. The problem here is both conceptual and technical, as it makes no sense to ask a BstNode to operate on another BstNode, and it means that both nodes are in scope, so you might use the wrong one by mistake. I often have an instance method call a private static method simply to avoid this (e.g. when calling something iterative which will 'change' the parameter as it goes).
This is odd:

if (node == null)
{
    return new BstNode(key);
}

This bit doesn't actually add anything to anything - despite being in a method called Add - and is liable to cover-over a bug elsewhere which passes this method a null. Even worse, a null goes in, and something strange but not null comes out, which means the bug is likely to go even further now that there is some actual state going around.

I can see why it's there (it's part of the recursive method), but this sort of thing should not be public. Better would be to make this method private (and perhaps static and iterative as discussed above), and call it from another Add method which throws on null. This way, the specifics of your implementation of Add will not leak into the public API, and you can instead provide helpful error messages. You could also remove the return value, which is meaningless and just adds more confusion for the consumer.
Channelling t3chb0t for a moment... he might say that finding the base node and adding to it are different concerns, and that you should write one private method to find the node to add to, and call it from another which then does the adding. Such a method could not, however, be directly exploited to find the solution to the problem unless it was implemented in such a way that is basically did solve the problem, which is a bit of a shame.
As always, inline documentation (///) would be nice on this public method.

other BstNode stuff

keyt is a bit of an odd name.
Parent is never set. This is clearly an error and would make me as a consumer very sad if I'd designed my code around the assumption that I can access the parent of a BstNode and then discovered that I can not.
All the members are public fields: perhaps this is an attempt to avoid the overhead of getters? If so, don't worry about that cost: they are usually optimized away completely for non-virtual properties (at least, whenever I look for them they don't seem to be there).

I'd have Left and Right as public { get; private set; }. Parent and Value can be readonly if the tree if nodes are never removed from the tree. Indeed, for an 'add-only' tree, I'd be inclined to have the setters on Left and Right check that they are not overwriting anything. This would break the first test method, but frankly it's horrifying and ought to be rewritten.
Passing BstNode around is fine if everything knows that it is a tree, but really you should have a BinarySearchTree class which encapsulates these nodes, as it is meaningless and dangerous to operate on them independently (e.g. if you call Add on a child node you may end up with a mal-formed tree): nobody should have write-access to BstNode except BinaryNodeTree. Doing so would require exposing methods to traverse the tree without allowing modifications, so that you can still write the specified search outside of the BinarySearchTree class. This also gives a nice solution to the root parameter name problem.

Generics

If it weren't for the really horrible spec (-1 is a perfectly valid number, it should not be used as output here) I'd suggest this should be generic. Performance might suffer (depening on how you facilitate comparisons), but a generic solution is always nice: you can write a concrete version for int if the performance isn't good enough later.

Performance

I don't know anything about binary trees, but this is a very simple non-self-balancing implementation, and has a worst-case lookup time of O(n). A self-balancing tree (such as a Red-Black Tree (there are others) can guarantee logarithmic lookup time, at the cost of a more complicated data-structure and higher and the overheads which come with it. (This doesn't, however, change how you would implemented the specification, as it assumes a binary tree and doesn't ask you to modify it).

Boring stuff

I'd appreciate a few more empty lines to break the code up a bit, e.g. before the while, between all those ifs in Add, and before the returns.


Upload a file asynchronously I need your opinion about my practice. I want to improve it.

The requirements are the following:

RESTFUL + OWIN (SelfHosted)
Endpoint - Upload a file asynchronously
Make a request to the endpoint from Postman

Well, This is what I got

For the OWIN selfhosted I followed the Microsoft Documentation

OWIN Self-host

Endpoint:

FileController.cs

    [HttpPost]
    [Route("api/v1/upload")]
    public HttpResponseMessage Post([FromUri]string filename)
    {
        var task = this.Request.Content.ReadAsStreamAsync();
        task.Wait();
        Stream requestStream = task.Result;

        try
        {
            Stream fileStream = File.Create("./" + filename);
            requestStream.CopyTo(fileStream);
            fileStream.Close();
            requestStream.Close();
        }
        catch (IOException)
        {
            throw new HttpResponseException( HttpStatusCode.InternalServerError);
        }

        HttpResponseMessage response = new HttpResponseMessage();
        response.StatusCode = HttpStatusCode.Created;
        return response;
    }

Postman:

The postman returns Status Code 201

Seems a bit needlessly complicated and not exactly async at all. You're just forcing an async call to be synchronous with .Wait(). Proper way is to use async "all the way down":

[HttpPost]
[Route("api/v1/upload")]
public async Task<HttpResponseMessage> Post([FromUri]string filename)
{
    try
    {
        using (Stream requestStream = await this.Request.Content.ReadAsStreamAsync())
        using (Stream fileStream = File.Create("./" + filename))
        {
            await requestStream.CopyToAsync(fileStream);
        }

        return new HttpResponseMessage { StatusCode = HttpStatusCode.Created };
    }
    catch (IOException)
    {
        throw new HttpResponseException(HttpStatusCode.InternalServerError);
    }
}


Printing all binary strings of length n I have completed my homework with directions as follows:

  Declare and implement a class named Binary. This class will have a
  method named printB(int n) that prints all binary strings of length n.
  For n = 3, it will print

000
001
010
011
100
101
110
111

  in this order.

Here is my code:

import java.util.Scanner;
class Binary
{

    String B;
    int temp;

    void printB(int n)
    {
        for(int i = 0; i < Math.pow(2,n); i++)
        {
            B = "";
            int temp = i;
            for (int j = 0; j < n; j++)
            {
                if (temp%2 == 1)
                    B = '1'+B;
                else
                    B = '0'+B;
                    temp = temp/2;
            }
            System.out.println(B);
         }
    }
}

class Runner
{
    public static void main(String [] args)
    {
        Scanner in = new Scanner(System.in);

        System.out.print("Enter n:");
        int n = in.nextInt();

        Binary myB = new Binary();

        myB.printB(n);
    }
}

My question is...is there anyway to make this shorter or more efficient?

Speaking from a straight Java perspective, one thing you can do is change your loop header. You evaluate the power every iteration and could do something like:

for (int i = 0, end = Math.pow(2, n); i < end; i++)

You also use standard String concatenation which is slow because once compiled, when you concatenate two strings a StringBuffer is created, and both strings are added to it, and then resulting string is returned. You save a step by using a StringBuffer from the get go which would you replace B = ""; with B = new StringBuffer(); (of course changing it's type accordingly). Instead of adding strings to you call insert (which can be chained) like B.insert(0, "1")

You're appending a char ('0' or '1') which you evaluate with modular division, you can just prepend it like B.insert(0, (temp % 2)) and save that entire if.

I probably said more than I should have for homework, but you appear to have already solved your problem and so I was just giving advice in addition to that.

EDIT

My solution would be as follows:

import java.util.Scanner;

public class Main {
    public static void main(String[] args) {
        Scanner in = new Scanner(System.in);
        System.out.print("Enter number >> ");
        int value = in.nextInt();
        in.close();
        Binary binary = new Binary(value);
        System.out.println(binary.getResult());
    }
}

class Binary {
    private int value = 0;
    private StringBuilder binaryValues;

    public Binary(int value) {
        this.value = value;
        this.binaryValues = new StringBuilder();
        this.process();
    }

    private void process() {
        for (int i = 0, end = (1 << this.value); i < end; i++) {
            StringBuilder binary = new StringBuilder(Integer.toString(i, 2));
            while (binary.length() < this.value)
                binary.insert(0, 0);
            this.binaryValues.append(binary).append("\n");
        }
    }

    public String getResult() {
        return this.binaryValues.toString();
    }
}

Maybe not shorter but no need to rewrite binary number generation when it's built in.