SQL Query efficiency within class I'm trying to figure out whether I should do the following:

This class (over-simplified version) retrieves  information about an order by having 1 main query function that select a specific column from the table and throws out the value for that column and that specific order ID.

class jobinfo
{

    protected $id;

    function __construct($job_id)
    {
        $this->id-$job_id;

    }

    protected function query_s($table, $col = '*', $where_col , $where, $return_col)
    {
        $SQL = mysql_query("SELECT $col FROM ".$table." WHERE $where_col='$where' LIMIT 1") or die(mysql_error());
        $data = mysql_fetch_array($SQL);
        return $data[$return_col];
    }

    public function qtyOrdered()
    {
        return $this->query_s('job_sheet','qty', 'job_id',$this->id, 1, 'qty');
    }

    public function dueDate()
    {
        return $this->query_s('job_sheet','due', 'job_id',$this->id, 1, 'due');
    }

}

The way I have it set up now is that I have this running in a while loop over 10,000 different orders like so:

$sql = mysql_query("SELECT job_id FROM job_sheet");
while($data = mysql_fetch_array($sql))
{
    $job_data = new jobinfo($data['job_id']);

    //table goes here to output values
    echo $job_data->dueDate();
    echo $job_data->qtyOrdered();
    ///
}

The one issue with this is that for every row that comes up, there are 2 queries that are happening in the back: one for the quantity and one for the due date. This ultimately causes to slow things down, especially when I'm outputting almost 11 columns for 11 different things. It adds up to a lot of queries per row.

My question is, should I go with something like this:

class jobinfo
{

    protected $id;
    protected $due;
    protected $qty;

    function __construct($job_id)
    {
        $this->id-$job_id;

        $this->retrieve();

    }

    protected function retrieve()
    {
        $sql = mysql_query("SELECT qty, due FROM job_sheet WHERE job_id=".$this->id." LIMIT 1");
        $data = mysql_fetch_array($sql);
        $this->due = $data['due'];
        $this->qty = $data['qty'];

        /// etc etc etc

    }

    public function qtyOrdered()
    {
        return $this->qty;
    }

    public function dueDate()
    {
        return $this->due;
    }
}

Which would do only one query per row in this case.

My dilemma is that the only time I find the second class more efficient is when some of this information is called via Ajax on a refresh rate of 5 seconds to be displayed somewhere. There are 15 users and most of the stay on one page that refreshes data every 5 seconds for statistical purposes.The other time is when the user searches for something in a custom search engine. The rest of the site is primarily showing only a few things and don't need to search the entire database to retrieve one specific value like the second class would have been doing. And so, this is the reason why I've set up my class as the first one: 'retrieve as you go' basically:

$job = new jobinfo(123); echo $job->qty();

I'm not sure what to do at this point and need some advice/suggestions.

Doing this in a loop will be slow, and doubling the calls by retrieving each column with a different query will make things worse.

Why not use a JOIN between your original query, something like this:-

SELECT database.job_id, job_sheet.qty, job_sheet.row
FROM database
INNER JOIN job_sheet
ON database.job_id = job_sheet.job_id

In your loop this would mean a single query rather than ~20000 queries.

If you do need to do a separate query for each time through the loop then a prepared statement would help but peformance will likely still be poor.


How to improve my PHP adding script? I have a script, which updates my table's column and writes an id to it.

I need to check whether the column is empty or not.  If it is not, I add a ,.

$subs = mysql_fetch_array(mysql_query("SELECT subscribed_user_id FROM users WHERE user_id=".(int)$_GET['user']));

$subs_array = array();

$subs_array=explode(',', $subs['subscribed_user_id']);

if(!in_array($_COOKIE['user_id'], $subs_array))
{
    if($subs['subscribed_user_id']=='')
    {
        $add='';
    } else {
        $add = $subs['subscribed_user_id'].',';
    }
    mysql_query("UPDATE users SET subers=subers+1, subscribed_user_id='".$add.$_COOKIE['user_id']."' WHERE user_id=".(int)$_GET['user']);
}

I have an idea: always add ,.  But when I select it, I don't use the full length of the array.  But, for example array.length-2, I think that it is not OK and I need to know how I can improve this script.

With regard to your question about adding ,, just do

$new_user_id = $_COOKIE['user_id'];

if ($subs['subscribed_user_id'] === '') {
  $new_user_id = $subs['subscribed_user_id'] . ',' . $new_user_id;
}

With that said, the details of how you add the comma is not the issue here.

I began rewriting the code, but since the db interactions make the code hard to test and work with, I honestly couldn't be bothered to properly finish it.

You should

Wrap the functionality in a actual function taking parameters to remove use of $_ globals.
Stop using the ancient and deprecated mysql extension. In fact, don't use (the much better) mysqli either, at the very least, adopt PDO, or better yet, a tool that removes the low level details of managing the DB.
Fix the injection vulnerability in your update query.
Consider whether you really should be comma separating values in the column. Perhaps you should instead add a new row?

Some code I started writing, but didn't finish because I have better things to do than setting up the db I would need.

function update_user_id($user_id, $subs, $cookie) {
  $subs = mysql_fetch_array(mysql_query("SELECT subscribed_user_id FROM users WHERE user_id = " . (int) $user_id));

  $subs_array = explode(',', $subs['subscribed_user_id']);

  if (!in_array($cookie['user_id'], $subs_array)) {
    $new_user_id = $cookie['user_id'];

    if ($subs['subscribed_user_id'] === '') {
      $new_user_id = $subs['subscribed_user_id'] . ',' . $new_user_id;
    }

    mysql_query("UPDATE users SET subers = subers + 1, subscribed_user_id = '" . $new_user_id . "' WHERE user_id = ". (int) $user_id);
  }
}


VBA code that pulls information from 1000+ files in directory performance Below is the code I am using to run through files in a folder, pull information from the files, and record that information on a sheet. I am not the original author of the main portion of the code. The files the code is opening are fairly large and there are 1000+ of them.  I also hard coded the cells the code is pulling information from.

Is there any way to make this code run more efficiently?

Sub LoopAllExcelFilesInFolder()

Dim wb As Workbook
Dim myPath As String
Dim myFile As String
Dim myExtension As String
Dim PartInfoArr() As Long
Dim ArrCount As Integer
Dim PartNum As Integer
Dim i As Integer
Dim DocDate As String
Dim j, r As Integer
Dim SeenPart, SeenCount As Integer

'Optimize Macro Speed
  Application.ScreenUpdating = False
  Application.EnableEvents = False
  Application.Calculation = xlCalculationManual
ArrCount = 0

'folder path for where to pull files from
  myPath = "C:\Users\.........."

'Target File Extension (must include wildcard "*")
  myExtension = "*.xlsm*"

'Target Path with Ending Extention
  myFile = Dir(myPath & myExtension)

'Loop through each Excel file in folder
  Do While myFile <> ""
    'Set variable equal to opened workbook
      Set wb = Workbooks.Open(Filename:=myPath & myFile)

    'Ensure Workbook has opened before moving on to next line of code
      DoEvents

'Coppy all file information
Sheets("Tool GT Block").Select
Range("B11:C11").Select

'While there are part numbers record all info for each into arry
While ActiveCell.Offset(i, 0) <> ""
    Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2").Offset(k, 0) = ActiveCell.Offset(i, 0)
    DocDate = Range("D8")
    Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2").Offset(k, 1) = DocDate
    Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2").Offset(k, 2) = Range("D11")
    Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2").Offset(k, 3) = Range("I9")
    Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2").Offset(k, 4) = Range("I10")
    Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2").Offset(k, 5) = Range("I12")
    Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2").Offset(k, 6) = Range("I13")
    Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2").Offset(k, 8) = Range("I15")
    Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2").Offset(k, 9) = Range("I16")
    Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2").Offset(k, 10) = Range("I17")
    Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2").Offset(k, 11) = Range("I18")
    Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2").Offset(k, 12) = Range("I21")
    'min packing processing overall
    Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2").Offset(k, 13) = Range("R10")

i = i + 1
k = k + 1
Wend

' reset  var for next file
i = 0

    'Save and Close Workbook
      wb.Close SaveChanges:=False

    'Ensure Workbook has closed before moving on to next line of code
      DoEvents

    'Get next file name
      myFile = Dir
  Loop

ResetSettings:
  'Reset Macro Optimization Settings
    Application.EnableEvents = True
    Application.Calculation = xlCalculationAutomatic
    Application.ScreenUpdating = True

'Message Box when tasks are completed
  MsgBox "Task Complete!"

End Sub

Preamble

The first step to optimizing this code is most likely not something you'll think helpful. It's generally accepted wisdom among the regular VBA reviewers here that writing code should follow a three step-process:

Make it work
Make it right
Make it fast

The problem with the code you presented is that it's really long and hard to understand (and accordingly optimize). Basically you're jumping the gun and try to run before you walked.

Constants

Constants are values that can be known at compile time and that do not change when the macro is run. You should extract such values into module-level constants like so:

Private Const myPath As String = "C:\Users\...."
Private Const myExtension As String = "*.xlsm"
' ... ?

Variable definitions

It's great that you're declaring your variables. I didn't see an Option Explicit though. It's generally cleaner and easier to require variable declarations to prevent bugs from misspellings and other quirks. Use any security that the language can give you.

In addition to that there's a small weirdness about VBA and As-Declarations:

Dim j, r As Integer
Dim SeenPart, SeenCount As Integer

This code actually will make the types of r and SeenCount Integer, but j and SeenPart will be declared and used as Variant, which can slow your program down.

To prevent this behaviour from ruining your day, I'd recommend declaring each variable on a separate line.

Last but not least: Integer is a 16-bit signed integer. Modern CPUs are highly optimized for arithmetics on 32-bit signed (and unsigned) integers. Surprisingly often it's faster in VBA to use Long over Integer.

Submethods

To make code easier to follow, it helps to keep the lifetime of variables and constructs as small as possible. This reduces the load on the reader's short-term memory and keeps capacity free to think about what the code actually means instead of just being preoccupied with which variables are actually relevant here.

Overall you should strive to make procedures as short as possible by extracting common code into a new procedure.

Consider:

Sub LoopAllExcelFilesInFolder()
    On Error GoTo ResetSettings
    OptimizeMacroSpeed()

    AggregateData()

ResetSettings:
    ReinstituteApplicationState()
    On Error GoTo 0
End Sub

This is very easy to understand at the first reading. Of course it doesn't contain all information that you need, but it helps you understand the overall goal of the code. Now AggregateData could read something like this:

Sub AggregateData()
    Dim myFile As String
    myFile = Dir(myPath & myExtension)

    Do While myFile <> ""
        Dim workbook As Workbook
        Set workbook = Workbooks.Open(Filename := myPath & myFile)
        ' Ensure workbook has opened
        DoEvents
        CopyData(workbook)

        workbook.Close SaveChanges := False
        ' Ensure workbook has closed
        DoEvents
        myFile = Dir
    Loop
End Sub

At this point it should be fairly obvious what CopyData should be doing, so I won't elaborate further :)

Finally Optimizations

First things first: Accessing a Worksheet is slow. Accessing a Workbook also is. What you accordingly want to do is minimize the number of times you do so.

Since VBA is an interpreted language (after having been compiled to p-code), every instruction is executed independently. This means that the copy-process is terrifyingly slow:

Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2")

^^ this code is executed every single line, which means a tremendous overhead.

Luckily VBA has a builtin mechanism to help us with this:

With Workbooks("MTM File.xlsm").Worksheets("MTM Pack Process Data").Range("A2")
    .Offset(k, 0) = ActiveCell.Offset(i, 0)
    .Offset(k, 1) = Range("D8")
    .Offset(k, 2) = Range("D11")
    ' ...
End With

Instead of executing the whole chain of Workbooks(..).Worksheets(..).Range("A2") every line, now VBA executes the chain only once and keeps a reference of that object in the with-block. This object can now be accessed directly and most of the overhead is removed.

A word of caution

The word of caution also is a possible optimization... Working with Selection and ActiveWorkbook and ActiveWorksheet is downright dangerous. In the middle of macro execution, your user could click on things, change the reference and suddenly data is not where you expect it anymore and everything breaks.

To prevent that you should always (read: whenever possible) keep a reference to a Range or Worksheet you're working with in a variable. That is pretty safe from the user and also allows for some minor optimizations.


Traversing a multidimensional structure and applying strip() to all strings I'm trying to apply string.strip() to all the leafs that are strings in a multidimensional collection, but my Python is a bit rusty (to say the least). The following is the best I've come up with, but I suspect there's a much better way to do it.

def strip_spaces( item ):
    if hasattr( item, "__iter__" ):
        if isinstance( item, list ):
            return [strip_spaces( value ) for value in item]
        elif isinstance( item, dict ):
            return dict([(value,strip_spaces(value)) for value in item])
        elif isinstance( item, tuple ):
            return tuple([ strip_spaces( value ) for value in item ])
    elif isinstance( item, str ) or isinstance( item, unicode ):
        item = item.strip()
    return item

I don't understand why you are checking for an __iter__ attribute, as you don't seem to use it. However I would recommend a couple of changes:

Use Abstract Base Classes in the collections module to test duck types, such as "Iterable"
Use types.StringTypes to detect string types

.

import collections
import types

def strip_spaces( item ):
    if isinstance( item, types.StringTypes ):
        return item.strip()

    if isinstance( item, collections.Iterable ):
        if isinstance( item, list ):
            return [ strip_spaces( value ) for value in item ]

        elif isinstance( item, dict ):
            return dict([ ((strip_spaces(key), strip_spaces(value)) \
                                    for key, value in item.iteritems() ])

        elif isinstance( item, tuple ):
            return tuple( [ strip_spaces( value ) for value in item ] )

    return item


Deviating from the norm Back on track with my C++ saga:

  The standard deviation of a list of numbers is a measure of how much
  the numbers deviate from the average. If the standard deviation is
  small, the numbers are clustered close to the average.  If the
  standard deviation is large, the numbers are scattered far from the
  average.  The standard deviation, \$ S \$, of a list of \$ N \$
  numbers \$ x \$ is defined as follows

  $$ S = \sqrt{\dfrac{\sum\limits_{i=1}^{N} \left( x_i - \bar{x} \right)^{2}}{N}} $$

  Where \$\bar{x} \$ is the average of the \$ N \$ numbers \$ x1, x2, .... \$ Define a function that takes a partially filled array of
  numbers as its arguments and returns the standard deviation of the
  numbers in the partially filled array.  Since a partially filled array
  requires two arguments, the function will actually have two formal
  parameters: an array parameter and a formal parameter of type int
  that gives the number of array positions used.  The numbers in the
  array will be of type double Embed your function in a suitable test
  program.

stddev.cpp:

/**
 * @file stddev.cpp
 * @brief
 * @author syb0rg
 * @date 11/6/14
 */

#include <iostream>
#include <fstream>
#include <cmath>

constexpr int NUM_ELEM = 28;

double mean(double arr[])
{
    double sum = 0;
    for (int i = 0; i < NUM_ELEM; ++i)
    {
        sum += arr[i];
    }
    return (sum / NUM_ELEM);
}

double stdDev(double arr[])
{
    double avg = mean(arr);
    double sum = 0;
    for (int i = 0; i < NUM_ELEM; ++i)
    {
        arr[i] = arr[i] - avg;
        sum += std::pow(arr[i], 2);
    }
    return std::sqrt(sum / NUM_ELEM);
}

int main()
{
    std::ifstream file("test.txt", std::ios_base::in);
    double input[NUM_ELEM] = {0.0};

    for(int i = 0; (file >> input[i]) && file.good(); ++i);
    std::cout << stdDev(input) << std::endl;
    file.close();
}

As usual, a few points to complete what has already been said:

An std::ifstream is implicitly constructed with std::ios_base::in. No need to explicitly pass it to the constructor.
You don't need to explicitly call file.close(): since you are not using it again before the end of the scope where it has been declared, it will automagically be closed when destructed. That's the power of RAII! :D
You have two functions supposed to be reusable in other projects, mean and stdDev. The fixed size defined outside of the functions will prevent them from being reusable. You could use std::array instead and have this signature for mean:

template<std::size_t N>
double mean(const std::array<double, N>& arr);

You could even templatize the type so that you can work with something else than double. Morevoer, using std::array would allow you to use a clean range-based for loop:

template<typename T, std::size_t N>
auto mean(const std::array<T, N>& arr)
    -> T
{
    T sum{}; // generally performs zero-initialization
    for (auto&& val: arr)
    {
        sum += val;
    }
    return sum / N;
}

Alternatively, you could use the function above and compute the sum with std::accumulate:

template<typename T, std::size_t N>
auto mean(const std::array<T, N>& arr)
    -> T
{
    return std::accumulate(std::begin(arr), std::end(arr), T{}) / N;
}

This one is more subtle and I like to bother people with it, but return is not a function, so don't put parenthesis around what you return. In your case (in most cases actually), it will work as expected, but if your return type is decltype(auto), then you may have surprises. Consider the following example:

auto foo()
    -> decltype(auto)
{
    int res = 0;
    // do stuff with res
    return (res);
}

Int this particular case, the compiler will deduce the return type int& because you returned a parenthesized identifier and so you will be returning a reference to a temporary, which isn't what you want (and which is really bad). Had you dropped the parenthesis around the return expression, then the deduced would have been the expected int. Bottom line: don't put parenthesis around the returned expression if you want to avoid surprises.


.hover() for fading effect Is there a better way to write the JS for this? I'm asking because I read from somewhere that using .hover isn't recommended. Also, if I move the mouse in and out of the box really fast, the box fades in and out the exact number of times I entered/left the box. How do I prevent this?

jsFiddle

$(".project-mask").hover(
    function() {
        $(".thumbnail").fadeOut(300);
        $(".description").fadeIn(300);
    },
    function() {
        $(".thumbnail").fadeIn(300);
        $(".description").fadeOut(300);
    }
);
.project-mask { height:260px;position:relative;width:260px }
.thumbnail, .description { position:absolute;width:100% }

blah blah blah

This type of functionality is what .hover() was designed for. I wouldn't recommend using anything else. As far as avoiding building up a queue of events when hovering, jQuery has the .stop() method for that very reason.

$(".project-mask").hover(
    function() {
        $(".thumbnail").stop(true, true).fadeOut(300);
        $(".description").stop(true, true).fadeIn(300);
    },
    function() {
        $(".thumbnail").stop(true, true).fadeIn(300);
        $(".description").stop(true, true).fadeOut(300);
    }
);

Here is a fiddle


Python Steganographer using PIL I've made a Python Steganographer using the Python Imaging Library. It basically encodes the binary text in the last 'n' bits of the red content in each pixel of the image. Here is my code, please suggest some improvements.

from PIL import Image

import os

def str2bin(message):
    binary = bin(int.from_bytes(message.encode('utf-8'), 'big'))
    return binary[2:]

def bin2str(binary):
    n = int(binary, 2)
    return n.to_bytes((n.bit_length() + 7) // 8, 'big').decode()

def hide(filename, message, bits=2):
    image = Image.open(filename)
    binary = str2bin(message) + '00000000'

    if (len(binary)) % 8 != 0:
        binary = '0'*(8 - ((len(binary)) % 8)) + binary

    data = list(image.getdata())

    newData = []

    index = 0
    for pixel in data:
        if index < len(binary):
            pixel = list(pixel)
            pixel[0] >>= bits
            pixel[0] <<= bits
            pixel[0] += int('0b' + binary[index:index+bits], 2)
            pixel = tuple(pixel)
            index += bits

        newData.append(pixel)

    image.putdata(newData)
    image.save('\\'.join(filename.split('\\')[0:-1]) + '/coded-'+os.path.basename(filename), 'PNG')

    return len(binary)

def unhide(filename, bits=2):
    image = Image.open(filename)
    data = image.getdata()

    binary = ''

    index = 0

    while not (len(binary) % 8 == 0 and binary[-8:] == '00000000'):
        value = '00000000' + bin(data[index][0])[2:]
        binary += value[-bits:]
        index += 1

    message = bin2str(binary)
    return message

if __name__ == '__main__':

    file = open('tmiab.txt', encoding='utf-8')

    hide('E:\\Python\\Steganography\\img.png', file.read(), 2)
    print(unhide('E:\\Python\\Steganography\\coded-img.png', 2)[:10000])

I am able to encode the novel Three Men In A Boat in a 1600 by 1200 pixel image by just replacing the last two bits.

I am looking for some improvements like reducing time consumption.

Please help. Thanks.

Time consumption suggestions

Doing list(Image.open(filename).getdata()) means that you read the entire image before starting any processing. This means time and memory overhead in the order of number of pixels. It looks like you only ever need to process one pixel at a time, so you can instead iterate over the results of getdata() directly, treating the pixels as a stream. This becomes crucial when dealing with even bigger data structures, especially when the full data won't fit in memory. If the result of getdata() is not the best for your purposes there are many other ways of iterating over pixels in an image. Iterating over the text data is probably less crucial (since the data should be much smaller), but would also improve matters over reading the entire thing at once.

You are currently doing many value conversions, from UTF-8 string to binary and back, binary to int and back, and pixel to list (presumably to separate colour values). Avoiding these conversions should massively improve the speed of your program. In general, settling on a single "datatype of exchange" is a good idea, and in this case integers are an obvious choice - they map easily to both text and pixel values, and they are blazingly fast to work with.

General suggestions

You have optional parameters which are never used with anything other than their default values. This adds clutter, and makes me suspicious of whether the functions would even do the right thing if called with anything else.

Use argparse to allow specifying the input data file and image file. This makes the program much more versatile and acceptance testable.

Test driving this code should point towards some limitations of this program:

What if the data doesn't fit in memory?
What if the image is too small to encode the entire text?
What if the user specifies bits > 8?
Why assume UTF-8 rather than just encoding arbitrary byte values? The extra limitation of UTF-8 makes it harder to ensure that the data is correctly encoded and decoded, and limits the utility of the program. You might for example want to encode an image within another image.
What if the image is not eight bits per pixel?
What is the maximum perceptive colour distance between an unmodified and a steganographic pixel? That is, could a person possibly detect the colour difference, for example in a high quality photo with big uniform patches or a uniformly coloured generated picture?

pep8 is a great little tool to ensure your code is idiomatic Python. For example: in Python variable names are always under_scored by convention.


Implementing Haskell's `delete` Learn You a Haskell shows the delete function.

  delete: deletes first occurrence of item

ghci> delete 'h' "hey there ghang!"
"ey there ghang!"

Please critique my implementation.

delete' :: (Eq a) => a -> [a] -> [a]
delete' y xs = delete'' y xs []
                 where delete'' _ [] _ = []
                       delete'' d (a:as) acc
                         | a == d    = reverse acc ++ as
                         | otherwise = delete'' d as (a:acc)

I don't like the fact that I'm using the : operator, but then calling reverse and acc ++ as.

Also, I don't know how to capture the first pattern match of delete'' with a single pattern match and guards alone.

I think you're overcomplicating this a bit. Something like delete certainly shouldn't need reverse or ++ to implement.

For the initial list, there are two cases - it either contains an element, or is empty. That much exists in your code. The empty case is simple, just return the empty list:

delete' :: (Eq a) => a -> [a] -> [a]
delete' y [] = []

Ok, so what about when the list is not empty. Well, in that case, we can simply think about it as follows: if the next element is equal to the element we want to remove, then remove it and return the rest of the list. Otherwise, append the next element and remove the first occurrence from the tail of the list. In code, this looks like:

delete' y (x:xs) = if x == y then xs else x : delete' y xs


Search algorithms in julia I've been trying to implement the basic search algorithms: Sequential search for ordered and unordered arrays and the binary search algorithms, as part of a package.

So, this is the implementation:

"""
`seqsearch{T}(x::AbstractArray{T}, item::T)` ↦ found::Bool
Searches for an item using the **Sequential Search** approach.
"""
function seqsearch{T}(x::AbstractArray{T}, item::T)
  index = 1
  found = false

  while index <= length(x) && !found
    x[index] == item ? found = true : index += 1
  end

  return found
end

"""
`ordseqsearch{T}::(x::AbstractArray{T}, item::T) ↦ found::Bool`
Searches for an item in a sorted array using the **Sequential Search** approach.
"""
function ordseqsearch{T}(x::AbstractArray{T}, item::T)
  index = 1
  found = false
  stop  = false

  while index <= length(x) && !found && !stop
    x[index] == item ? found = true : x[index] > item ? stop = true : index += 1
  end

  return found
end

"""
`binarysearch{T}::(x::AbstractArray(T), item::T) ↦ found::Bool`
Searches for an item in a sorted array using the **Binary Search** algorithm
"""
function binarysearch{T}(x::AbstractArray{T}, item::T)
  first = 1
  last = length(x)
  found = false

  while first <= last && !found
    middle = fld(first+last, 2)

    if x[middle] == item
      found = true
    else
      if item > middle
        first = middle + 1
      else
        last = middle - 1
      end
    end

  end

  return found
end

I want to make the implementations as fast as possible, with as less dependencies as possible.

So, are there any ways to make it cleaner (and/or) faster?

I would get rid of variables found and stop, and just return true on match, false after the body (which means there was no match). The reason is that you keep checking in the loop conditional their value; that's unnecessary computation.

Edit Actually, it would be even better if you returned the index of the found element instead of just a boolean value indicating whether the element is in the input array. In case there is no match, return a value that cannot be a valid array index, for example, -1.


Creating a Navbar Menu from JSON data with Lodash I am trying to create a navbar menu from JSON data. Actually I have achieved it but I am looking for feedback not to call getItems twice? How can I improve my code?
Thank you
var _ = require('lodash');

let menuConfig = [
    {
        ID: 1,
        TAG: "M:A",
        PARENT_TAG: "MAIN",
        TITLE: "A Title"
    },
    {
        ID: 2,
        TAG: "AS1",
        PARENT_TAG: "M:A",
        TITLE: "A Subtitle 1"
    },
    {
        ID: 3,
        TAG: "AS2",
        PARENT_TAG: "M:A",
        TITLE: "A Subtitle 2"
    },
    {
        ID: 4,
        TAG: "AS3",
        PARENT_TAG: "M:A",
        TITLE: "A Subtitle 3"
    },
    {
        ID: 5,
        TAG: "M:B",
        PARENT_TAG: "MAIN",
        TITLE: "B Title"
    },
    {
        ID: 6,
        TAG: "BS1",
        PARENT_TAG: "M:B",
        TITLE: "B Subtitle 1"
    },
    {
        ID: 7,
        TAG: "BS2",
        PARENT_TAG: "M:B",
        TITLE: "B Subtitle 2"
    },
    {
        ID: 8,
        TAG: "M:C",
        PARENT_TAG: "MAIN",
        TITLE: "C Title"
    },
    {
        ID: 8,
        TAG: "CS1",
        PARENT_TAG: "M:C",
        TITLE: "C Subtitle 1"
    }
]

function getMenu() {
    let grouped = _.groupBy(menuConfig, "PARENT_TAG");
    let menu = getItems(grouped.MAIN, grouped);
    console.log(JSON.stringify(menu, null, 3));
}

function getItems(items, grouped) {
    let subMenu = [];
    _.forEach(items, (item) => {
        let newItem = getItem(item, grouped)
        if (newItem) {
            subMenu.push(newItem);
        }
    });
    return subMenu;
}

function getItem(item, grouped) {
    if (grouped[item.TAG]) {
        let subMenu = getItems(grouped[item.TAG], grouped);
        if (subMenu && subMenu.length) {
            return {
                title: item.TITLE,
                subMenu: subMenu
            }
        }
    } else {
        let newItem = {
            title: item.TITLE
        }
        return newItem;
    }
}

getMenu();

Output needs to be like this;
[
  {
    "title": "A Title",
    "subMenu": [
      {
        "title": "A Subtitle 1"
      },
      {
        "title": "A Subtitle 2"
      },
      {
        "title": "A Subtitle 3"
      }
    ]
  },
  {
    "title": "B Title",
    "subMenu": [
      {
        "title": "B Subtitle 1"
      },
      {
        "title": "B Subtitle 2"
      }
    ]
  },
  {
    "title": "C Title",
    "subMenu": [
      {
        "title": "C Subtitle 1"
      }
    ]
  }
]

You can use Array.prototype.reduce to group items by TAG as the key and using the relationship between PARENT_TAG and TAG.
This approach will give the expected result by running the array only once.

const menuConfig = [{
  ID: 1,TAG: "M:A", PARENT_TAG: "MAIN", TITLE: "A Title"
}, {
  ID: 2, TAG: "AS1", PARENT_TAG: "M:A", TITLE: "A Subtitle 1"
}, {
  ID: 3, TAG: "AS2", PARENT_TAG: "M:A", TITLE: "A Subtitle 2"
}, {
  ID: 4, TAG: "AS3", PARENT_TAG: "M:A", TITLE: "A Subtitle 3"
}, {
  ID: 5, TAG: "M:B", PARENT_TAG: "MAIN", TITLE: "B Title"
}, {
  ID: 6, TAG: "BS1", PARENT_TAG: "M:B", TITLE: "B Subtitle 1"
}, {
  ID: 7, TAG: "BS2", PARENT_TAG: "M:B", TITLE: "B Subtitle 2"
}, {
  ID: 8, TAG: "M:C", PARENT_TAG: "MAIN", TITLE: "C Title"
}, {
  ID: 8, TAG: "CS1", PARENT_TAG: "M:C", TITLE: "C Subtitle 1"
}]

function getMenu() {
  const menu = getItems(menuConfig, 'MAIN');
  console.log(menu);
}

function getItems(items, grandParentTag) {
  const newItems = items.reduce((modifiedObj, currentItem) => {
    const parentTag = currentItem.PARENT_TAG;
    const tag = currentItem.TAG;

    if (!modifiedObj[grandParentTag]) {
      modifiedObj[parentTag] = {};
    }

    if (!modifiedObj[grandParentTag][parentTag]) {
      modifiedObj[parentTag][tag] = {
        title: currentItem.TITLE,
        subMenu: [],
      };
    } else {
      modifiedObj[grandParentTag][parentTag].subMenu.push({
        title: currentItem.TITLE,
      });
    }

    return modifiedObj;

  }, {});

  return Object.values(newItems[grandParentTag]);
}

getMenu();


Reordering an array: Even elements descending, odd elements ascending I am trying to work out a solution. I have to order the elements in the array a,  such that a[0] is greater than a[1],  a[1] is less than a[2], a[2] is greater than a[3], and so on...

Basically my method checks if the index is odd or even , and simultaneously makes a decision to swap elements to suit the criteria of the question.

I am getting the following output: 7, 1, 6, 2, 5, 3, 4

public class ArrayRearrange {

    public static void main(String[] args) {
        // TODO Auto-generated method stub
        int[] a = { 2, 1, 4, 6, 5, 3, 7 };

        for (int i = 0; i < a.length; i++) {
            for (int j = i + 1; j < a.length; j++) {
                if (i % 2 == 0 && a[i] < a[j]) {
                    int temp = a[i];
                    a[i] = a[j];
                    a[j] = temp;
                }
                if (i % 2 != 0 && a[i] > a[j]) {
                    int temp = a[i];
                    a[i] = a[j];
                    a[j] = temp;
                }
            }
        }
        for(int i=0;i<a.length;i++)
        System.out.print(a[i]+" ");

    }

}

Let me know if there's a better way to do it.

Putting everything in the main method is bad practice, and is hard to maintain. Separate it into methods. This also removes duplicate code, such as when you swap values.
// TODO Auto-generated method stub is generated in eclipse, and is only there to remind you to finish the method. It is not really to stay.
Don't use one-letter variable names, except for counters. a should be array.
Don't be lazy when you print something out. Still follow conventions.
The two if statements are doing identical things. Combine them with an OR.

Final code:

public class ArrayRearrange {

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

        rearrange(array);

        for(int i = 0; i<array.length; i++) {
            System.out.print(a[i] + " ");
        }
    }

    private static void rearrange(int[] array) {
        for (int i = 0; i < array.length; i++) {
            for (int j = i + 1; j < array.length; j++) {
                if ((i % 2 == 0 && array[i] < array[j]) || (i % 2 != 0 && array[i] > array[j])) {
                    swap(array, i, j);
                }
            }
        }
    }

    private static void swap(int[] array, int i, int j) {
        int temp = array[i];
        array[i] = array[j];
        array[j] = temp;
    }

}


Merge sort in Golang I am just starting out with golang. I implemented the merge sort algorithm just to practice.

Any suggestions or criticisms regarding the coding style?

package main

import (
    "fmt"
)

func main() {
    A := []int{3, 5, 1, 6, 1, 7, 2, 4, 5}
    fmt.Println(sort(A))
}

// top-down approach
func sort(A []int) []int {
    if len(A) <= 1 {
        return A
    }

    left, right := split(A)
    left = sort(left)
    right = sort(right)
    return merge(left, right)
}

// split array into two
func split(A []int) ([]int, []int) {
    return A[0:len(A) / 2], A[len(A) / 2:]
}

// assumes that A and B are sorted
func merge(A, B []int) []int {
    arr := make([]int, len(A) + len(B))

    // index j for A, k for B
    j, k := 0, 0

    for i := 0; i < len(arr); i++ {
        // fix for index out of range without using sentinel
        if j >= len(A) {
            arr[i] = B[k]
            k++
            continue
        } else if k >= len(B) {
            arr[i] = A[j]
            j++
            continue
        }
        // default loop condition
        if A[j] > B[k] {
            arr[i] = B[k]
            k++
        } else {
            arr[i] = A[j]
            j++
        }
    }

    return arr
}

The only style-based comment I have is that we usually don't use upper case names/letters for parameters or local variables.


Throwing exceptions if there is not exactly one box retrieved (Note: this is originally posted as this SO question  )

I'm trying to refactor the error logging and exception throwing into a method.

Original code:

private Box retrieveBox() {
    BoxParameter param = new BoxParameter("red", 10, 5, 3);
    List<Long> boxIds = boxDao.retrieveByParameter(param);

    if (boxIds.isEmpty()) {
        // A
        logger.warn("box not found");
        throw new BoxNotFoundException();
    }
    if (boxIds.size() > 1) {
        // B
        logger.warn("multiple boxes found");
        throw new MultipleBoxesFoundException();
    }

    Optional<Box> maybe = boxDao.retrieve(boxIds.get(0));
    if (maybe.isPresent()) {
        return maybe.get();
    }

    // C
    logger.warn("box not found");
    throw new BoxNotFoundException();
}

Refactored code:

private Box retrieveBox() {
    BoxParameter param = new BoxParameter("red", 10, 5, 3);
    List<Long> boxIds = boxDao.retrieveByParameter(param);

    if (boxIds.isEmpty()) {
        // A
        logErrorAndThrowBoxNotFoundException();
    }
    if (boxIds.size() > 1) {
        // B
        logErrorAndThrowMultipleBoxesFoundException();
    }

    Long boxId = boxIds.get(0);
    Optional<Box> maybe = boxDao.retrieve(boxIds.get(0));
    if (maybe.isPresent()) {
        return maybe.get();
    }

    // C-1
//  logger.warn("box not found");
//  throw new BoxNotFoundException();

    // C-2
    logErrorAndThrowBoxNotFoundException();
    return null;  // needs to be here to make it compile
}

private void logWarningAndThrowMultipleBoxesFoundException() throws MultipleBoxesFoundException {
    logger.warn("multiple boxes found");
    throw new MultipleBoxesFoundException();
}

private void logWarningAndThrowBoxNotFoundException() throws BoxNotFoundException {
    logger.warn("box not found");
    throw new BoxNotFoundException();
}

I really dislike the return null part in C-2, but without it, I'll have to go back to C-1, which defeats the purpose of refactoring.

Is there a better way to do it?

I'd suggest you just get rid of the throwing method and throw it in the retrieveBox method like this:

throw new BoxNotFoundException("box not found");

You should catch it outside this method and handle it, do the logging etc when you catch it. Logging is part of exception handling and should be done when the exception is being handled, not before.


Button click handlers for a JavaScript text-based adventure game I'm pretty new to JavaScript, and I'm trying to created a text-based game. I've written a few scenarios and based on the player's decision I have different scenarios written out. As of now I have the code written and it works but I feel like I can be more efficient with an if else statement in my js.

Here's what I have so far in terms of player decisions.

 $(document).ready(function(){

        $(".sit_1, .sit_2, .sit_3a, .sit_3b, .sit_4a").hide();
        $(".choice_1, .choice_2a, .choice_2b, .choice_3a, .choice_3b, .choice_4a, .choice_4b").hide();
        $(".sit_1").fadeIn(2000);
        $(".choice_1").fadeIn(4000);

        $(".choice_1").click(function(){
         $(".sit_1").hide();
         $(".choice_1").hide();
         $(".sit_2").fadeIn(4000);
         $(".choice_2a").fadeIn(8000);
         $(".choice_2b").fadeIn(8000);

         });

        $(".choice_2a").click(function(){
         $(".sit_2").hide();
         $(".choice_2a").hide();
         $(".choice_2b").hide();
         $(".sit_3a").fadeIn(4000);

         });

        $(".choice_2b").click(function(){
         $(".sit_2").hide();
         $(".choice_2a").hide();
         $(".choice_2b").hide();
         $(".sit_3b").fadeIn(4000);
         $(".choice_3a").fadeIn(4000);
         $(".choice_3b").fadeIn(4000);

         });

        $(".choice_3a").click(function(){
         $(".sit_3b").hide();
         $(".choice_3a").hide();
         $(".choice_3b").hide();
         $(".sit_4a").fadeIn(3000);
         $(".choice_4a").fadeIn(4000);
         $(".choice_4b").fadeIn(4000);

         });

        });

         You are lost in the woods.

         You find a cave. Do you go inside or go look for food?

         You walk into the cave.

         You walk away from the cave, to search for food. You find berries. Do you eat them or not?

         You eat the berries.

     <input type="button" class="choice_1" value="Continue on...">

     <input type="button" class="choice_2a"value="Go inside">

     <input type="button" class="choice_2b" value="Look for food">

     <input type="button" class="choice_3a" value="Eat the berries">

     <input type="button" class="choice_3b" value="Don't eat the berries">

Not sure how clear this is but I basically want all my situations (sit_) to be hidden along with the player choices (choice_) until the player presses a button or makes a choice and then the old situations along with the old choices will disappear to make way for the new situations and choices.

As I said this code does work but I feel like I can make it more efficient with an if/else statement but I'm not sure how to go about it. Any help would be greatly appreciated.

Also, should I make the choices into an array to make it easier?

You are falling into a conceptual trap of trying to define javascript behaviors for each element on the page by numbering each and every one with a different id or class name (i.e. choice_*).  Any time you see this pattern in your coding, this should be a red flag that there is probably a better approach to your problem (which is why I think you are asking for this review).

With jQuery (or really javascript in general), you should be thinking in terms of what the behavior of an element is supposed to be using classes to identify elements which need to exhibit similar behavior, much in the same way you use classes to define elements that have common CSS stylings.  For example, all of your choice buttons fundamentally have the same behavior.  That is, they hide other options and expose a new situation.  Because this basic behavior is common, there is no reason that you need to duplicate code for each of these buttons.

So the question then becomes how do you generalize your code to have a single onclick handler for all of the choice buttons?

To do this, you have to have an understanding of which buttons expose which situations - something you are now implementing via choice_* class naming.

In jQuery/javascript, there are probably two main strategies to do this. One is to tie the elements together via their hierarchy in the DOM and to traverse the DOM hierarchy to hide/show related elements based on their location relative to each other on the DOM.  For example, when hiding/showing elements, you might hide all the siblings in same level of DOM as the one you are trying to show.

The other main approach is to use data attributes to describe how elements should interact with one another.

I will focus on the data attribute approach here as I think this makes the most sense for your use case.  You have a potentially complex set of interactions in a "Choose Your Own Adventure" style of game where you ultimately may have multiple story branches each which could lead to different situations via different paths.  This would likely be hard to model via HTML hierarchy alone once you move beyond just a trivial proof of concept.  Data attributes are also a powerful tool for a budding javascript developer to learn.

I would propose some changes to your HTML to make each "situation" its own self-contained set of HTML elements, like the concept of a page.  You could then hide/show each situation (including choices) as a single entity.  I also think in this case it makes your HTML much easier to read and maintain.

            You are lost in the woods.

        <input type="button" class="situation_choice"
            value="Continue on..."
            data-situation-target="found_cave">

            You find a cave. Do you go inside or go look for food?

        <input type="button" class="situation_choice"
            value="Go inside"
            data-situation-target="enter_cave">
        <input type="button" class="situation_choice"
            value="Look for food"
            data-situation-target="food_search">

            You walk into the cave.

            You walk away from the cave, to search for food. You find berries. Do you eat them or not?

        <input type="button" class="situation_choice"
            value="Eat the berries"
            data-situation-target="eat_berries">
        <input type="button" class="situation_choice"
            value="Don't eat the berries"
            data-situation-target="ignore_berries">

            You eat the berries.

            You ignore the berries. You are still hungry.

Note that we apply a data attribute called situation on the container of each situation "page" to uniquely identify it amongst all situations.  We also have data attributes on each option specifying the situation-target that option relates to (i.e. which situation will be shown when the button is clicked).

I have added another class situation_default to show which situation(s) are visible for display on initial page load.

Finally, I have also introduced a class for situation_text here that is not used in the javascript at all, but could perhaps give you a handle for applying CSS styling.

You use these data attributes within your javascipt to implement the logic as to what situation is shown when an option is clicked.

$(document).ready(function() {
    // Create jQuery handle to all situations.
    // Having this outside click handler allows you to access it while
    // preventing you from having to query the DOM every time you want to
    // act on this selection of elements as a whole.
    var $allSituations = $('.situation');

    // Get selector for default situation(s) which will show on page load
    var $defaultSituation = $('.default_situation');

    // Show default situation(s)
    $allSituations.hide();
    $defaultSituation.fadeIn(2000);

    $('.situation_choice').on('click', function() {
        // Find target situation element for this choice.
        // We use the 'situation-target' data attribute for the
        // clicked element, this, within the handler to determine
        // the target.
        var targetName = $(this).data('situation-target');
        var $target = $('.situation[data-situation="' + targetName + '"]')

        // if we find a single target element, we can now show it
        if($target.length === 1) {
            // hide all situations
            $allSituations.hide();
            // show target element
            $target.fadeIn(4000);
        }
    });
});


Removing nested blocks from a string I wrote this function in scala that uses tail recursion to remove nested blocks from a text.

Usage examples:

removeBlocks("123{456}789", "{", "}")                            yields "123789"
removeBlocks("123{a{b{c}b}a}789", "{", "}")                      yields "123789"
removeBlocks("123
456
789", "
", "
")          yields "123789"
removeBlocks("123<<
<<<
>>789", "
", "
")      yields "123<<>>789"

This is one of my first attempts in a functional language, and I'm deeply dissatistied with my code. It's too long, too nested, and could probably be made more readable. I'm coming from C# and hope to be forgiven for this bad functional code. I'll be happy to hear how it can be improved.

  def removeBlocks(text: String, startMarker: String, endMarker: String) = {
    val startMarkerSize = startMarker.size
    val endMarkerSize = endMarker.size
    val startMarkerHead = startMarker.head
    val endMarkerHead = endMarker.head

    def removeBlocksAcc(s: String, acc: String, nestingLevel: Int): String =
    if (s.isEmpty) acc
      else {
        val (startMarkerCandidate, tail1) = s.splitAt(startMarkerSize)
        if (startMarkerCandidate == startMarker)
          removeBlocksAcc(tail1, acc, nestingLevel + 1)
        else {
          val (endMarkerCandidate, tail2) = s.splitAt(endMarkerSize)
          if (endMarkerCandidate == endMarker)
            removeBlocksAcc(tail2, acc, math.max(nestingLevel - 1, 0))
          else {
            val (safePart, candidate) = s.tail.span(c => c != startMarkerHead && c != endMarkerHead)
            if (nestingLevel == 0)
              removeBlocksAcc(candidate, acc + s.head + safePart, nestingLevel)
            else
              removeBlocksAcc(candidate, acc, nestingLevel)
          }
        }
      }
    removeBlocksAcc(text, "", 0)
  }

I think you are trying to do some pattern matching on strings.

So I did this:

def escape(s: String): String = Seq("{", "}", "whatever").foldLeft(s)((x, y) => x.replaceAllLiterally(y, "\\" + y))

def removeBlocks(text: String, open: String, close: String): String = {
    val startReg = (s"${escape(open)}(.*)$$").r
    val endReg = (s"${escape(close)}(.*)$$").r
    val otherText = "(.)(.*)$".r

    @tailrec
    def removeBlocksAux(text: String, acc: String = "", lvl: Int = 0): String = {
        assume((text.length() > 0 || lvl == 0) && lvl >= 0)
        (text, lvl) match {
            case (startReg(text), lvl)     => removeBlocksAux(text, acc, lvl + 1)
            case (endReg(text), lvl)       => removeBlocksAux(text, acc, lvl - 1)
            case (otherText(c, text), 0)   => removeBlocksAux(text, acc + c)
            case (otherText(c, text), lvl) => removeBlocksAux(text, acc, lvl)
            case ("", _)                   => acc
        }
    }
    removeBlocksAux(text)
}

I tried to find a way to define the regular expressions inside the match group, but I failed at that, it may not be a good idea after all, but I would like to have a more compact and efficient representation for it. The escape function should escape all special cases of operators in Scala regex, that sounds boring, so there may be already a method for that.

I'm also not a big fan of having a function inside a function, but I kept that design decision. Finally, I'd simply use parser combinators for this, but I guess that's not the point.


Python election challenge Basically, there is an election on 3 regions where either candidate needs to win 2 in order to win the election. Idea is to simulate 10000 elections to see which candidate wins. I think my solution works but I am not happy with the huge list of if statements and wondering of if there is a better way for this.

# Assignment simulate elections page 257

# 2 Candidates A and B

# Candidate A has following odds:
# 87% change of winning in region 1
# 65% changes of winning in region 2
# 17% changes of winning in region 3

from random import random

candidate_a_won = 0
candidate_b_won = 0

for i in range(0, 10000):
    result = random()
    candidate_a = 0
    candidate_b = 0
    if result + .87 >= 1:
        candidate_a += 1
    else:
        candidate_b += 1
    if result + .65 >= 1:
        candidate_a += 1
    else:
        candidate_b += 1
    if result + .17 >= 1:
        candidate_a += 1
    else:
        candidate_b += 1
    if candidate_a > candidate_b:
        candidate_a_won += 1
    else:
        candidate_b_won += 1

print('Candidate A won elections {} times, candidate B won elections {} times'.format(candidate_a_won, candidate_b_won))

Your code has a problem. Candidate A can loose region 1, but win region 3.
Where with your code they can only win region 3 if they win region 2 and 1.
This is as they are independent, and so you should random in each check.
You want to move your chances outside the loop into an array.
You can simplify your ifs into one comprehension.
Candidate a wins if they have more than half the regions, you don't need to calculate the amount of regions candidate b wins.

And so you could change your code to the following:

from random import random

AMOUNT = 10000
region_chances = [87, 65, 17]
region_chances = [1 - n / 100 for n in region_chances]
regions = len(region_chances)
candidate_a_won = sum(
    sum(random() >= chance for chance in region_chances) * 2 > regions
    for _ in range(AMOUNT)
)
candidate_b_won = AMOUNT - candidate_a_won
print('Candidate A won elections {} times, candidate B won elections {} times'.format(candidate_a_won, candidate_b_won))

If you want to extend on the above then it'll be hard if you decide to have more than two candidates, or even amounts of states.
But as this is a challenge, I don't think you need to worry about these situations.


Sum of HList (where all elements are Nat's) I wrote the following to output, at the type level, the sum, Nat, of an input HList:

trait HListSum[L] {
  type Out
}
object HListSum {

  type Aux[L, O] = HListSum[L] { type Out = O }

  def apply[L <: HList](implicit ev: HListSum[L]): ev.type = ev

  implicit def hListSumInductive[H <: Nat, L <: HList, S <: Nat, T <: Nat](
    implicit rest: HListSum.Aux[L, T],
             all: Sum.Aux[H, T, S]): HListSum.Aux[H :: L, S] = new HListSum[H :: L] {
    type Out = S
  }

  implicit val hlistSumHNil: HListSum.Aux[HNil, _0] = new HListSum[HNil] {
    type Out = _0
  }
}

Testing:

import net.HListSum
import shapeless._
import nat._

scala> HListSum[_1 :: _2 :: _3 :: HNil]
res0: net.HListSum.Aux[shapeless.::[shapeless.nat._1,shapeless.::[shapeless.nat._2,shapeless.::[shapeless.nat._3,shapeless.HNil]]],this.Out] = net.HListSum$$anon$4@132c4879

scala> val expected: res0.Out = _6
expected: res0.Out = Succ()

scala> HListSum[_0 :: _0 :: _1 :: HNil]
res1: net.HListSum.Aux[shapeless.::[shapeless.nat._0,shapeless.::[shapeless.nat._0,shapeless.::[shapeless.nat._1,shapeless.HNil]]],this.Out] = net.HListSum$$anon$4@a7b83a8

scala> val expected2: res1.Out = _1
expected2: res1.Out = Succ()

scala> val expected2: res1.Out = _3
<console>:19: error: type mismatch;
 found   : shapeless.nat._3
    (which expands to)  shapeless.Succ[shapeless.Succ[shapeless.Succ[shapeless._0]]]
 required: res1.Out
    (which expands to)  shapeless.Succ[shapeless._0]
       val expected2: res1.Out = _3
                                 ^

Please critique my code. Also, am I using induction to determine the sum for the non-HNil case?

First for some little things. I'd put bounds on the L type parameter and the Out type member to capture the facts that you know will always be true about them:

trait HListSum[L <: HList] {
  type Out <: Nat
}

This makes the intent clearer to readers and allows you to use Out in places you couldn't otherwise—e.g. this simple example wouldn't compile without the bound:

scala> def foo[N <: Nat]: Unit = ()
foo: [N <: shapeless.Nat]=> Unit

scala> def bar[L <: HList](implicit sum: HListSum[L]): Unit = foo[sum.Out]
bar: [L <: shapeless.HList](implicit sum: HListSum[L])Unit

I'd also change the return type of apply to be a little more specific (or rather less specific, I guess—more focused on what's relevant, in any case):

def apply[L <: HList](implicit ev: HListSum[L]): Aux[L, ev.Out] = ev

This is mostly for legibility—apart from how the type is printed in the console I'm not sure off the top of my head whether there's any real difference between this and the ev.type version.

I'd make two changes to the hListSumInductive implementation (apart from changing the case of the l in the name to be consistent with hlistSumHNil):

implicit def hlistSumInductive[H <: Nat, T <: HList, TS <: Nat](implicit
  rest: HListSum.Aux[T, TS],
  all: Sum[H, TS]
): HListSum.Aux[H :: T, all.Out] = new HListSum[H :: T] {
  type Out = all.Out
}

The first is that I've renamed L to T and T to TS, since using H and T to name the head and tail types of an hlist is a pretty standard convention. More significantly, I've dropped the S type parameter (representing the total sum) altogether, since we don't need a type parameter to refer to that type (all.Out works just fine, since we don't need to refer to it in other implicit parameters).

One other note about this method: I'm not sure whether you chose Sum[H, TS] over Sum[TS, H] intentionally, but it's the right thing to do if you care about compile times (and you should in a case like this), since the instance will be resolved more quickly when the larger number is on the right, and in this operation the larger number will be TS more often.

One other tiny thing—I'd probably rename O in Aux:

type Aux[L <: HList, Out0] = HListSum[L] { type Out = Out0 }

Mostly because O is easy to confuse with 0, and because I personally tend to use the 0 suffix as a convention in these cases. That's entirely a matter of taste, though.

Lastly, if I were writing this for a library I'd probably use a sealed abstract class instead of a trait and would make the object final, but neither change is terribly important.


Is this base 62 encoding algorithm okay? This Scala code snippet is supposed to encode a SHA1 hash in base 62.

Can you find any issues? I'm asking since I might not be able to change the algorithm and, for example, fix issues in the future.

I'd like to be able to also implement it in JavaScript in the future.

def mdSha1() = java.security.MessageDigest.getInstance("SHA-1") // not thread safe

def hashSha1Base62DontPad(text: String): String = {
  val bytes = mdSha1().digest(text.getBytes("UTF-8"))
  val bigint = new java.math.BigInteger(1, bytes)
  val result = encodeInBase62(bigint)
  result
}

private val Base62Alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"

def encodeInBase62(number: BigInt): String = {
  // Base 62 is > 5 but < 6 bits per char.
  var result = new StringBuilder(capacity = number.bitCount / 5 + 1)
  var left = number
  do {
    val remainder = (left % 62).toInt
    left /= 62
    val char = Base62Alphabet.charAt(remainder)
    result += char
  }
  while (left > 0)
  result.toString
}

It is 'standard' to have 0-9 at the beginning of the 'alphabet' for numbers....

I would have suggested that you use the native functionality in BigInteger to convert the value to a String in any given radix, but unfortunately, it does not support more than radix 36. Still, you should follow that standard and start with 0-9 instead of ending with it.

I would also suggest two things:

you should convert the alphabet to an array immediately:

private val Base62Alphabet = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz".toCharArray()

you should not have the magic number '62' in your code, but should base it off the array size:

private val Radix = Base62Alphabet.length

your code then becomes:

val remainder = (left % Radix).toInt
left /= Radix

Finally, I don't like that you have variable-length results from the conversion. You should ensure that all hashes of data with the same length have the same length of output... -left-padding with 0 as needed. It is possible for something to hash to 0x0000000000 (hex).... which will give you 0-value number.bitCount.


Generating Pandigital Numbers A Base10 Pandigital Number is a number which uses all the digits 0-9 once:

1234567890
2468013579
etc...

My naive solution was just to use a bunch of nested loops to do this but it's quite slow. And I'm blanking on a more efficient way to do it? The following takes ~6 seconds.

IEnumerable<long> GeneratePandigital()
{
    var other=Enumerable.Range(0,10);

foreach(var a in other)
foreach(var b in other.Except(new int [] {a}))
foreach(var c in other.Except(new int [] {a,b}))
foreach(var d in other.Except(new int [] {a,b,c}))
foreach(var e in other.Except(new int [] {a,b,c,d}))
foreach(var f in other.Except(new int [] {a,b,c,d,e}))
foreach(var g in other.Except(new int [] {a,b,c,d,e,f}))
foreach(var h in other.Except(new int [] {a,b,c,d,e,f,g}))
foreach(var i in other.Except(new int [] {a,b,c,d,e,f,g,h}))
foreach(var j in other.Except(new int [] {a,b,c,d,e,f,g,h,i}))
{
    yield return a * 1000000000L +
    b * 100000000L+
    c * 10000000L+
    d * 1000000+
    e * 100000+
    f * 10000+
    g * 1000+
    h * 100+
    i * 10+
    j;
}
}

The easiest way to do this is to note that each value that is generated will be a permutation of the values 0 - 9. This will generate 10! = 3628800 possibilities. There are a number of algorithms to generate permutations, the easiest in this case would simply be to generate them in lexicographic order.

Wiki has a rundown of some algorithms you can use to generate permutations.


Flattening a doubly linked list I am a Java dev and trying to prepare for interviews in C++. Can someone please review the below solution for me?

  Assume that you are given the head and tail pointers of a doubly linked list where each node can also have a single child pointer to another similar doubly linked list. There are no cycles in this structure outside of the traditional double links. Write a procedure in C++ that flattens this structure into a single list.

**I am ignoring that it is a doubly linked list for now to make the posted code more simple, since the "doubly" part doesn't seem to be of any help to me for this question.

void LL::flatten(Node *head,Node *tail)
{
    if(!head||!tail) return;

    while(!head->down &&!(head==tail))
    {
        head = head->next;
    }

    // Flattening is complete
    if(head==tail && !head->down) return;

    tail->next = head->down;
    head->down = null;

    //getTail returns the last node of the linkedlist
    flatten(head->next, getTail(tail->next));

}

class Node
{
public:
    Node()
    {
        next = prev = down = null;
        data = -1;
    }
private:
    Node * next;
    Node * down;
    Node * prev;
    int data;
};

I have some questions:

Is the "doubly" part required. What am I missing?
I have read it is better non-recursively. How is that? Since we have only pointers on the stack and the actual objects/list structure are on the heap, this should not take huge space on the stack, apart from usual recursion overheads.
Does someone have a non-recursive way of doing it in C++ or Java. The code on this forum is in C# and I am having trouble understanding it with enumerations and all the iterations
The getTail(tail->next) function is \$O(n)\$. Is there a way of not using this function ?. Even If I attach the 'down' linked list to the next node instead of last node, I will need the last node of the 'down' LL so that I can attach it to the next node of parent.

Though I know there might be many solution already posted elsewhere, I am looking for review for my code as well, this will help me understand my gaps while coding in c++ (at least up to interview standards).

You could write this code in Java first, it would not be much different from a C++ implementation. It seems that your problem is not the language, but rather the algorithm: E.g. you only update the next pointer, not the prev pointer. The following must be true for every link between two nodes: node->next->prev == node.

Let me try to answer your questions:

The "doubly" part is not required to write something that can be flattened, but it is explicitly asked for so you must maintain it. It will allow you to traverse the list in two directions, instead of just one. This can be valuable in many situations.
The question states that the child pointer is to a list similar to the first list. This means that it can also have child pointers of its own. A recursive solution would be easy to write and easy to understand.
Why do you want a non-recursive algorithm? This is not stated in the question.
I believe you can use any language you like in this forum.


Displaying a random poem from a database I'm new to web development, and I'm constantly torn between good code and bad code. I was wondering if you could help me improve my following code and see if I'm doing it right in terms of how I use it on the line by line basis: - t code checks to see if a cookie exists if it doesnt exist it sets a cookie to last for 23 hours and 30 minutes after the cookie is set it pulls a random poem from a database using and and sets it on page to be displayed if the user visits the page more than once and the cookie is set no data will be retrieved and leave old data till the cookie expires -

<?php

if (@!$_COOKIE['true']){
    setcookie('true','true',time()+86100);

    $timeCheck = date('H:i', strtotime('23:30:00'));

    $currentTime = date('H:i', time());
    if ($currentTime >= $timeCheck){
            require_once 'upload/mysqlserver/login.php';

    $query = "SELECT * FROM poems";
    $queryRun = mysql_query($query);
     if ($queryRun = mysql_query($query)){
                // getting data
                if (mysql_num_rows($queryRun)== NULL){
                   echo "No poems in DataBase";
                }else{
                    $randNum = mysql_num_rows($queryRun);
                    $maxNum = rand(1, $randNum);

                    $query2 = "SELECT poem_name, poem, author, time_posted FROM poems WHERE id=$maxNum";
                    $queryRun2 = mysql_query($query2);

                        $row = mysql_fetch_assoc($queryRun2);
                        $poemName = $row['poem_name'];
                        $poem = $row['poem'];
                        $author = $row['author'];
                        $time_posted = $row['time_posted'];

                        $PoTDset = $poemSet = "

Poem name:

".$poemName ."

"
                        ."
Poem:

"."
".$poem."

"
                        ."
Author:
 ".$author."
"
                        ."
Posted on:
".$time_posted."

";

                        $handle = fopen('navigation/PoTD/setPoTD.php', 'w');
                        fwrite($handle, "<?php"."\n"."return ".'"'.$PoTDset.'"'."\n".";\n?>");
                        @fclose($handle);

                 }
            }
        }
}

$pageTemp->content .= include_once "navigation/PoTD/setPoTD.php";
?>

In addition to the depreciation and injection issues pointed out in the comments, I would just add that SELECT * FROM poems will fetch all columns for all rows, which is terribly inefficient. SELECT COUNT(*) FROM poems will return the number of records in the table, and you won't have to call mysql_num_rows.


Code allowed to test mail invitations by users (from another post) on an instance of Visual studio debugging I am currently working on an application in ASP.Net MVC, with a SQL Server 2016 database and Entity Framework 6.

The application can process specific data and generate forms (in DOC format), and in some cases it sends emails to targeted people.

My problem is with this piece of code (The code set is further down):

#region Fix proxy proxy locally on URL
Var currentHost = "localhost: 57061";
Var newhost = $ "{GetIPAddressV4 ()}: 90";
Url = url.Replace (currentHost, newhost);
#endregion

What I learned from the explanation I was given was:

  This code allowed to test mail invitations by users (from another
  post) on an instance of Visual studio debugging.

  The purpose of this step is to modify the URL sent in the email so
  that the new URL points to the instance of Debug VS. To use this
  trick, it uses iisexpress-proxy.

I have an opinion on this code but I would like an expert feedback on this.

My questions:

Is this code clean?
Is it dangerous (reliability, safety)?
Can we replace it?

The code set is here:

 public class SendMailMgr
    {
        #region MailInvit

        public static void SendMailInvit(BaseViewModels model, string userId, string htmlInvit)
        {
            var usr = PersonalInfoDbMgr.GetByUserId(userId);

            if (usr != null)
            {
                var url = string.Format("{0}://{1}{2}",
               model.Request.Url.Scheme,
               model.Request.Url.Authority,
               model.Url.Action("FirstStep", "Account", new { id = usr.AspNetUser.Id }));

                #region Fix prob de proxy en local sur l'URL

                var currentHost = "localhost:57061";
                var newhost = $"{GetIPAddressV4()}:90";
                url = url.Replace(currentHost, newhost);

                #endregion Fix prob de proxy en local sur l'URL

                var mailT = BuildMailMgr.MailInvit(new Business.Tools.Mail.ViewModels.MailInvit()
                {
                    UrlInvit = url,
                    FirstName = usr.FirstName,
                    LastName = usr.LastName,
                    Profile = usr.Profile,
                    HtmlMessage = htmlInvit,
                });

                mailT.To = usr.Email;
                MailMgr.SendMail(mailT);
            }
            else
            {
                model.Feedback.AddError("SendMailInvit User Not find ", " SendMailInvit User Not find => UserId : " + userId);
            }
        }

        #endregion MailInvit

        public static string GetIPAddressV4()
        {
            var res = "";
            IPHostEntry ipHostInfo = Dns.GetHostEntry(Dns.GetHostName());

            for (int i = 0; i < ipHostInfo.AddressList.Length; i++)
            {
                IPAddress ipAddress = ipHostInfo.AddressList[i];
                var ip = ipAddress.ToString();
                if (ip.Length <= 12)
                {
                    res = ip;
                }
            }
            return res;
        }
    }

Class names shouldn't be plural except for a few exceptions, and BaseViewModels does not seem to be such a case.

Avoid this kind of logic:

if (usr != null)
{
    // lots to do
}
else
{
    // one thing to do
}

Especially here: you could have done

if (usr == null)
{
    model.Feedback.AddError("SendMailInvit User Not find ", " SendMailInvit User Not find => UserId : " + userId);
    return;
}

and then the rest of the method could be the long list of other instructions.

I've never worked with ASP.NET MVC, so I don't know whether this is the usual way of doing things, but passing a model and then setting properties to give feedback -- e.g. model.Feedback.AddError("SendMailInvit User Not find ", " SendMailInvit User Not find => UserId : " + userId); -- looks really odd to me.

Especially since this class doesn't have anything to do with the UI; which means you're mixing up tiers.

Don't use regions.

This seems like a hacky solution:

var currentHost = "localhost:57061";
var newhost = $"{GetIPAddressV4()}:90";
url = url.Replace(currentHost, newhost);

Wouldn't it be much simpler to simply have the base URL as a value in a .config file? Especially considering what's happening in GetIPAddressV4().

If I were you I'd move the whole "compile the URL" logic to a separate method.

Avoid abbreviations like UrlInvit, htmlInvit etc. It only makes you code harder to read.


English-Russian irregular words tester Could you please offer some suggestions about my self learning project?

It is really hard-coded, but I have no idea what to do next. I'm using angular (only few features for now) and grunt + bower.

tester.html

    First form
    Second form
    Third form
    Translation

{{ verb }}

        <input ng-model="val[$index]" ng-hide="isActive($index)" type="text">

{{ verb }}

          {{ val[$index] }}

{{ verb }}

  Check

  Next

iwt.coffee

angular.module('IWTTester', [])

  .controller('IWTTesterController', [
    '$http'
    '$scope'
    ($http, $scope) ->

      $http.get 'js/words.json'
      .success (data, status, headers, config) ->
        console.log data, status, config, 'good'
        $scope.irregulars = data
        $scope.init()
      .error (data, status, headers, config) ->
        console.log data, status, config, 'bad'

      $scope.isActive = (formIndex) ->
        formIndex is $scope._activeForm

      $scope.isStage = (value) ->
        value is $scope._stage

      $scope.init = ->
        ROWS = $scope.irregulars.length
        COLS = $scope.irregulars[0].length

        create_counter = (module) ->
          i = 0
          -> (i++) % module
        rowCounter = create_counter(ROWS)
        colCounter = create_counter(COLS)

        $scope.val = []
        $scope._stage = 'input'

        update = ->
          $scope.verbs = $scope.irregulars[rowCounter()]
          $scope._activeForm = colCounter()
        update()

        $scope.isSuccess = (i) ->
          $scope.val[i] is $scope.verbs[i]

        $scope.isDanger = (i) ->
          $scope.val[i] isnt $scope.verbs[i] and not $scope.isActive(i)

        $scope.check = ->
          $scope._stage = 'validation'

        $scope.next = ->
          $scope.val = []
          update()
          $scope._stage = 'input'
  ])

  .directive('iwtTester', ->
    restrict: 'AE'
    templateUrl: 'templates/tester.html'
    controller: 'IWTTesterController'
  )

words.json

[
  [
    "be",
    "was",
    "been",
    "быть"
  ],
  [
    "bring",
    "brought",
    "brought",
    "приносить"
  ]
]

First of all, I would architecture the data in a more readable and flexible way:

[
  {
    infinitive: "be",
    past: "was",
    participle: "been",
    russian: "быть"
  },
 {
   ...
 },
...
]

That tells me as reader a lot and leaves vast room for future changes without breaking the rest of your code.

Further, if you want to make your code readable for others (or even for yourself after a while), I would recommend to use descriptive names for your functions and variables. When I read

<tr ng-show="isStage('input')">

I get no clue what the function isStage does from its name.
So I am forced to scan the rest of your code for its definition, which costs time. You could help me by giving it a name that would give me a rough idea without the need to go elsewhere.

Another thing I see is the use of $http inside controller, which is an anti-pattern. It sadly appears in many tutorials, yet it breaks one of the most fundamental principles of clean code - the single responsiblity principle. Your controller is responsible for gluing your scope (ViewModel layer) with your View, but not for pulling data from your Model layer. That is the job of a dedicated Service.


Simple login script with sha256 hashing I have been using this code for a while. I got it from a tutorial 5 years back. Now I'm wondering if it still is secure, or if it's time to find a new one? Maybe it has never been secure enough.

LOGIN

<?php
//Connect to database
require("include/config.php");

//Empty Variable
$submitted_username = '';

//Check if something is posted
if(!empty($_POST)){

    //Check if user exists
    $query = "SELECT * FROM usrs WHERE usr_email = :usr_email";
    $query_params = array(':usr_email' => $_POST['usr_email']);
    try{
        $stmt = $db->prepare($query);
        $result = $stmt->execute($query_params);
    }
    catch(PDOException $ex){ die("Failed to run query: " . $ex->getMessage()); }

    //Fetch result
    $row = $stmt->fetch();

    //Reset var
    $login_ok = false;

    //If usr exists, check password
    if($row){
        //Check if password matches
        $check_password = hash('sha256', $_POST['usr_password'] . $row['usr_salt']);
        for($round = 0; $round < 65536; $round++){
            $check_password = hash('sha256', $check_password . $row['usr_salt']);
        }
        if($check_password === $row['usr_password']){
            $login_ok = true;
        }
    }

//LOGIN OK
    if($login_ok){
        //UNSET VARS
        unset($row['usr_salt']);
        unset($row['usr_password']);

        //LOG USR LOGIN DATA
        $dateTime = date('Y-m-d H:i:s');
        //Create array
        $usr_data = array(
            ':usr_id' => $row['id'],
            ':usr_name' => $row['usr_fname']. " " . $row['usr_lname'],
            ':dateTime' => $dateTime,
            ':ip' => $_SERVER['REMOTE_ADDR']
        );

        //CREATE SESSION
        $_SESSION['usr'] = $row;

        //REDIRECT TO STARTPAGE
        header("Location: start.php");

    }
    else{
        echo "<script type='text/javascript'>alert('Fel uppgifter..');</script>";
        $submitted_username = htmlentities($_POST['usr_email'], ENT_QUOTES, 'UTF-8');
    }
}
?>

CONFIG

$options = array(PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8');
try { $db = new PDO("mysql:host={$host};dbname={$dbname};charset=utf8", $username, $password, $options); }
catch(PDOException $ex){ die("Failed to connect to the database: " . $ex->getMessage());}
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$db->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC);

//SET HEADER
header('Content-Type: text/html; charset=utf-8');

//START SESSION
if(!isset($_SESSION)){
    session_start([
        'cookie_lifetime' => 86400,
    ]);
}

//SET TIMEZONE
date_default_timezone_set('Europe/Stockholm');
?>

CHECK IF LOGGED IN

<?php

//CHECK IF USR IS LOGGED IN
if(empty($_SESSION['user'])){
    echo "<script>window.location = '/index.php'</script>";
    die("Redirecting to index.php");
}

There's a timing attack. To see whether a user has registered or not, I can try to login. If it fails fast, the user is not registered. If it takes time, the user is registered. To fix this information disclosure, calculate some dummy hash even if the user cannot be found in the database.

The SQL part is fine.

The $submitted_username calls htmlentities too early. The only correct time to call htmlentities is exactly at the point where you embed a text into an HTML snippet. That doesn't happen in your code, though. To fix this, rename the variable from submitted_username to submitted_username_html, to prevent it from being used in any other context. For example, code that reads db_insert($submitted_username_html) looks wrong enough to warrant a thorough code review.

Your code could benefit from being split into a few well-named functions. After doing that, the main code might read like this:

$logged_in_user = log_in($_POST);
if ($logged_in_user === FALSE) {
    // …
} else {
    // …
}

When you extract all the detailed code into a function, you can use early returns for all the error cases. And when you step through the code using a debugger, you can easily skip over all the details of the login process, if you are not interested in it.

In the config part, you should prefix all variable names with db_, to avoid confusing the username with the submitted_username. Or group them into an object called db_config. Then you can access it as $db_config['username']. There, the identifier username is appropriate since it is qualified by the word db_config, which makes it unambiguous.

The SHA-256 algorithm you use for hashing is not secure enough anymore since specialized hardware can compute it too fast, even with 65536 iterations. PHP has a built-in set of password hashing algorithms. Just use that instead of iterating on your own. Migrating from your custom password hashing to the PHP default will cost a bit of time and work, but it's worth the effort. And since you are not the first to do that, there's already plenty of documentation on it. Probably. Hopefully.


Integer Power C++ Function with base and exponent parameters I would like to know if this is a good solution for a function that has to computer the power of integers given base and exponent.
The solution must include solution for negative numbers.
I'm trying to learn C++ trough a Deitel book and I'm doing several exercises.
#include <iostream>
using namespace std;

double integerPower(int base, int exponent){
    double result = 1;
    if(exponent >=0){
    for (int i = 0; i < exponent; i ++){
        result *= base;
        }
    } else {
        exponent *= -1;
        for (int i = 0; i < exponent; i ++){
                result /= base;
                }
    }
    return result;
}

int main() {
    int base, exponent;
    double result;
    cout << "Insert the base: " << endl;
    cin >> base;
    cout << "Insert the exponent: " << endl;
    cin >> exponent;
    result = integerPower(base, exponent);
    cout <<"The result is: " << result << endl;
    return 0;
}

Use the standard library
If you are going to give the result as a double, then this function isn't really a pure integer power calculator. And in that case, just use std::pow(). It has overloads for integer exponents that the compiler can optimize for.
A pure integer power function
If you just want to work purely on integers, then the typical algorithm to calculate a number raised to an arbitrary power efficiently is by recognizing that, for example, \$x^4 = ((x * x) * (x * x))\$, and so you can calculate \$y = x * x\$, and then \$x^4 = y * y\$. This only needs two multiplications instead of 3. So basically, you can divide and conquer the problem:
int integerPower(int base, int exponent) {
    if (exponent == 0)
        return 1;

    int result = integerPower(base, exponent / 2);
    int result *= result;

    if (exponent & 1)
            result *= base;

    return result;
}

The above doesn't work for negative exponents, but then again that is not very useful if the result is just an integer.


Counting numbers with specific digits The challenge:

  Suppose you are inhabitant of a planet where 1, 7, and 9 are lucky digits. A lucky number for you is a number that contains only your lucky digits in it. For ex: 1, 79, 911, 9917 etc., are lucky, where as 5, 172, 93, 170 are not. Given a integer N, count the number of lucky numbers in the range 1 to N.
  [Constraints : 1 ≤ N ≤ 10^12. Time limit : 3s]

How can I make this code faster? As of now it exceeds way beyond 3 secs.

#include<stdio.h>
#include<time.h>
int lucky(long long int num)
{
    while(num>0)
    {
        if(!(num%10==1 || num%10==7 || num%10==9))
            return 0;
        num = num/10;
    }
    return 1;
}
int main()
{
    clock_t tic = clock();
    long long int num,i,count = 0;
    scanf("%lld",&num);

    for(i=1;i<=num;i++)
    {
        count += lucky(i);
    }

    printf("%lld\n",count );
    clock_t toc = clock();

    printf("Elapsed: %f seconds\n", (double)(toc - tic) / CLOCKS_PER_SEC);
    return 0;
}

The trick to this problem is that you shouldn't check each number. It takes way too long.

An important thing I noticed is that there's only 3 digits that count. If N is 10, then there are 3 lucky numbers, because there are 3 lucky digits. And if N is 100, then there are 12 lucky numbers, because there are 3*3 combinations of lucky numbers. But there's 12 in total, because we write '01' and '07' and '09' as 1, 7 and 9.

So by looking at each digit of N, we can solve the problem faster.

Have a look at this table, where I wrote all the lucky numbers up to N = 1000.

  1     7     9 = 3
 11    17    19
 71    77    79
 91    97    99 = 12
111   117   119
171   177   179
191   197   199
711   717   719
771   777   779
791   797   799
911   917   919
971   977   979
991   997   999 = 39

Take N = 1000 with lucky digits 2, 7, 9.

This one is easy. 1000 means we score all the 3 digit numbers (which is 3 + 3^2 + 3^3). The first digit does not match a lucky digit, so we are done.

N = 1000 with lucky digits 1, 7, 9 is a tad harder to explain: First, we get all the 3 digit numbers because N is a 4 digit number. Next, since the first digit is a lucky digit, we need to score all the points you can make with the next digit. But because the next digit is not equal to or greater than a lucky digit, that score is 0.

N = 1150 with lucky digits: again, score 39 for having 4 digits. Next, score 150, but without taking the free score for having 3 digits. Since the first digit of 150 is a lucky digit but not greater than a lucky digit, you get 0 times all the 2 digit lucky numbers. So 0. 50, however, starts with 5. 5 is greater than 1, but smaller than 7, so you get 1 * the amount of lucky numbers with 1 digit. N = 1150, score is 42.

N = 1750:

39 points for being 4 digits (3 + 3^2 + 3^3)
1 is not bigger than any lucky digits, so 0 * 3^3
1 is a lucky digit, so continue with the next digit
750 starts with 7, 7 is bigger than 1 but not bigger than 7 or 9, so 1 * 3^2 = 9 points here
7 is a lucky digit, so continue with the next digit
50 starts with 5, 5 is bigger than 1 but not bigger than 7 or 9, so 1 * 3^1 = 3 points here

Makes for a total of 39 + 0 + 9 + 3. = 51.

N = 11:

3 points for being 2 digits (3)
1 is not bigger than any lucky digits, so 0 * 3
1 is a lucky digit, so continue with the next digit
it's the last digit, so count each lucky digit the last digit is equal to or greater than as 1 points - in this case, that's only '1', so 1 point it is

So for N = 11, 4 lucky numbers.

N = 2000:

39 points for being 4 digits (3 + 3^2 + 3^3)
2 is bigger than 1 lucky digit, so 1 * (3^3) = 27
2 is not a lucky digit, so end there

makes 39 + 27 = 66.

Here's a table to check:

   1     7     9 = 3
  11    17    19
  71    77    79
  91    97    99 = 12
 111   117   119
 171   177   179
 191   197   199
 711   717   719
 771   777   779
 791   797   799
 911   917   919
 971   977   979
 991   997   999 = 39
1111  1117  1119 - 3
1171  1177  1179 - 6
1191  1197  1199 - 9
1711  1717  1719 - 12
1771  1777  1779 - 15
1791  1797  1799 - 18
1911  1917  1919 - 21
1971  1977  1979 - 24
1991  1997  1999 = 27 + 39 = 66

So, why does this matter?

Because this algorithm I just explained is about \$O(log n)\$. That is, N = 1000 takes 4 times as long as N = 1. N = 10^12 would take 12 times as long as N = 1.

Your algorithm is \$O(n)\$,  as it checks each number. That means N = 1000 takes a thousand times (1000) as long as N = 1. This means that for the higher cases, your code takes too long, as N = 10^12 means your code takes 1.000.000.000.000 times longer to complete than for N = 1.


First Time HTML5/CSS Site I am working on my first HTML5/CSS web site and, like all of my first-time projects, they end up cumbersome, crude, and hard to work with when changes need to be made later in the life cycle. I am attaching some code with a small amount of HTML and CSS. I have read up on CSS and HTML and, after quite alot of effort, have ended up with this. Please look at the code and fill me in on any inefficiencies that I may have introduced due to lack of experience. I do feel like I am doing a little bit too much manually right now and this is just a basic web site. Any opinion will be appreciated. FYI: I have left the colors a bit funny in order to easily see changes to the web site.

/*
 Created on : Mar 24, 2014, 9:05:35 AM
 Author     : meggleston
*/
#listheader
{
 position:absolute;
 top:90px;
 width:100%;
 background:cadetblue;
 height: 80px;
 text-align: center;
}

#header
{
 top: 0;
 position: absolute;
 height: 90px;
 width: 100%;
 background: #007400;
 margin: 0px;
 padding: 0px;
}

#logo
{
 position:absolute;
 background-image:url(../assets/pics/logomain.png);
 background-repeat: no-repeat;
 background-re:no-repeat;
 background-position: left top;
 background-size:100px 90px;
 height: 100;
 width: 100;
}

body
{
 margin: 0px;
 padding: 0px;
}

#site1Container
{
 position:absolute;
 background-color: #cd0a0a;
 top:170px;
 width: 100%;
 height: 180px;
}

#site2Container
{
 position:absolute;
 background-color: bisque;
 top:350px;
 width: 100%;
 height: 180px;
}

#site3Container
{
 position:absolute;
 background-color: darkgoldenrod;
 top:530px;
 width: 100%;
 height: 180px;
}

#site1logo
{
 position:absolute;
 top:20%;
 background-image:url(../assets/pics/logo1.png);
 background-repeat: no-repeat;
 background-re:no-repeat;
 background-position: left top;
 background-size:100px 100px;
 height: 100%;
 width: 100;
}

#site2logo
{
 position:absolute;
 top:20%;
 background-image:url(../assets/pics/logo2.png);
 background-repeat: no-repeat;
 background-re:no-repeat;
 background-position: left top;
 background-size:100px 100px;
 height: 100;
 width: 100;
}

#site3logo
{
 position:absolute;
 top:20%;
 background-image:url(../assets/pics/logo3.png);
 background-repeat: no-repeat;
 background-re:no-repeat;
 background-position: left top;
 background-size:100px 100px;
 height: 100;
 width: 100;
}

#site1desc
{
 position: absolute;
 left:150px;
 top: 20%;
}

#site2desc
{
 position: absolute;
 left:150px;
 top: 20%;
}

#site3desc
{
 position: absolute;
 left:150px;
 top: 20%;
}

  <title>Site Title</title>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <link rel="stylesheet" href="css/basecss.css"/>

List of Web Sites Header

Site description goes here.

Site description goes here.

Site description goes here.

Semantically, I would suggest using HTML5 elements more.

For example, instead of...

Use instead: (the ID can stay if you want it to)

This would change your CSS from #header {} to just header {}

It is common to see multiple  tags in complicated templates.  So something like this might be appropriate if that is the case...

CSS would be:

header.site-header {}

Consistency, consistency, consistency.

Most of your class and ID names are completely lowercase.  However a few classes use camelCase like: #site3Container

Make your naming conventions consistent.  Either use all camelCase or use all lowercase or all UPPERCASE etc.

Typically, I use lowercase for HTML and CSS (faster to write).  I use camelCase for PHP.  This is no rule however.

Positioning

Absolute positioning has its uses.  It's almost never used for basic layout however.

Your exact layout can be achieved without the use of ANY position: absolute;.  It is my general recommendation to avoid position: absolute; at almost all times.  If you are finding you need to use it for something, chances are you don't actually need to, you just don't know how else to do it yet.  Consult someone.

Here is a JSFiddle of your current html and css

Here is a JSFiddle demonstrating the same layout, without positioning.

*Note that the description text now has a maximum width of 960px (common site width, though 1140 is becoming more common).  The description divs are also center so there will always be equal space on left and right sides.

My modifications above also feature one particular additional CSS selector:

* { margin: 0; padding: 0; }

The asterisk * is a wildcard, it essentially means ANY element.  The selector above resets the margin and padding of every element to 0.  This is usually done via something called a "reset stylesheet".  Which leads me to my next subject...

Reset Stylesheets

Reset stylesheets contain CSS code which standardizes what your website will look like across all browsers.  It sets all padding and margins to 0 for instance, so that no default styling is added to your site that you potentially, or even probably, don't want.

A quick Google search lands us at this website: http://www.cssreset.com/

There, you can see many popular reset stylesheets, all with their own different sort of caveats and specialities.  I personally recommend Normalize.css  It keeps and standardizes some typical styles you may not necessarily want to reset to 0.  It also contains some standard typography styling, so your text looks great without you having to do any work.  Reset stylesheets should always be the very first .css file you link.  So you would include yours like this for instance:

    <title>Site Title</title>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <link rel="stylesheet" href="css/reset.css"/>
    <link rel="stylesheet" href="css/basecss.css"/>

Classes over ID's

An ID is specific, they can be used once.  It IS this "thing" and it is only ever this "thing".  A class can be just as specific, or as broad as you would like.  A class can be applied to multiple elements on the same page.  An example, expanding from the JS Fiddles above, would be to combine the styles for the description divs.

We start with this CSS:

#site1desc
{
    width: 100%;
    max-width: 960px;
    margin: 0 auto;
}

#site2desc
{
    width: 100%;
    max-width: 960px;
    margin: 0 auto;
}

#site3desc
{
    width: 100%;
    max-width: 960px;
    margin: 0 auto;
}

By modifying the HTML so that every
 is now this:
 We can drastically cut down on our total CSS code.  Our full HTML now looks like the below...

Site description goes here.

Site description goes here.

Site description goes here

The only CSS we need changes from the 3 selectors above to just this:

.contentdesc
{
    width: 100%;
    max-width: 960px;
    margin: 0 auto;
}

JSFiddle Demo of the above

***Note: CSS classes are referenced with a period  .className is different from #idName


Most efficient FizzBuzz solution in Ruby I would appreciate any feedback concerning my analysis of the most efficient FizzBuzz solution programmed in Ruby. I submit that a Case-statement solution is more efficient than utilizing a ConditionalIf-statement in my recent blog-post within my conclusion provided thusly:

  Consequently, the predictability pattern is the reason why a "Case" statement, or branchIf, is optimal and less expensive than a "Conditional If" statement, as clarified by Igor Ostrovsky's Blog Post: Fast and Slow If-Statements: Branch Prediction in Modern Processors

    "If the condition is always true or always false, the branch prediction logic in the processor will pick up the pattern. On the other hand, if the pattern is unpredictable, the if-statement will be much more expensive."

  Back to my optimized FizzBuzz solution- when the "Case" statement processes a number initializing the method, the constraint or case stops calculating when the condition is satisfied, and it will not continue to verify the constraint by branching unless divisible as it were for an IF/ELSIF construction, which saves time, performing faster, as the best solution possible, ultimately proving Danielle Sucher's point:

    "I'd expect if/elsif to be faster in situations where one of the first few possibilities is a match, and for case to be faster in situations where a match is found only way further down the list (when if/elsif would have to make more jumps along the way on account of all those branchUnlesses)."

  Furthermore, a real programmer can impress an interviewer by asking if the range invoked will be consecutive or random.  Although a "Case" statement solution for FizzBuzz is generally more efficient, it will certainly be faster for a random range of numbers called.

I've approached several academics and professionals alike but nobody has challenged, so I would like to broaden the forum for additional contribution:

require 'benchmark'

def fizzbuzz(array)
 array.map!{ |number|
 divisibleBy3 = (number % 3 == 0)
 divisibleBy5 = (number % 5 == 0)

 case
   when divisibleBy3 && divisibleBy5
    puts "FizzBuzz"
   when divisibleBy3
     puts "Fizz"
   when divisibleBy5
     puts "Buzz"
   else
   puts number
  end
 }
end

puts Benchmark.measure{fizzbuzz(Array(1..10000))}
puts "fizzbuzz"
# puts RubyVM::InstructionSequence.disasm(method(:fizzbuzz))

def super_fizzbuzz(array)
 array.map! { |number|
  if(number % 3 == 0 && number % 5 == 0)
   puts "FizzBuzz"
  elsif(number % 3 == 0)
   puts "Fizz"
  elsif (number % 5 == 0)
   puts "Buzz"
  else
  puts number
 end
 }
end

puts Benchmark.measure{super_fizzbuzz(Array(1..10000))}
puts "super_fizzbuzz"
# puts RubyVM::InstructionSequence.disasm(method(:super_fizzbuzz))

# Additional Documentation
 # https://en.wikipedia.org/wiki/Assembly_language
 # underTheHood => http://ruby-doc.org/core-2.0.0/RubyVM/InstructionSequence.html
 # http://ruby-doc.org/stdlib-2.0.0/libdoc/benchmark/rdoc/Benchmark.html

Upon executing the above, terminal-output correspondingly revealed the following when scraped with grep:

 Desktop  ruby fizzbuzz.rb | grep 0.0
 0.010000   0.000000   0.010000 (  0.010018)
 0.010000   0.000000   0.010000 (  0.011353)

Using a destructive update like map! is weird, the user does not expect his array to be mutated. I suggest one of two possibilities:

Replace map! with each and leave the rest of the code as is.
Replace map! with map, remove puts and handle printing out of the function. Adding a join("\n") may be needed.

The first is more obvious, the second one only has one responsability.

As a minor note, it is common style to use do end for multiline blocks, not {}.