Updating Pivot Tables in Excel I'm writing some code for someone to update a pivot table field based on the contents entered in a particular cell.

I am unfamiliar with VBA and programming for excel but I followed some tutorials and would like if someone here could look over what I've done just to see if everything looks ok before I give it to them as I don't have access to their excel worksheet.

Private Sub Worksheet_SheetChange(ByVal Sh As Object, ByVal Target As Range)
If Not Intersect(Target, Application.Range(B8)) _
    Is Nothing Then
        UpdatePivotFieldFromRange _
        B8, B3, Outfield
End If
End Sub

Public Sub UpdatePivotFieldFromRange(RangeName As String, Fieldname As       String, PivotTableName As String)

Dim rng As Range
Set rng = Application.Range(RangeName)

Dim pt As PivotTable
Dim Sheet As Worksheet

For Each Sheet In Application.ActiveWorkbook.Worksheets
    On Error Resume Next
    Set pt = Sheet.PivotTables(PivotTableName)
Next
If pt Is Nothing Then GoTo Ex
On Error GoTo Ex

pt.ManualUpdate = True
Application.EnableEvents = False
Application.ScreenUpdating = False

Dim Field As PivotField
Set Field = pt.PivotFields(Fieldname)
Field.ClearAllFilters
Field.EnableItemSelection = False
SelectPivotItem Field, rng.Text
pt.RefreshTable

Ex:
pt.ManualUpdate = False
Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub

Public Sub SelectPivotTableItem(Field As PivotField, ItemName As String)
Dim Item As PivotItem
For Each Item In Field.PivotItems
    Item.Visible = (Item.Caption = ItemName)
Next
End Sub

So, let's start with the Worksheet event and what you're trying to do. You only want to trigger the pivot update if the changed cell is B8. There's no need to intersect, just check the target's address.

Private Sub Worksheet_SheetChange(ByVal Sh As Object, ByVal Target As Range)
    If Target.Address = "B8"
            UpdatePivotFieldFromRange "B8", "B3", Outfield
    End If
End Sub

Note that I properly indented the code and got rid of that horrid line continuation.

Now, we can move onto UpdatePivotFieldFromRange. Kinda...

You're immediately recreating the target range at the beginning of this sub, so just pass Target in as a range instead of a string. That changes the event to this:

Private Sub Worksheet_SheetChange(ByVal Sh As Object, ByVal Target As Range)
    If Target.Address = "B8"
            UpdatePivotFieldFromRange Target, "B3", Outfield
    End If
End Sub

And your declaration of UpdatePivotFieldFromRange to this:

Public Sub UpdatePivotFieldFromRange(rng As Range, Fieldname As String, PivotTableName As String)

But that's still not right. There's no reason for this to be public, declare it private instead. I also don't like the name rng. It's not meaningful, but I'll leave it to you to give it a meaningful name.

There is so much wrong here I'm not sure where to start.

For Each Sheet In Application.ActiveWorkbook.Worksheets
    On Error Resume Next
    Set pt = Sheet.PivotTables(PivotTableName)
Next
If pt Is Nothing Then GoTo Ex

There's no reason to call Application. ActiveWorkbook.Worksheets works just fine.
But you're begging for bugs. If the user clicks on another workbook while this is running, the sub is suddenly working on a different workbook. Use a Workbook typed variable instead.

Dim book As Workbook
Set book = ActiveWorkbook

For Each sheet In book.Worksheets

The next thing to note is you're setting On Error GoTo Next at each iteration of the loop. It should be moved outside of the loop.
But, you shouldn't ever use On Error GoTo Next. Seriously, Never do this. You're telling vba to simply swallow the error. It's analogous to having an empty catch block. Find a way to check to see if the pivot table exists instead.
Finally, you're using GoTo to jump to your error handler. You should also never do this. It's much better to Raise an Error.

That said, I do like that you're always ensuring that ScreenUpdating gets turned back on. (Well, you would be, if you set OnError GoTo Ex directly upon entering the subroutine.)

Again, there's no reason for a private function to be declared Public. Look into scoping and declaration in vba. It's important. If these were functions instead of Subs, they would be available as user formulas.

Secondly, do you really intend for this to hide every field except the one you're looking for? I doubt it.

Public Sub SelectPivotTableItem(Field As PivotField, ItemName As String)
Dim Item As PivotItem
For Each Item In Field.PivotItems
    Item.Visible = (Item.Caption = ItemName)
Next
End Sub

I have a feeling that what you really meant to do was turn the particular field on/off like this.

Public Sub SelectPivotTableItem(Field As PivotField, ItemName As String)
    Dim Item As PivotItem
    For Each Item In Field.PivotItems
        If Item.Caption = ItemName Then
            Item.Visible = True
        End If
    Next
End Sub


Placeholder text for HTML search field I'm not a great JS coder so I need your expertises to look into my code (although nothing very special and it works fine) to see if it is good practise or not because I heard that dodgy JS coding might slow down the site especially when it comes to reading mouse or user events.

function textbox_clear_default(form_field, mouse_event)
{
    var field_data = (document.getElementById(form_field).value).replace(/ /gi, '');
    var lower_case = field_data.toLowerCase();

    if (form_field == 'text_site_search')
    {
        if (lower_case == 'sitesearch')
        {
            document.getElementById(form_field).value = '';
        }
    }

    if (mouse_event == 'onclick')
    {
        if (lower_case == 'sitesearch')
        {
            document.getElementById(form_field).value = '';
        }
    }

    if (mouse_event == 'onblur')
    {
        if (lower_case == 'sitesearch' || lower_case == '')
        {
            document.getElementById(form_field).value = 'Site Search';
        }
    }
}
<input id="text_site_search" type="text" name="text_site_search" value="Site Search" onclick="textbox_clear_default(this.id, 'onclick')" onblur="textbox_clear_default(this.id, 'onblur')" />

This is the way I would approach it. Live demo here: http://jsbin.com/iHEgalU/3/edit
Sure, bad js can slow your site down. Something like this is going to be pretty fast no matter what you do (unless you go out of your way to make it slow), but better is better and I say it's good to write the best, fastest, and most totally rock-sauce code you possibly can. So here goes.

Some people may consider it overkill considering the simplicity of your task, but I prefer writing in a manner that allows me to easily expand my script while keeping everything tidy and following good practices. You can extract the improved logic and use it in whatever style you like. I have commented this quite a bit. Refer to the live demo where there are no comments if you'd rather see it that way.

<input id="text_site_search" type="text" name="text_site_search" value="Site Search">

Avoid inline event functions! Notice that I have removed them from the html and put them in the js where they belong! This is much cleaner! HTML can't "do" things, so it doesn't need to know about click functions.

var myStuff = {
  options: { //these will be defaults, used if a parameter is not passed to init()
    sel: '',
    placeholder: 'My Placeholder'
  },
  init: function(sel, placeholder) { //this will kick everything off, that's all we need to run, you can pass options in like this or by object. I tend to use objects, but no need here.
    if (sel) { this.options.sel = sel }
    if (placeholder) { this.options.placeholder = placeholder }
    //there are several ways you could replace the passed in options...this is just an example that's ok for this case. jQuery has a method called $.extend, for example

    var obj = this; //cache a reference to "this" which currently refers to "myStuff"
    var inputSearch = document.getElementById(sel); //search the dom for the element and cache the reference
    //no need to keep searching the dom for an element. Search once and store the result.

    inputSearch.addEventListener('focus', function() {
      if (this.value === obj.defaultValue) { //"this" refers to the element when inside an event handler
        this.value = '';
      }
    });

    inputSearch.addEventListener('blur', function() {
      if (this.value === "") {
         this.value = obj.defaultValue;
      }
    });
  }
};

myStuff.init('text_site_search', 'Site Search');

The options thing here isn't really all that helpful given the simplicity. If this is all this will ever do, I'd probably just use the parameters directly (just use sel where you need it), but I wanted to demonstrate the concept.

Note: IE8 and lower do not support addEventListener. Look up the conditional that deals with that issue if needed.

The funny thing about this whole script is that it is remarkably similar to html5 placeholders, though older browsers won't support that.

<input type="text" name="text_site_search" placeholder="Site Search">


Toggle functions for drop down for a clock time widget I'm fairly new to JQuery/JavaScript and I'm trying to refactor the following code:

$('#hour').click(function() {
  $('#clockHours').slideToggle();
  $('#clockMinutes').hide();
});

$('#minutes').click(function() {
  $('#clockMinutes').slideToggle();
  $('#clockHours').hide();
});

//print clicked onto views
$('.timeSelect').click(function() {
  var text = $( this ).text();
  $('#hourView').val( text );
});

$('.minuteSelect').click(function() {
  var text = $( this ).text();
  $('#minuteView').val( text );
});

I haven't had much luck figuring out how to improve the code. Here is a CodePen of the current working code and here's what I have tried.

There a few simple things you could do to help clean up this code.  The first thing I always recommend is to create a closure for your script using an IIFE.  This allows your code to run in its own scope and keeps everything out of the global scope. You can also pass in jQuery to the function expression and safely refer to it as $.

(function($, undefined ) {
  // pass in undefined for very old (ES3) browsers
  // code goes here
})( jQuery );

The next thing you should consider is caching your selections.  Since accessing the DOM is one of jQuerys slowest operations, you want to minimize this as much as possible. Anything you refer to more than once, you should cache.

var $min = $('#clockMinutes');
var $hr = $('#clockHours');

I prefix all of my jQuery selections with $ so I know they are jQuery objects.

You should also start using on for defining your events instead of the shorthand of click.  Behind the scenes, click uses on so just skip the middle man.

$min.on('click', function() {
  //code here
});

You might also want to consider DRYing your code.  Since the click functions on both the clockMinutes and clockHours are so similar, you can combine them and just pass in the "main" element you want to work on.  For example:

function updateScreen( $el ) {
  $el.slideToggle();
  if ( $el === $hour ) {
    $min.hide();
  } else {
    $hour.hide();}
  }
}

Here is an updated code sample based on the above.  Note: I call an init function on document.ready to kick off the functionality.

(function( $, undefined ) {
  //place holder variables for our selections
  var $min, $hour, $minView, $hrView;

  function init() {
    $min = $('#clockMinutes');
    $hour = $('#clockHours');
    $minView = $('#clockMinutes');
    $hrView = $('#minuteView');
    $('#hour').on('click', function() { updateScreen($hour); });
    $(' #minutes').on('click', function() { updateScreen($min); });
    $('.timeSelect').on('click', function() { updateView($hrView); });
    $('.minuteSelect').on('click', function() {updateView($minView);});
  }

  function updateView( $view ) {
    $view.val( $(this).text() );
  }

  function updateScreen( $el ) {
    $el.slideToggle();
    if ( $el === $hour ) {
      $min.hide();
    } else {
      $hour.hide();
    }
  }

  $(function() {
    init();
  });

})(  jQuery );

Hope you find this helpful. Let me know if you have any questions.


Design Tic tac toe game Today I had an interview, where I was asked to solve this question:

Design Tic tac toe game

Game rules:

  Design a simple tic tac game with 2 modes for 3*3 matrix. At the
  beginning, ask if single player or two player mode has to be played
  (more details on the mode below).

  The players input position on matrix by inputting 2 integers, like:

0,0    0,1    0,2

1,0    1,1    1,2

2,0    2,1    2,2

  If anywhere player gives incorrect input, he has to be prompted to
  give the right value again.

  Print the updated matrix after every input. Example: after 3 moves,
  one game might look like

1    1    _

2    _    _

_     _    _

  Single player mode

  In this mode, player plays against the computer.
  You’ve to first assign 0’s or  X’s to the Player. X’s play first. The
  computer can randomly place its assigned symbol at any available
  position within the matrix.

  Two player mode

  In this mode, two players play against each other.

  The game ends when either player wins, or there are no moves left
  (draw). At the end of game, print the final result Ask if the player
  wants to play another game, and start a new game/end the game
  accordingly.

I got the feedback that complexity could be improved, How to improve it?
and how to make it more readable

Code goes below:

def get_input(mat, player):
    input_value = map(int, raw_input().split(','))
    while(validate_input(mat, input_value) == False):
        print "not correct input"
        input_value = map(int,raw_input().split(','))
    mat[input_value[0]][input_value[1]] = player
    return input_value

def validate_input(mat, input_value):
    x = input_value[0]
    y = input_value[1]
    if x not in [0,1,2] or y not in [0,1,2]:
        return False
    if mat[x][y]:
        return False
    return True

def switch_play(player):
    player = 2 if player==1 else 1
    return player

def print_game(mat):
    print mat

def get_random(mat):
    input_value = [random.randint(0,2)] *2
    while(validate_input(mat, input_value) == False):
        print "not correct input"
        input_value = [random.randint(0,2)] *2

    mat[input_value[0]][input_value[1]] = 2
    return input_value

def game_completed(mat):
    if not game_won(mat):
        for i in xrange(3):
            for j in xrange(3):
                if not mat[i][j]:
                    return False

    return True

def game_won(mat):
    for i in xrange(3):
        row = mat[i]
        if len(set(mat[i])) == 1 and set(mat[i]) != {0}:
            print 'won the game:player', mat[i][0]
            return True
    for i in xrange(3):
        col = [row[i] for row in mat]
        if len(set(col)) == 1 and set(col)!={0}:
            print 'won the game:player', mat[0][i]
            return True
        dia1 = [mat[i][i] for i in xrange(3)]
        dia2 = [mat[2-i][i] for i in xrange(3)]
        if (len(set(dia1)) == 1 and set(dia1)!={0}):
                print 'won game: player', mat[0][0]
                return True
        if (len(set(dia2)) == 1 and set(dia2)!={0}):
                print 'won game: player', mat[2][2]
                return True
        return False

print "hoose player 1 your type, X or O?"
p1 = raw_input()
p2 = 'O' if p1=='X' else 'X'
player = 1
mat = [[0,0,0], [0,0,0], [0,0,0]]
computer_playing = False
while True:
    if computer_playing and player==2:
        get_random(mat, player)
    else:
        get_input(mat, player)
    print_game(mat)
    if game_completed(mat):
        print "game is completed"
        break
    player = switch_play(player)

I think your interviewer meant that you can improve checking the winning situation and do it in \$O(1)\$ time by keeping track of the scores in rows, columns and both diagonals (total 8 variables for 3x3 board):

[row1, row2, row3, column1, column2, column3, diagonal, anti-diagonal]

Then, you can increment these value by 1 in case of "player 1" and decrement by one in case of "player 2". This will lead to only checking these 8 values (or you can also use the fact that you know where the last move took place) for 3 or -3 for a winning situation. You can find out more about this idea here.

Or, you can go over all the possible winning combinations like suggested here or here.


Queries to get unpaid and unpaid invoices Here is my code:
add_hook('ClientAreaPageInvoices', 1, function($vars) {
    $user_id = $_SESSION["uid"];
    $today = Carbon::now()->format('Y-m-d');
    $query_1 = Capsule::table('tblinvoices')
        ->select(Capsule::raw('SUM(total) as total_aberto'))
        ->where(array('userid' => $user_id, 'status' => 'Unpaid'))
        ->get();
    $query_2 = Capsule::table('tblinvoices')
        ->select(Capsule::raw('SUM(total) as total_vencido'))
        ->where(array('userid' => $user_id, 'status' => 'Unpaid'))
        ->whereDate('duedate', '<=', $today)
        ->get();
    $query_3 = Capsule::table('tblinvoices')
        ->select(Capsule::raw('SUM(total) as total_pago'))
        ->where(array('userid' => $user_id, 'status' => 'Paid'))
        ->get();
    $array_1 = json_decode(json_encode($query_1), True);
    $array_2 = json_decode(json_encode($query_2), True);
    $array_3 = json_decode(json_encode($query_3), True);
    $totalAberto = $array_1[0]['total_aberto'];
    $totalVencido = $array_2[0]['total_vencido'];
    $totalPago = $array_3[0]['total_pago'];
    return array ('totalAberto' => $totalAberto,
                  'totalVencido' => $totalVencido,
                  'totalPago' => $totalPago);
});

Is there any way to simplify the entire code in general?
It works well - I just want to learn other practices to make this type of consultation in a more optimized, and simplified way.

There seems to be a section in your code that you repeat three times. This can probably be placed in a separate function. Something like this:
function getInvoiceTotal($status, $onlyOverdue = FALSE)
{
    $query = Capsule::table('tblinvoices');
    $query->select(Capsule::raw('SUM(total) as total'))
          ->where(['userid' => $_SESSION['uid'],
                   'status' => $status]);
    if ($onlyOverdue && ($status == 'Unpaid')) {
        $query->whereDate('duedate', '<=', Carbon::now()->format('Y-m-d'));
    }
    return $query->get()->total;
}

add_hook('ClientAreaPageInvoices', 1, function($vars) {
    return ['totalAberto'  => getInvoiceTotal('Unpaid'),
            'totalVencido' => getInvoiceTotal('Unpaid', TRUE),
            'totalPago'    => getInvoiceTotal('Paid')];
});

Regretably I don't have Laravel installed so I cannot test this code, but I think the general idea is clear. Perhaps someone with more knowledge of Laravel can add to this answer?
Some notes:

I changed array(....) to [....].
I didn't understand the json_decode(json_encode()) bit. Perhaps to go from objects to arrays? I don't see what's wrong with using the actual objects. Here's where I have most doubt that my code is correct, since I haven't tested it.
I left out a lot of variables you used. Using variables, with good names, can make code easier to read, but in really good code, when a variable is only used once, it shouldn't be necessary.
I strongly advise you to write your code in English only. I had to find out what the meaning of 'Aberto', 'Vencido' and 'Pago' is. Not every programmer speaks Protuguese.


Scrolling event being fired too many times This code works and does the job, but it's poor, rudimentary and has some issues with the scrolling event. Can somebody help me to optimize the scrolling event?
Prototype
Structure and desired behaviour:

There are two containers, .layout__left (red) and .layout__right (green).
The green container will have the main content and will scroll normally.
The red container will have the navigation so it will have less content and should scroll up to a certain point only, until the menu reaches the border of the page.

This is what I am doing:

First both containers scroll normally.
When scrolling down the red container, if the scroll position + window size is bigger than the li:last child position (couldnt think of anyway to detect the "bottom"), then the container has a position fixed.
When scrolling up, the fixed position is disabled and when the menu reaches the top of the document, the position is fixed again, and then removed.

As I said, it does the job, but, it's slow and has glitches while scrolling. I think it's because I am doing on the scroll event on every pixel change.
I tried throttling the function call, but the code doesn't function at all:
    $(window).on('scroll', function() {
    $.throttle(100, sectionScroll));
    });

    sectionScroll = function() {
    var lastScrollTop = 0, delta = 5;
    var left = $('.layout__left');
    var right = $('.layout__right');
    var scroll = $(this).scrollTop();
    var viewport = $(window).height();
    var lastChild = $('.navigation > ul > li:last-child').offset().top;

    if(Math.abs(lastScrollTop - scroll) <= delta)
    return;

    if (scroll > lastScrollTop){
    // SCROLL DOWN
    left.removeClass('top absolute');

    // if the last item of the left is visible
    if (  (scroll + viewport) > (lastChild + 30)  ) {
    // make the left fixed
    left.addClass('fixed bottom');
    console.log("bottom!");
    }

    // SCROLL UP
    } else {
    // remove fixed class from left
    left.removeClass('fixed bottom');
    left.addClass('absolute bottom');

    if ( scroll <= left.position().top   ) {
    left.removeClass('bottom absolute');
    left.addClass('top fixed');
    left.removeClass('top fixed');
    console.log("top!");
    }
    }

    lastScrollTop = scroll;
    };

Any feedback?

Bug!
You have a small bug in your code:
$.throttle(100, sectionScroll));

See it? You have one-too-many parentheses. This line should be:
$.throttle(100, sectionScroll);

Debounce
You say in your title that the scrolling event is being fired too many times.
Well, this makes perfect sense: as long as the user has their mouse over the scroll bar and is moving the bar ever-so-slightly, the scroll event is going to try and fire as quick as it can.
Even for the tiniest movement, this event could fire over 50 times.
However, there is a simple way to prevent your code from trying to run that many times: a debounce.
A debounce is when you setup a variable to prevent code (typically an event) from running while another piece of code (often the same code) is still running.
Every time this scroll event fires, a new thread is going to be created with the event code running in it; a debounce would prevent these from running at the same time.
It is very simple to create a debounce; you just need a variable and a conditional.
Here is what the basic design would look like:
var debounce = true;
function foo() {
    if(debounce) {
        debounce = false;
        [code]
        debounce = true;
    }
}

The first line in the conditional would stop any other thread from running because they are all sharing a single debounce variable.
Now, all you need to do to apply this to your code would be to treat foo in the example as your sectionScroll function, create the debounce variable, and then encase the function code in the if statement.
Then, after adding the lines for setting and un-setting the debounce, you should be good to go.

The above is really the meat of my review, everything else is just miscellaneous.

Indentation
You have none! Which, in my opinion, is better than having bad indentation.
Luckily for you there is a handy-dandy tool that you probably already know about: JSFiddle. At the very top of the page is a button labeled "TidyUp".

Paste your code into the "JavaScript" code box.
Give that "TidyUp" button a smack.
???
Profit!

It's really as simple as that: using that "TidyUp" button will fully and properly indent your code.
However, don't get too used to this: you want to be able to tell right from wrong without having to consult something else.
Study the changes that that button made to your code. You may want to go back and forth a couple of times. And, read other JavaScript code that has proper indentation. Over time, you should be able to develop a strong sense of what is right, and what is wrong.
Note: I actually had to use this tool in order to review your code; I couldn't read it otherwise!


Beginners project: calculating area This was my second project as a beginner in Java programming. Please provide feedback/ tips on my program.

import java.util.Scanner;

public class Area {
    public static void main(String[] args) {

        Scanner Area = new Scanner (System.in);

        double length, width,totalArea, height;
        String shape, unit;
        int loop = 0;

        while (loop == 0){

        System.out.println("Which shape do you want to find the area?");
        shape = Area. next();

        switch (shape) {

        case "rectangle":
            System.out.println("Please enter the leghth of the rectangle: ");
            length = Area. nextDouble ();
            System.out.println("Please enter the width of the rectangle: ");
            width = Area. nextDouble();
            System.out.println("What is the unit of the shape?");
            unit = Area.next();
            totalArea = length*width;
            System.out.println(totalArea+unit);
            break;

        case "square" :
            System.out.println("Please enter the leghth of the square: ");
            length = Area. nextDouble ();
            System.out.println("Please enter the width of the square: ");
            width = Area. nextDouble();
            System.out.println("What is the unit of the shape?");
            unit = Area.next();
            totalArea = length*width;
            System.out.println(totalArea+unit);
            break;

        case "triangle" :
            System.out.println("Please enter the base of the triangle: ");
            length = Area. nextDouble ();
            System.out.println("Please enter the height of the triangle: ");
            width = Area. nextDouble();
            System.out.println("What is the unit of the shape?");
            unit = Area.next();
            totalArea = ((length*width)/2);
            System.out.println(totalArea+unit);
            break;

        case "parallelogram" :
            System.out.println("Please enter the base of the parallelogram: ");
            length = Area. nextDouble ();
            System.out.println("Please enter the height of the parallelogram: ");
            width = Area. nextDouble();
            System.out.println("What is the unit of the shape?");
            unit = Area.next();
            totalArea = length*width;
            System.out.println(totalArea+unit);
            break;

        case "circle" :
            System.out.println("Please enter the radius of the circle: ");
            length = Area. nextDouble();
            System.out.println("What is the unit of the shape?");
            unit = Area.next();
            totalArea = 3.14*(Math.pow(length, 2));
            System.out.println(totalArea+unit);
            break;

        case "rhombus" :
            System.out.println("Please enter the first diagonal of the rhombus: ");
            length = Area. nextDouble();
            System.out.println("Please enter the second diagonal of the rhombus: ");
            width = Area. nextDouble();
            System.out.println("What is the unit of the shape?");
            unit = Area.next();
            totalArea = ((length*width)/2);
            System.out.println(totalArea+unit);
            break;

        case "trapezoid" :
            System.out.println("Please enter the first base of the trapezoid: ");
            length = Area. nextDouble();
            System.out.println("Please enter the second base of the trapezoid: ");
            width = Area. nextDouble();
            System.out.println("Please enter the height of the trapezoid: ");
            height = Area.nextDouble();
            System.out.println("What is the unit of the shape?");
            unit = Area.next();
            totalArea = (((length+width)*height)/2);
            System.out.println(totalArea+unit);
            break;

        default:
            System.out.println("The shape you entered is not recongnized");
            }
        }
    }
}

First, you have improved yourself. But I still see the habit of your

 int loop = 0;

        while (loop == 0){...

structure in both the programs you have written so far (Your program doesn't actually iterate as you don't change the loop variable). Actually, while loops are faster than any other loops. On the other hand, it is also important to write more clean & clear code.

As your loop-body is not more than 5000 or more lines, it's better to use a do-while(){...} looping structure.

Polymorphism (Factory Pattern + Strategy Pattern)

You can use Polymorphism in your program to make it nice. (If you don't know what is polymorphism)

Switch cases are good but replacing switch-case with polymorphism is fantastic. Look, you have different things to do in each switch case. Basically, different shapes formed with different ways.

Shape Interface

public interface Shape {

    double calculateArea();
    void buildShape(Scanner scanner);
}

ShapeClub.java (Main class)

public class ShapeClub {

    public static void main(String[] args){

        Scanner scanner = new Scanner (System.in);
        boolean retry = false;

        do{
            System.out.println("Which shape do you want to find the area?");
            Shape unknownShape = ShapeFactory.getShape(scanner.next());
            unknownShape.buildShape(scanner);

            System.out.print("Do you want to try a different shape?[Y/N]:");
            retry = scanner.next().equalsIgnoreCase("Y");
        }while (retry);
        scanner.close();
    }
}

ShapeFactory.java (Factory class)

public class ShapeFactory {

    private ShapeFactory(){}

    public static Shape getShape(String shapeName){
        switch (shapeName){
            case "rectangle":
                return new Rectangle();
            case "circle":
                return new Circle();
            default:
                return null;
        }
    }
}

Rectangle.java (Concrete class)

public class Rectangle implements Shape {

    private double length = 0.0;
    private double width = 0.0;
    private String unit ="";

    @Override
    public double calculateArea() {
        return length*width;
    }

    @Override
    public void buildShape(Scanner scanner) {
        System.out.println("Please enter the leghth of the rectangle: ");
        length = scanner.nextDouble ();
        System.out.println("Please enter the width of the rectangle: ");
        width = scanner.nextDouble();
        System.out.println("What is the unit of the shape?");
        unit = scanner.next();
        System.out.println(calculateArea()+unit);
    }
}

Circle.java (Another concrete class)

public class Circle implements Shape {

    private double radius = 0.0;
    private String unit ="";

    @Override
    public double calculateArea() {
        return Math.PI*(Math.pow(radius, 2));
    }

    @Override
    public void buildShape(Scanner scanner) {
        System.out.println("Please enter the radius of the circle: ");
        radius = scanner.nextDouble();
        System.out.println("What is the unit of the shape?");
        unit = scanner.next();
        System.out.println(calculateArea()+unit);
    }
}

Hope you understood it. :)


Game to shuffle and guess a word I'm new to C++ and while learning, I wrote a simple program that asks for a word, shuffle sthe characters and then asks another person to find the original word, given the shuffled letters.

I would like to know what is right and wrong in this code, what and where can I improve.

#include <iostream>

void shuffle(std::string& word);

int main(int argc, char *argv[])
{
    std::string word, shuffled, guess;
    int count(0);

    std::cout << "Word to guess: ";
    std::cin >> word;

    shuffled = word;
    shuffle(shuffled);

    do {
        std::cout << "Guess(" << shuffled << "): ";
        std::cin >> guess;
        count++;
        if (guess != word) {
            std::cout << "No." << std::endl;
        }
    } while (guess != word);

    std::cout << "Congratulation!" << std::endl;
    std::cout << "Word found in " << count
              << " guess" << (count>1?"es":"")
              << "." << std::endl;

    return 0;
}

void shuffle(std::string& word)
{
    srand(time(nullptr));
    for (unsigned int i(0); i < word.length(); ++i) {
        std::swap(word[i], word[rand() % word.length()]);
    }
}

The most obvious point is probably that the standard library already includes std::shuffle, so there's little point in creating your own.

If you do decide to write your own shuffle, you probably want to look up the Fisher-Yates Shuffle algorithm. What you're doing right now can introduce biases (actually, it introduces a couple of different biases in entirely different ways).

If you're going to generate random numbers in C++, you're generally better off using the "new" generators found in <random> instead of using srand/rand from <stdlib.h>/<cstdlib>.

Probably also want to use the standard distribution classes instead of rolling your own with the modulus operator (that's one of your current sources of bias1).

Even if you do insist on writing your own and using srand/rand, you must call srand only once, probably when your program starts, not separately each time you're going to use some random numbers.
I advise against using std::endl. Most of the time (including this case) printing a new-line ('\n') will do what you want, and frequently be substantially faster. In addition to writing a new-line, std::endl flushes the stream, which is almost never necessary or desirable.
Instead of:

std::cout << "Word found in " << count
          << " guess" << (count>1?"es":"")

I'd rather use:

std::cout << "Word found in " << count
      << (count > 1 ? "guesses" : "guess";

At least to me, this makes it somewhat easier to follow what's going on (and a trade between easier to read and saving half a dozen bytes of string constant seems pretty easy to me, at least most of the time).
Finally, Add the appropriate includes for the features you use.

std::string => #include <string>
std::srand() => #include <cstdlib>
std::time() => #include <ctime>
srand() => #include <stdlib.h>      // Note the difference between C++ and C version.
time() => #include <time.h>

1. consider a generator that produced 0, 1, or 2 and you used %2 to get only 0 or 1. You'd get 1 only when the generator produced 1, but you'd get 0 when it produced either 0 or 2, so assuming the generator produced 0, 1, and 2 equally frequently, your output would be 0 about twice as often as it was 1. With larger ranges, the bias tends to be less extreme (and less visible) but the same basic mechanism causes the same basic problem.


UTF8 Codepoint decode and length v.2 This is the complete rewrite of a function to decode an UTF-8 codepoints and return the encoding length (or 0 if the codepoint is \0). In response to the review of my previous version I decided that I needed a different approach and I should recognize ALL the invalid encodings (as per reviewer suggestion). If an invalid encoding is found, the first byte is returned with length 1.

I found a very good article about decoding UTF-8 by Bjoern Hoehrmann and followed the same approach which is based on the following Finite Automaton:

I directly coded it in C:

#define fsm
#define fsmGOTO(x)    goto fsm_state_##x
#define fsmSTATE(x)   fsm_state_##x :
#define fsmSTART

int utf8_cp(char * restrict txt, int32_t * restrict ch)
{
  int len;
  uint8_t *s = (uint8_t *)txt;
  uint8_t first = *s;

  fsm {
    fsmSTART {
      if (*s == 0x00) { *ch = *s;        len = 0; fsmGOTO(end); }
      if (*s <= 0xC1) { *ch = *s;        len = 1; fsmGOTO(end); }
      if (*s <= 0xDF) { *ch = *s & 0x1F; len = 2; fsmGOTO(2);   }
      if (*s == 0xE0) { *ch = *s & 0x0F; len = 3; fsmGOTO(4);   }
      if (*s <= 0xEC) { *ch = *s & 0x0F; len = 3; fsmGOTO(3);   }
      if (*s == 0xED) { *ch = *s & 0x0F; len = 3; fsmGOTO(5);   }
      if (*s <= 0xEF) { *ch = *s & 0x0F; len = 3; fsmGOTO(3);   }
      if (*s == 0xF0) { *ch = *s & 0x07; len = 4; fsmGOTO(6);   }
      if (*s <= 0xF3) { *ch = *s & 0x07; len = 4; fsmGOTO(7);   }
      if (*s == 0xF4) { *ch = *s & 0x07; len = 4; fsmGOTO(8);   }
      fsmGOTO(invalid);
    }

    fsmSTATE(8) {
      s++; if ( *s < 0x80 || 0x8f < *s) fsmGOTO(invalid);
      *ch = (*ch << 6) | (*s & 0x3F);
      fsmGOTO(3);
    }

    fsmSTATE(7) {
      s++; if ( *s < 0x80 || 0xbf < *s) fsmGOTO(invalid);
      *ch = (*ch << 6) | (*s & 0x3F);
      fsmGOTO(3);
    }

    fsmSTATE(6) {
      s++; if ( *s < 0x90 || 0xbf < *s) fsmGOTO(invalid);
      *ch = (*ch << 6) | (*s & 0x3F);
      fsmGOTO(3);
    }

    fsmSTATE(5) {
      s++; if ( *s < 0x80 || 0x9f < *s) fsmGOTO(invalid);
      *ch = (*ch << 6) | (*s & 0x3F);
      fsmGOTO(2);
    }

    fsmSTATE(4) {
      s++; if ( *s < 0xA0 || 0xbf < *s) fsmGOTO(invalid);
      *ch = (*ch << 6) | (*s & 0x3F);
      fsmGOTO(2);
    }

    fsmSTATE(3) {
      s++; if ( *s < 0x80 || 0xbf < *s) fsmGOTO(invalid);
      *ch = (*ch << 6) | (*s & 0x3F);
      fsmGOTO(2);
    }

    fsmSTATE(2) {
      s++; if ( *s < 0x80 || 0xbf < *s) fsmGOTO(invalid);
      *ch = (*ch << 6) | (*s & 0x3F);
      fsmGOTO(end);
    }

    fsmSTATE(invalid) {*ch = first; len = 1;}

    fsmSTATE(end)   { }
  }
  return len;
}

The code above, not only handles the invalid encondings, it's also much faster (33%) than my previous version!

I also compared my code with one directly based on the original version by Bjoern Hoehrmann (using a table to represent the FSM) and I found them being equally fast but mine being much clearer in terms of understanding how it works.

Any comment, feedback or suggestion is greatly appreciated.

Overall, this seems like a pretty straightforward implementation of the FSM you show. It's understandable, which is great!
Document What It Is
The only thing is, what if someone reading this has never seen the image of the FSM? You should, at the very least, include a comment with a URL showing where the FSM comes from! Or, if you can easily explain it in words, do that.
Don't Do Nothing
What is the point of the fsm and fsmstart macros? They do nothing. I don't mind that you're starting new scope with each one, but an empty macro seems pointless. You could just as easily use a comment. It would be confusing to someone reading the code for the first time, thinking they did something.
Be Careful With Macros
While we're no the subject of macros... What's the point of the other macros? The fsmSTATE macro saves you no typing at all, and doesn't appear to have any other function. Just use the label directly in your code. The fsmGOTO label saves you a little typing but doesn't really seem all that useful. Macros are known to have a number of dangerous issues, so it's best to avoid them if at all possible.
Don't Use Magic Numbers
What is state 6? State 4? What is the significance of the numbers? Reading the linked page, it sounds like they are character classes or something like that. Would it be possible to have named constants that make this more obvious?
Be Careful With Multiple Statements in a Single Line
You have a lot of places where you've written several statements on the same line, like this:
s++; if ( *s < 0x80 || 0xbf < *s) fsmGOTO(invalid);

This can be problematic for a number of reasons. First, it's difficult to debug. When you're stepping through the code in a debugger, it can be very difficult to tell which statement had what effect. Second, it's hard to read.


How can I refactor this oversized Ruby method into smaller bits and have unit tests for them? This is the existing code.  Clearly way too much is going on in this index method and furthermore it's untested (and hard to test in its current form).

  def index
#    @s = ""
#    @now=DateTime.now
    @from = params[:from]? ('01/01/'+params[:from]).to_date : '01/01/1991'.to_date
    @to = params[:to]? params[:to]+'/12/31' : '2099/12/31' # + DateTime.now.year + 100.years#  .year)+100.to_s).to_date
    if params[:search_text_1]
      @srch = params[:search_text_1]
      if params[:search_text_2] # An advanced search
        #
        # groups...
        @groups_comparison = ' and group_id in (-1'
        if params[:groups]
          params[:groups].each do |group_id|
            @groups_comparison+= ','+group_id
          end
          @groups_comparison+= ')'
        end
        #
        @srch_2 = params[:search_text_2] # May be null
        @version = (params[:version].to_f) #.is_a?(Integer))? params[:version].to_i : 0 # May be 0
        @version_comparison = '(version_number ' + params[:version_comparison] + ' ' + @version.to_s
        if params[:include_blank_version]  # If not checked doesn't pass.'
          @version_comparison+= " or version_number is NULL or version_number = '' )"
        else
          @version_comparison+= " and version_number is not NULL and version_number <> '' )"
        end
        @date_comparison = ' and ((content_date is NULL) or (content_date > ' + @from.to_s + '))'   #+ ' and content_date <= ' + @to.to_s + '))'

        case params[:join_operator].downcase
          when /and/
            @conditions =
              [@version_comparison + @groups_comparison + @date_comparison +
              ' and ((url_address LIKE ? or alt_text LIKE ?) and (url_address LIKE ? or alt_text LIKE ?))',
              "%"+@srch+"%", "%"+@srch+"%", "%"+@srch_2+"%", "%"+@srch_2+"%"]
          when /or/
            @conditions =
              [@version_comparison + @groups_comparison + @date_comparison +
              ' and ((url_address LIKE ? or alt_text LIKE ?) or (url_address LIKE ? or alt_text LIKE ?))',
              "%"+@srch+"%", "%"+@srch+"%", "%"+@srch_2+"%", "%"+@srch_2+"%"]
        end
      else # Just a simple search
        @conditions = ['(url_address LIKE ? or alt_text LIKE ? or version_number LIKE ?)', "%"+@srch+"%", "%"+@srch+"%", "%"+@srch+"%"]
      end
    else
      @conditions = ''
    end
    @links = Link.all(:joins => :group, :order => 'groups.group_name, links.position', :conditions => @conditions)

    session[:row_shading] = (params[:row_shading] == 'true') ? 'true' : 'false' rescue 'false'
    session[:full_details] = (params[:full_details] == 'true') ? 'true' : 'false' rescue 'false'
    respond_to do |format|
      format.html # index.html.erb
      format.xml  { render :xml => @links }
    end
  end

Create a model to hold all the logic and call that with parameters and have tests for the model methods:

  Unit Tests

require 'spec_helper'

describe "Prepare Search methods" do

  describe "PrepareSearch#start_date" do

    subject { PrepareSearch.start_date('2000') }
    it { should == DateTime.new(2000,1,1) }

  end

  describe "PrepareSearch#end_date" do

    subject { PrepareSearch.end_date('2009') }
    it { should == DateTime.new(2009,12,31) }

  end

  describe "PrepareSearch#groups" do

    subject { PrepareSearch.groups(['1','2','3']) }
    it { should == " and group_id in (-1,1,2,3)" }

  end

  describe "PrepareSearch#versions" do

    before(:each) do
      @versions= {:version => 0, :version_comparison => '>=', :include_blank_version => true}
    end
    subject { PrepareSearch.versions(@versions) }
    it { should == " and (version_number >= 0.0 or version_number is NULL or version_number = '' )" }

  end

  describe "PrepareSearch#dates" do

    subject { PrepareSearch.dates(Date.new(2009), Date.new(2012) ) }
    it { should == " and ((content_date is NULL) or( (content_date >= \"2009-01-01\") and (content_date <= \"2012-01-01\") ) )" }

  end

  describe "PrepareSearch#basic_search" do

    subject { PrepareSearch.basic_search("test") }
    it { should == ["(url_address LIKE ? or alt_text LIKE ? or version_number LIKE ?)","%test%","%test%","%test%"] }

  end

  describe "PrepareSearch#text_search" do

    subject { PrepareSearch.text_search("tests","or","rs") }
    it { should == " and ( (url_address LIKE \"%tests%\" or alt_text LIKE \"%tests%\" or version_number LIKE \"%tests%\") or (url_address LIKE \"%rs%\" or alt_text LIKE \"%rs%\" or version_number LIKE \"%rs%\") )" }

  end

end

  New Model "PrepareSearch"

class PrepareSearch

  def self.start_date(start_year)
    (start_year + '/01/01').to_date
  end

  def self.end_date(end_year)
    (end_year + '/12/31').to_date
  end

  def self.groups(groups=[])
    @groups_comparison= ' and group_id in (-1'
    groups.each do |group_id|
      @groups_comparison << ','+group_id
    end
    @groups_comparison+= ')'
  end

  def self.versions(version_information={} )
    @version = version_information[:version].to_f
    @version_comparison = ' and (version_number ' + version_information[:version_comparison] + ' ' + @version.to_s
    if version_information[:include_blank_version]
      @version_comparison+= " or version_number is NULL or version_number = '' )"
    else
      @version_comparison+= " and version_number is not NULL and version_number <> '' )"
    end
  end

  def self.dates(from, to)
    @from= from
    @to= to
     ' and ((content_date is NULL) or( (content_date >= "' + @from.to_s + '") and (content_date <= "' + @to.to_s + '") ) )'
  end

  def self.text_search(words_1,comparison_operator='',words_2='')
    @words_1 = words_1
    @words_2 = words_2
    @comparison_operator= comparison_operator
    if @words_2.length > 0
      @text_search = ' and ( (url_address LIKE "%'+@words_1+'%" or alt_text LIKE "%'+@words_1+'%" or version_number LIKE "%'+@words_1+'%") '
      @text_search+= @comparison_operator
      @text_search+= ' (url_address LIKE "%'+@words_2+'%" or alt_text LIKE "%'+@words_2+'%" or version_number LIKE "%'+@words_2+'%") )'
    else
      ' and (url_address LIKE "%'+@words_1+'%" or alt_text LIKE "%'+@words_1+'%" or version_number LIKE "%'+@words_1+'%")'
    end
  end

  def self.basic_search(words)
    @words = words
    @conditions = ['(url_address LIKE ? or alt_text LIKE ? or version_number LIKE ?)', "%"+@words+"%", "%"+@words+"%", "%"+@words+"%"]
  end

end

  Refactored index action

  def index
    @from = PrepareSearch.start_date(params[:from] ||= '1991')
    @to = PrepareSearch.end_date(params[:to] ||= '2299')

    if params[:search_text_1st_phrase] # both simple and advanced search use this field.
      @words_1 = params[:search_text_1st_phrase]
      if params[:commit].downcase == 'advanced search'
        @join_operator = params[:join_operator].downcase
        @words_2 = params[:search_text_2nd_phrase]
        @text_search = PrepareSearch.text_search(@words_1, @join_operator, @words_2)
        @groups_comparison = PrepareSearch.groups(params[:groups])
        @version_information = { :version => params[:version], :version_comparison => params[:version_comparison], :include_blank_version => params[:include_blank_version] }
        @version_comparison = PrepareSearch.versions(@version_information)
        @date_comparison = PrepareSearch.dates(@from, @to)
        @conditions = '1=1' + @groups_comparison+ @version_comparison+ @date_comparison + @text_search
      else
        @conditions = PrepareSearch.basic_search(@words_1)
      end
    else
      @conditions = ''
    end
    @links = Link.all(:joins => :group, :include => :group, :order => 'groups.group_name, links.position', :conditions => @conditions)
    session[:full_details] = (params[:full_details] == 'true') ? 'true' : 'false' rescue 'false'
    respond_to do |format|
      format.html
    end
  end

Also (next steps):

Write controller/integration tests to check actual search results for various combinations of parameters.  Needs a bunch of test data.  Probably at least 20 links and 4 groups for useful testing data.
consider making an object for all the params and passing them to one method call.


Login UI for an Android app This code starts a new thread for login and reacts according to the JSON result returned by the server.

I think this code has too many conditionals, exception handlers, and nested functions.

public void Login() {
    mButton.setClickable(false);
    mButton.setText("Loading...");
    final String username = mUserName.getText().toString().trim();
    final String password = mPassWord.getText().toString().trim();

    new Thread() {
        public void run() {
            final String result = NetUtil.loginByGet(username, password);
            if (result != null) {
                try {
                    JSONTokener jsonTokener = new JSONTokener(result);
                    JSONObject jsonObject = (JSONObject) jsonTokener.nextValue();
                    final String message = jsonObject.getString("message");
                    if (jsonObject.getInt("error") == 0) {
                        String token = jsonObject.getString("token");
                        boolean isSaveSuccess = InfoUtil.saveUserInfo(LoginActivity.this,token);
                        if (isSaveSuccess) {
                            runOnUiThread(new Runnable() {
                                @Override
                                public void run() {
                                    Toast.makeText(LoginActivity.this, "登录成功", Toast.LENGTH_SHORT).show();
                                    initIntent(MainActivity.class);
                                    finish();
                                }
                            });
                        }else {
                            runOnUiThread(new Runnable() {
                                @Override
                                public void run() {
                                    Toast.makeText(LoginActivity.this, "保存登录信息失败", Toast.LENGTH_SHORT).show();
                                    mButton.setClickable(true);
                                    mButton.setText("登录");
                                }
                            });
                        }
                    } else if(jsonObject.getInt("error") == 1) {
                        runOnUiThread(new Runnable() {
                            @Override
                            public void run() {
                                Toast.makeText(LoginActivity.this, "" + message,
                                        Toast.LENGTH_SHORT).show();
                                mButton.setClickable(true);
                                mButton.setText("登录");
                            }
                        });
                    }
                }catch (Exception e) {
                    e.printStackTrace();
                }
            } else {
                runOnUiThread(new Runnable() {
                    @Override
                    public void run() {
                        Toast.makeText(LoginActivity.this, "请求失败", Toast.LENGTH_SHORT).show();
                        mButton.setClickable(true);
                        mButton.setText("登录");
                    }
                });
            }
        }
    }.start();
}

Using an AsyncTask instead would be much better. You've got many callbacks to the UI thread, all of this you could do in the post execute method instead:

private class LoginTask extends AsyncTask<Void, Void, String>{

    private String username;
    private String password;
    private Exception exception;
    private boolean isSaveSuccess;

    public LoginTask(String username, String password){
        this.username = username;
        this.password = password;
    }

    @Override
    protected String doInBackground(Void... voids) {
        try{
            String result = NetUtil.loginByGet(username, password);
            //Grab the token
            isSaveSuccess = InfoUtil.saveUserInfo(LoginActivity.this,token);
        }catch(Exception ex){
            //Catch any given exception to use it in the post execution (e.g. show a toast)
            this.exception = ex;
        }
    }

    @Override
    protected void onPostExecute(String result) {
        //Here it goes your UI logic
    }

}

To call it, just launch an execution when login button is clicked:

public void doLogin(){
    mButton.setClickable(false);
    mButton.setText("Loading...");
    String username = mUserName.getText().toString().trim();
    String password = mPassWord.getText().toString().trim();
    new LoginTask(username, password).execute();
}


Lower, Higher, or Same Card Game This is my second script in my first language so I'm trying to get oriented and better foundations. I noticed in the while loop I got really messy but I couldn't google my way to a better solution that I could understand. With that, I'd appreciate any feedback that makes my code more "pythonic" or clean.

import random

def card_deck():
    card_faces = ['2', '3', '4', '5', '6', '7', '8', '9', '10', 'Jack', 'Queen', 'King', 'Ace']
    card_suits = ['♠', '♦', '♥', '♣']
    deck = []
    for i in card_suits:
        for j in card_faces:
            deck.append(j + ' ' +  i)
    return deck

def card_draw(deck):
    random.shuffle(deck)
    deck = deck.pop()
    return deck

def card_value(card):
    card = str(card)
    if card[0] == 'J':
        return 10
    elif card[0] == 'Q':
        return 12
    elif card[0] == 'K':
        return 13
    elif card[0] == 'A':
        return 14
    else:
        card = int(card[0:2])
        return card

correct_guesses = 0
deck = card_deck()
card = card_draw(deck)

while True:
    card_points = card_value(card)
    if not deck:
        print("You've done it! You've won with", correct_guesses, "correct guesses!")
        break
    next_card = card_draw(deck)
    next_card_points = card_value(next_card)
    print("\nYou're card is: ", card)
    user_guess = input('\n(H)igher, (L)ower, or (S)ame?:\t')
    user_guess = user_guess.lower()
    if user_guess not in ('h', 'l', 's'):
        break
    if card_points < next_card_points and user_guess == 'h':
        print("Correct!")
        correct_guesses += 1
        card = next_card
        continue
    elif card_points > next_card_points and user_guess == 'l':
        print("Correct!")
        correct_guesses += 1
        card = next_card
        continue
    elif card_points == next_card_points and user_guess == 's':
        print("Correct!")
        correct_guesses += 1
        card = next_card
        continue
    else:
        print('You lose the next card was ', next_card)
        print('You had', correct_guesses, 'correct guesses.')
        break
```

I may have went overboard on the Python-ification, but this is a refactoring of the current solution.

What I did was create a Card class which contained the suit and value of the card. To get the value, I added all items to an array and then got the index of the card's value from the array's offset, which allowed card_value to be removed.

There was lots of duplication when displaying that the user got the correct answer. I made this a function and was able to minimize the method's body by using an iterator which contained the current card and the previous card (thus avoiding having to set the previous card to the current card and then get the next card.)

Here is a rewritten version: (comments to follow)

import random
import itertools

card_faces = [
    '2', '3', '4', '5', '6', '7', '8', '9', '10', 'Jack', 'Queen', 'King',
    'Ace'
]

class Card:
    """
  Defines a Card with a suit and face, and allows retrieving the points of the card
  """

    def __init__(self, face_suit):
        self.face = face_suit[0]
        self.suit = face_suit[1]

    def getPoints(self):
        return card_faces.index(self.face) + 2

    def __str__(self):
        return self.face + ' ' + self.suit

    def __gt__(self, other):
        return self.getPoints() > other.getPoints()

    def __lt__(self, other):
        return self.getPoints() < other.getPoints()

    def __eq__(self, other):
        return self.getPoints() == other.getPoints()

def generate_card_deck():
    """
    Creates a shuffled list of cards as [(c0, c1), (c1, c2), ...]
    """
    card_suits = ['♠', '♦', '♥', '♣']
    deck = list(map(Card, itertools.product(card_faces, card_suits)))
    random.shuffle(deck)
    return deck

def user_guessed_correctly():
    """
  Congratulates the user if they have a correct guess, and increments
  the counter
  """
    print("Correct!")
    global correct_guesses
    correct_guesses += 1

def pairwise(iterable):
    "s -> (s0,s1), (s1,s2), (s2, s3), ..."
    a, b = itertools.tee(iterable)
    next(b, None)
    return zip(a, b)

correct_guesses = 0
deck = pairwise(generate_card_deck())

while True:
    card, next_card = next(deck, (None, None))
    if not card:
        print("You've done it! You've won with", correct_guesses,
              "correct guesses!")
        break

    print("Your card is: ", card)
    user_guess = input('(H)igher, (L)ower, or (S)ame?:\t').lower()
    if user_guess not in ('h', 'l', 's'):
        break

    if (card < next_card and user_guess == 'h') or (
            card > next_card and user_guess == 'l') or (card == next_card
                                                        and user_guess == 's'):
        user_guessed_correctly()
    else:
        print('You lose: the next card was ', next_card)
        print('You had', correct_guesses, 'correct guesses.')
        break


Substring a String from given start index and given substring length I want to substring a String from given start index and given substring length.

if the string is empty then returns String.Empty
if the string length is greater than start index then it substrings from given start index to given length...
if the string length is less than start index then it returns  from start index to last index

I just want to know if it can be improved and shorter with all these checks.

if the string is null or empty
if the start index is not less than zero
if the start index is less than the length of given input string.
if the given input string length is less than the desired substring length.
if the given input string length is greater than the desired substring length.

Sample Input and Ouput for my program:

SplitEntity("", 1, 15)               =>   ""
SplitEntity("abcdef", 0, 3)          =>   "abc"
SplitEntity("abcdef", 3, 100)        =>   "def"
SplitEntity("abcdef", 0, 100)        =>   "abcdef"
SplitEntity("abcdef", -1, 100)        =>   ""

My function is below:

Public Function SplitEntity(entity As String, startIndex As Integer,
                                       subStringLength As Integer) As String
            Dim spilttedString As String = String.Empty

            If (Not String.IsNullOrEmpty(entity) AndAlso startIndex >=0 ) Then

                If (entity.Length > startIndex) Then
                    If entity.Length >= (startIndex + subStringLength) Then
                        spilttedString = entity.Substring(startIndex, subStringLength)

                    ElseIf entity.Length < (startIndex + subStringLength) Then
                        spilttedString = entity.Substring(startIndex, (entity.Length - startIndex))
                    End If

                End If

            End If

            Return spilttedString

        End Function

for something like this I would suggest making it an Extension Method.

To that end I would suggest a name change as well.  Perhaps Substring_NoException.

To make it complete I think having the same overloads as Substring would help.

As for reducing/eliminating the conditional checks, one option is to use a Try/Catch block and simply catch every exception that Substring would and return an empty string except for the length being too long.

It might look something like this:

Imports System.Runtime.CompilerServices

    ''' <summary>
    ''' Extension method for <c>Substring</c> to return empty strings instead of throwing exceptions
    ''' </summary>
    ''' <param name="startIndex">The zero-based character position of a substring in this instance</param>
    '''
    <Extension()>
    Public Function Substring_NoException(input As String, startIndex As Integer) As String
        Return input.Substring_NoException(startIndex, input.Length - startIndex)
    End Function

    ''' <summary>
    ''' Extension method for <c>Substring</c> to return empty strings instead of throwing exceptions
    ''' </summary>
    ''' <param name="startIndex">The zero-based character position of a substring in this instance</param>
    ''' <param name="length">The number of characters in a substring</param>
    '''
    <Extension()>
    Public Function Substring_NoException(input As String, startIndex As Integer, length As Integer) As String
        Try
            Return input.Substring(startIndex, length)
        Catch ex As ArgumentOutOfRangeException
            If ex.ParamName = "length" AndAlso length > input.Length - startIndex Then
                Return input.Substring(startIndex)
            End If
            Return String.Empty
        End Try
    End Function

usage is:

Dim testString As String = "abcdef"
Dim testString2 = testString.Substring_NoException(3, 100)

Returned value is:

testString2 = "def"


First attempt at a SVG parser I figured it's about time to jump into some Haskell, so here's a first attempt at an oddly specific SVG parser:

import System.Environment
import Text.ParserCombinators.Parsec

parseFile fname = parseFromFile (manyTill (try tag) (try readEnd)) fname

tag = do manyTill anyChar . try $ lookAhead tagStart
         char '<'
         name <- many $ noneOf " "
         props <- tagContents
         char '>'
         junk
         return (name, props)

tagContents = do props <- manyTill property . try . lookAhead $ char '>'
                 junk
                 return props

property = do spaces
              name <- many1 $ noneOf "="
              string "=\""
              val <- manyTill anyChar $ char '"'
              junk
              return (name, val)

junk = optional . many $ oneOf "\n\r\t\\/"

readEnd = do optional $ string "</svg>"
             junk
             eof
tagStart = do char '<'
              tagName

tagName = string "rect"
          <|> string "polygon"
          <|> string "polyline"
          <|> string "circle"
          <|> string "path"
          <|> string "g"
          <|> string "svg"

Before anyone asks, one of the objectives for this program (other than learning) is that it should accept invalid SVGs (for example, a partial SVG or one that has been saved as an rtf), which is why I'm making liberal use of try, and cherry-picking tags.

As implied by the title, this is my first non-tutorial attempt at Haskell, so give me all the style pointers you can muster (and highlight anything that signals a broken understanding on my part).

I'd also appreciate pointers on how to cherry-pick properties (and not just tags). I've tried writing property as

property = do manyTill anyChar . try $ lookAhead tagName
              name <- many1 $ noneOf "="
              string "=\""
              val <- manyTill anyChar $ char '"'
              junk
              return (name, val)

and defining propName as

propName = string "points"
           <|> string "x"
           <|> string "y"
           <|> string "r"
           <|> string "d"
           <|> string "cx"
           <|> string "cy"
           <|> string "width"
           <|> string "height"
           <|> string "transform"

This seems like it should work since it's basically how I got tag jumping to the next desired tag, but it gives me unexpected input errors.

Edit The Second:

import System.Environment
import Text.ParserCombinators.Parsec

type TagName = String
type Property = (PropName, Value)
type PropName = String
type Value = String

parseFile fname = parseFromFile (manyTill (try tag) (try readEnd)) fname

tag :: GenParser Char st (TagName, [Property])
tag = do manyTill anyChar . try $ lookAhead tagStart
         name <- tagStart
         props <- tagContents
         char '>'
         junk
         return (name, props)

tagContents :: GenParser Char st [Property]
tagContents = do props <- manyTill property . try . lookAhead $ char '>'
                 junk
                 return props

property :: GenParser Char st (PropName, Value)
property = do manyTill anyChar . try $ lookAhead propName
              name <- many1 $ noneOf "="
              string "=\""
              val <- manyTill anyChar $ char '"'
              junk
              return (name, val)

junk = many $ oneOf "\n\r\t\\/{} "

tagStart :: GenParser Char st TagName
tagStart = do char '<'
              name <- tagName
              return name

readEnd = do optional $ string "</svg>"
             junk
             eof

tagName = oneStringOf ["rect", "polygon", "polyline", "circle", "path", "g", "svg"]
propName = oneStringOf ["points", "x", "y", "r", "d", "cx", "cy", "width", "height", "transform"]

oneStringOf :: [String] -> GenParser Char st String
oneStringOf = choice . map (try . string)

Added try to the definition of oneStringOf

Ok, here are some things I noticed:

First of all, while the code seems easy enough to follow as it is, a couple of comments here and there certainly couldn't hurt.

tag = do manyTill anyChar . try $ lookAhead tagStart
         char '<'
         name <- many $ noneOf " "

It seems weird that you first use tagStart to find until where to match, but then don't use it to actually match the tag start.

junk = optional . many $ oneOf "\n\r\t\\/"

Since many can already match the empty string, making it optional doesn't change anything.

tagName = string "rect"
          <|> string "polygon"
          <|> string "polyline"
          <|> string "circle"
          <|> string "path"
          <|> string "g"
          <|> string "svg"

That looks a bit repetitive. I'd define a helper function which matches one of a list of strings:

-- Takes a list of strings and returns a Parser which matches any of those strings
oneStringOf = choice . map (try . string)

tagName = oneStringOf ["rect", "polygon", "polyline"] -- etc

By adding the try we also made it so that it actually works correctly now (thanks to Joey Adams for pointing out that fix).

As for why your alternative definition for property doesn't work: I'm not sure whether it's the only thing that keeps it from working, but one mistake is that you wrote tagName when you meant propName.


Test whether a number is a Harshad Number A little code I wrote that will check if the entered 4-digit number is Harshad or not (Harshad number is one which divided by the sum of its digits will leave no remainder, special Harshad is the same but will also work with flipped sum of digits, example - 1729/19 (19 is the sum of digits) and when flipped 1729/91 will leave no remainder as well).

I would love to see how it could become better (I'm still new so was limited with the commands I could've used).

; Harshad.asm - check if input from user (4 digits, 1000-9999) is a Harshad number and if so, check if it's a "special" Harshad number
;
    .MODEL SMALL
    .STACK 100h
    .DATA
RequestStr     DB 'Enter a 4 digit number (1000-9999):',13,10,'$'
IsHarshad      DB ' is a Harshad number.',13,10,'$'
SpecialHarshad DB 'It is also a special Harshad number.',13,10,'$'
NotHarshad     DB ' is not a Harshad number',13,10,'$'
IncorrectInput DB 13,10,'Input is incorrect.',13,10,'$'
Num            DW ? ;Will fit a "word" size (16 bits)
DigitSum       DW ? ;Sum of digits
TEN            DW 10
TENbyte        DB 10
Temp           DB ? ;Used to check if number is also special Harshad during the div process
Temp2          DB ? ;Used with special Harshad div process
;
     .CODE
     MOV AX,@DATA                ;DS can be written to only through a register
     MOV DS,AX                   ;Set DS to point to data segment
     MOV AH,9                    ;Set print option for INT 21h
     MOV DX,OFFSET RequestStr    ;Set DS:DX to point to RequestString
     INT 21h                     ;Print RequestStr
;
NumberInput:
    ;First digit
    MOV AH,1        ;Set scanning (input) option for INT 21h
    INT 21h         ;Scan first digit
    MOV DX,0
    SUB AL,'0'      ;Converting from ascii value to numeral value
    CMP AL,1        ;First digit must be between 1 and 9 in order for the number to be of 4 digits
    JB WrongInput   ;Otherwise jump to WrongInput label
    CMP AL,9
    JA WrongInput
    MOV AH,0
    MOV DigitSum,AX ;Store only first digit's value at the variable DigitSum
    MUL TEN         ;Multiply AX by 10
    MOV Num,AX
    ;Second digit
    MOV AX,0
    MOV AH,1
    INT 21h
    SUB AL,'0'
    CMP AL,0
    JB WrongInput
    CMP AL,9
    JA WrongInput
    MOV AH,0
    ADD DigitSum,AX ;Add only second's digit value to DigitSum
    ADD Num,AX      ;Add AX's value (which has been multiplied by 10 with the first digit) to Num variable
    MOV AX,0
    MOV AX,Num      ;Move new Num's value to AX to multiply it by 10
    MUL TEN
    MOV Num,AX
    ;Third digit
    MOV AX,0
    MOV AH,1
    INT 21h
    SUB AL,'0'
    CMP AL,0
    JB WrongInput
    CMP AL,9
    JA WrongInput
    MOV AH,0
    ADD DigitSum,AX
    ADD Num,AX
    MOV AX,0
    MOV AX,Num
    MUL TEN
    MOV Num,AX
    ;Forth digit
    MOV AX,0
    MOV AH,1
    INT 21h
    SUB AL,'0'
    CMP AL,0
    JB WrongInput
    CMP AL,9
    JA WrongInput
    MOV AH,0
    ADD DigitSum,AX ;Now DigitSum contains the sum of each of the 4 digits in the number
    ADD Num,AX ;Num contains full 4 digits number
    JMP CheckHarshad
WrongInput:
    MOV AH,9
    MOV DX,OFFSET IncorrectInput
    INT 21h
    JMP CodeEnd
CheckHarshad:
    MOV AX,0
    MOV DX,0
    MOV AX,Num
    DIV DigitSum ;Number will be stored in AX and the remainder in DX
    CMP DX,0 ;Check if there is remainder or not
    JE CheckSpecialHarshad
    MOV AH,9
    MOV DX,OFFSET NotHarshad
    INT 21h
    JMP CodeEnd
CheckSpecialHarshad:
    MOV AH,9
    MOV DX, OFFSET IsHarshad
    INT 21h
    MOV AX,0
    MOV AX,DigitSum
    DIV TENbyte
    MOV Temp,AL
    MOV Temp2,AH
    MOV AX,0
    MOV AL,Temp2
    MUL TENbyte
    ADD AL,Temp
    MOV Temp2,AL ;Temp2 now has the DigitSum number flipped
    MOV AX,0
    MOV AX,Num
    DIV Temp2
    CMP AH,0
    JNE CodeEnd
    MOV DX,OFFSET SpecialHarshad
    MOV AH,9
    INT 21h
CodeEnd:
    MOV AH,4Ch
    INT 21h
    END

Written for NASM

Your program is a little hard to follow, mostly due to poor formatting. There is a lot of unnecessary duplication too and the user has no opportunity for correcting mistakes. The following prologue will address these initial criteria;

Prompt operator for what is required.
Use a function that allows the user to correct mistakes.
Test that entry is 4 characters long and the first digit is not zero.

      org   0x100

      DOS   equ 21H
     SHOW   equ  9
 BUFFERED   equ 10  ; Buffered input
   OUTCHR   equ  2
   GETCHR   equ  8

Begin:  mov ah, SHOW        ; Function 9 display string terminated with '$'
        mov dx, Prompt
        int DOS

        mov ah, BUFFERED    ; Function 10 Get buffered input
        mov dx, Entry
        int DOS

; Test that your criteria of 4 character entry and first digit not zero has been met.

        cld                 ; Make sure SI auto-increments
        mov si, dx
        inc si              ; Point to number of characters entered
        lodsb
        or  al, al
        jz  Err             ; You might want to put some sort of prompting text here
        cmp al, 4
        jnz Err
        cmp byte [si], '1'
        jae Evaluate

Err:    mov ah, SHOW        ; Function 9 again
        mov dx, ErrTxt
        int DOS
        ret                 ; Return to DOS

You do have these elements in your program, but I think you'll agree when there are in one place it's a lot easier to recognize which error testing mechanism has been implemented. Notice too, formatting is monumentally important, especially in ASSM.

Now we are ready to do the calculations and because we're not going to be using any DOS functions, there is a little more freedom in which registers we can use and for what. The criteria now becomes, establish a binary value of our entry and binary total of all digits.

Evaluate:

    ; AX = Working register, CX = character count

        movzx   cx, al      ; Move character count into CX
        xor     dx, dx      ; DX will be binary representation of input
        mov     bx, dx      ; BL will be total of digits
        mov     ax, dx

  .get: lodsb           ; Read character from input string
        cmp     al, '0'
        jb      Err
        cmp     al, '9'
        ja      Err

       and      ax, 1111b   ; Could be SUB AL,'0' too.
       add      bl, al      ; Bump total of digits
       xchg     ax, dx
       imul     ax, 10
       add      dx, ax
       loop     .get

     ; Now DX = Binary equivalent and BL divisor

        xor     ax, ax
        xchg    ax, dx
        push    ax          ; Going to need this again to determine if Special
        idiv    bx
        or      dl, dl      ; Determine if value is Harshad or not.

From this point I'd pretty much be doing the same as you have except there is still no need to address anything in memory other than prompting text as everything you need is on the stack and BL. Let us know what you come up with then we'll compare it with what I have.


Summing different tables values limiting the results I have two tables (a and b) which have both one date field of type TIMESTAMP. They represent two different type of actions (records): table a's actions are $200 worth each, while table b's ones worth $500.

I want to calculate the total count of dollars worth of both tables actions limiting the maximum number or dollars per day to $10.000. Is there a more clean or efficient way to do this other than:

SELECT SUM(daily_sum_total) AS total
FROM (
    SELECT CASE WHEN SUM(daily_sum) > 10000 THEN 10000 ELSE SUM(daily_sum) END AS daily_sum_total, day
    FROM (
        SELECT COUNT(date) * 200 AS daily_sum, DATE(date) AS day
        FROM a
        GROUP BY day
        UNION ALL
        SELECT COUNT(date) * 500 AS daily_sum, DATE(date) AS day
        FROM b
        GROUP BY day
    ) AS daily_tmp_table
    GROUP BY day
) as total_tmp_table;

You might try this:

select
  sum(amount) amount
from(
  select
    least(sum(amount),10000) amount
  from (
    select date,
           200 amount
    from   a
    union all
    select date,
           500 amount
    from   b) list_of_all
  group by
    date(date)) list_by_day


Split a big file into two and then merge back into the original After searching and reading some threads (1, 2, 3, 4 and 5) on how to split a big file into smaller ones and then merge them back into the original one I made the following VB.NET code on an empty project with module Module1 and reference to System.Windows.Forms:

' <Summary>
'
'   3 files: 1; 2; 3
'   1 has data
'   2 and 3 are empty
'
'   How to split file 1 into files 2 and 3:
'     remove 2º comment (leave 1º comment)
'     run program (select file 1 in the 1º file system prompt and file 2 in the 2º file system prompt)
'     add 2º comment back and remove 1º comment
'     run program (select file 1 in the 1º file system prompt and file 3 in the 2º file system prompt)
'
'   How to join/merge files 2 and 3 into file 1:
'     leave both 1º and 2º comments commented
'     run program (select file 3 in the 1º file system prompt and file 2 in the 2º file system prompt)
'     file 2 will be the merge of files 2 and 3 (same as file 1)
'
' </Summary>

Imports System.IO
Module Module1
    Sub Main()
        With New Windows.Forms.OpenFileDialog()
            .ShowDialog()
            Dim inputStream As FileStream = New FileStream(.FileName(), FileMode.Open, FileAccess.ReadWrite, FileShare.ReadWrite)
            .ShowDialog()
            Dim outputStream As FileStream = New FileStream(.FileName(), FileMode.Append, FileAccess.Write, FileShare.ReadWrite)
            'inputStream.Position = inputStream.Length \ 2 - inputStream.Length \ 2 Mod 10000
            Dim buffer As Byte() = New Byte(9999) {}
            Dim bytesRead As Integer
            bytesRead = inputStream.Read(buffer, 0, buffer.Length)
            While bytesRead > 0 'And inputStream.Position <= inputStream.Length \ 2
                outputStream.Write(buffer, 0, bytesRead)
                bytesRead = inputStream.Read(buffer, 0, buffer.Length)
            End While
            outputStream.Flush()
            inputStream.Close()
            outputStream.Close()
        End With
    End Sub
End Module

I'm looking to know if there's any different way to do this with a noticeable performance improvement. The size of the original file is almost 30 GB, which will be split into 2 files, sent online together with the program and then merged back together.

Being of that size it takes some time so performance is the focus here, there's no checks of the user's input, no exception/error handling, no pretty code with forms, options and methods/functions. It's just some quick and dirty code.

I would suggest, to get performance and still keep the original intact, that .net libraries are probably not the best choice.

One suggestion that would work, if you can allow smaller chunks(8GB), is 7-Zip.  It's free, open source and includes a file explorer.  It will split a 32GB file into 5 volumes in about 20 min.  Anyone with the same software installed can combine the volumes back again.  It even includes the option to calculate a checksum for the file to help confirm it's still the same file.


Uniformly generate a number from 0 to n-1 that isn't on a blacklist The task:

  Given an integer n and a list of integers l, write a function that
  randomly generates a number from 0 to n-1 that isn't in l (uniform).

Solution 1:

const getRandNotInList = (n , list) => {
  const rand = Math.floor(Math.random() * n);
  return list.includes(rand) ? getRandNotInList(n, list) : rand;
};

console.log(getRandNotInList(16, [1,2,3,4]));

Solution 2:

const getRandNotInList2 = (n, list) => {
  let rand;
  do { rand = Math.floor(Math.random() * n); }
  while(list.includes(rand));
  return rand;
};

console.log(getRandNotInList2(13, [2,3,4,5,6]));

The problem is backwards. You would never present a exclusion list for random values, rather you would present a list to randomly pick from.
Anyways to fit the problem.
Two ways this can be done

Guessing
Deterministically

Guessing
Your method use a random guess, you guess that the number is not in the list. If it is you guess again. This works well when the list contains only a small set of the numbers.
However when the exclusion list gets larger you need to make more guess to find one that is not in the list. The number of guess grows very fast as your exclusion list gets closer to excluding all.
This is the worst possible performance at \$O(m^{-(1/n)})\$ it has an Infinity
The other problem with guessing is that you can not know how long the function is going to run for. This makes it totally unsuitable for many real time applications.
Deterministically
You can improve the overall performance and have a function that has a known fixed time to get a result.
However is is much slower when the exclusion list is relatively small.
The performance is linear at \$O(n)\$
If the exclusion set does not change between calls it get even less complex at \$O(1)\$ which is the best you can get.
Example
The \$O(n)\$ solution
It returns undefined if it can not find a number rather than throw. You should not be solving the calling function problems as the other answers suggest.
const getRandNotInList = (n, list) => {
    const picks = [];
    const getPickable = () => {
        var i = 0; dir = Math.sign(n);
        list = new Set(list);
        while (i !== n) {
            !list.has(i) && picks.push(i);
            i += dir;
        }
    }
    n = Math.floor(n);
    if (n === 0) { return list.includes(0) ? undefined : 0 }
    getPickable();
    return picks[Math.random() * picks.length | 0]; // (| 0) same as Math.floor
                                                    // (only for positive ints).
}


std::vector memory manipulation with serialization and deserialization This is my code and I would like to get it code reviewed. It is functional and behaves as expected.

I pass some basic types to the Serialize() function and then deserialize the output to get back the original value.

size_t returnSize(const char* s)
{
       string string(s);
       return string.size();
};
template<typename T>
size_t returnSize(const T& t)
{
       return sizeof(t);
};

template<typename T>
string Serialize(const T& t)
{
    T* pt = new T(t);
    vector<char> CasttoChar;

    for (int i =0 ;i<returnSize(t);i++)
    {
        CasttoChar.push_back(reinterpret_cast<const char*>(pt)[i]);
    }
    delete pt;
    return  string( CasttoChar.begin(), CasttoChar.end() );

};

template<>
string Serialize<string>(const string& t)
{
    return Serialize(t.c_str());
};

template<typename T>
T DeSerialize(const string& cstr)
{
    const T* a = reinterpret_cast<const T*>(cstr.c_str());
    return *a;
}

template<>
string DeSerialize<string>(const string& cstr)
{
    return DeSerialize<char*>(cstr);
}

int _tmain(int argc, _TCHAR* argv[])
{
    int x = 97;
    string c = Serialize<int>(x);
    cout << DeSerialize<int>(c) << endl;

    char g = 'g';
    string c2 = Serialize<char>(g);
    cout << DeSerialize<char>(c2) << endl;

    string k = "blabla";
    string c3 = Serialize<const char*>(k.c_str());
    cout << DeSerialize<char*>(c3) << endl;

    string ka = "string";
    string c4 = Serialize<string>(ka);
    cout << (DeSerialize<string>(c4)).c_str();

    system("PAUSE");
    return EXIT_SUCCESS;
}

First of all, fundamentally: std::string is a data type to hold text. Exclusively. It should not be used to hold binary data. Use a std::vector<(unsigned) char> for that.

Secondly, you are using heap allocation without needing to:

T* pt = new T(t);

This makes no sense at all, and introduces the potential of a memory leak. You could simply make a copy – but you don’t even need to do that. Just work on the original data.

Next, about naming; it’s general convention in C++ to use underscore_separated_names rather than camelCase or PascalCase. This is only a convention but I’d follow it unless there’s a very compelling reason not to.

Also on the topic of naming: returnSize is redundant, return has no place in the name. Rename it to object_size or something along those lines. You are also inconsistent in your naming convention here.

To get the size of a zero-terminated string, use std::strlen, that’s more efficient than converting to a std::string.

Finally, the code simply doesn’t work. It creates a bit-by-bit shallow representation. That only works for simple composite objects, it no longer works for objects that use pointers or references internally – you’ve already noticed that, because otherwise you wouldn’t need to create a special chase for char* and std::string. Incidentally, for the case of char* you specialise returnSize but for the case of std::string you specialise Serialize and DeSerialize. That’s asymmetrical and means that you have several places that might require changing when new types are added. You should reduce this to a single point which requires specialisation.

Ignoring that, I’d rewrite the code as follows:

#include <algorithm>
#include <iostream>
#include <string>
#include <vector>

typedef unsigned char byte_t;
typedef std::vector<byte_t> buffer;

std::size_t object_size(const char* s) {
    return std::strlen(s);
};

template<typename T>
std::size_t object_size(T const& obj) {
    return sizeof(obj);
};

template<typename T>
buffer serialize(const T& obj) {
    std::size_t size = object_size(obj);
    buffer buf(size);

    byte_t const* obj_begin = reinterpret_cast<byte_t const*>(&obj);
    std::copy(obj_begin, obj_begin + size, buf.begin());

    return buf;
};

template<>
buffer serialize<std::string>(std::string const& str) {
    return serialize(str.c_str());
};

template<typename T>
T deserialize(buffer const& buf) {
    return *reinterpret_cast<const T*>(&buf[0]);
}

template<>
std::string deserialize<std::string>(buffer const& buf) {
    return deserialize<char*>(buf);
}

int main() {
    using std::cout;

    int x = 97;
    buffer c = serialize(x);
    cout << deserialize<int>(c) << "\n";

    char g = 'g';
    buffer c2 = serialize(g);
    cout << deserialize<char>(c2) << "\n";

    std::string k = "blabla";
    buffer c3 = serialize(k.c_str());
    cout << deserialize<char*>(c3) << "\n";

    std::string ka = "string";
    buffer c4 = serialize(ka);
    cout << deserialize<std::string>(c4) << "\n";
}

… but like I said, this solution doesn’t generalise and is pretty useless in practice. Have a look at Boost.Serialization for a proper implementation. Unfortunately this is quite a complex problem.


strTok function (thread safe, supports empty tokens, doesn't change string) I'm new to C language and want to explode a string like we do in PHP explode() function, I searched for a built-in function with the C standard library, and I found strtok , but It doesn't support empty tokens like 1,2,3,,5 . Inspired by the answers I found in this SO question I made this function, it is supposed to be thread safe and support empty tokens and doesn't change the original string

char* strTok(char** newString, char* delimiter)
{
    char* string = *newString;
    char* delimiterFound = (char*) 0;
    int tokLenght = 0;
    char* tok = (char*) 0;

    if(!string) return (char*) 0;

    delimiterFound = strstr(string, delimiter);

    if(delimiterFound){
        tokLenght = delimiterFound-string;
    }else{
        tokLenght = strlen(string);
    }

    tok = malloc(tokLenght + 1);
    memcpy(tok, string, tokLenght);
    tok[tokLenght] = '\0';

    *newString = delimiterFound ? delimiterFound + strlen(delimiter) : (char*)0;

    return tok;
}

I designed it to be used like

char* input = "1,2,3,4,5,6,7,,,10,";
char** inputP = &input;
char* tok;
while( (tok=strTok(inputP, ",")) ){
    printf("%s\n", tok);
}

delimiterFound + strlen(delimiter) sounds like a bug. If the delimiter is longer than one character, *newString will point too far into the original, maybe even beyond the end. Correct me if I am wrong, delimiterFound + 1 is what you are actually after.
Modern C allows, and strongly encourages, to declare variables as close to their use a possible. Consider

char * delimiterFound = strstr(string, delimiter);
....
char * tok = malloc(tokLenght + 1);

etc.
Always test that malloc didn't fail.
More spaces - around keywords, braces, etc - definitely improve readability:

    if (....) {
        ....
    } else {
        ....
    }


Truncating an integer from left to right and right to left Project Euler problem 37 says:

  The number 3797 has an interesting property. Being prime itself, it is possible to continuously remove digits from left to right, and remain prime at each stage: 3797, 797, 97, and 7. Similarly we can work from right to left: 3797, 379, 37, and 3.

Going right to left is trivial enough as you just keep dividing by 10:

IEnumerable<long> RTL (long x)
{
    while(x > 10) {
        x /= 10;
        yield return x;
    }
}

Going left to right was a little trickier. Just wondering if there's a more optimal way to do this.

IEnumerable<long> LTR (long x)
{
    while(x > 10)
    {
        long y = x;        //take a copy of current value of x;
        int powerten = 0;  //variable to count what the 10^n value is

        while(y > 10){     //calculate powerten by repeated dividing y / 10
            y /= 10;       // for 3797 this would be '3' i.e. 3.797 * 10^3
            powerten++;
        }

        long p = (long)Math.Pow(10, powerten); // 10 ^ 3 = 1000
        long z = (x/p)*p;                      // 3797/1000*1000 = 3000
        x = (x - z);                           // subtract from x
        yield return x;
    }
}

It runs pretty fast but I'm just wondering is there a more optimal way to remove the leftmost significant digit of a number on each iteration without all the "gymnastics".

I would suggest returning the truncations in order of increasing lengths for truncation from the left (checking the shorter tails for primality is faster, so if the number is not a left-truncatable prime, that should be detected faster on average when the tails are yielded in that order),

IEnumerable<long> LTR (long x)
{
    long powerOfTen = 10;
    while(powerOfTen <= x)
    {
        long tail = x % powerOfTen;
        powerOfTen *= 10;
        yield return tail;
    }
}

is nice and short if you know that the argument x is never so large that the next larger power of 10 exceeds long range. I f x may be that large, you need to guard against overflow, slightly longer:

IEnumerable<long> LTR(long x)
{
    // if (x < 10) return ??;
    long bound = x/10, lastPowerOfTen = 1;
    while(lastPowerOfTen <= bound)
        lastPowerOfTen *= 10;      // doesn't overflow
    }
    long powerOfTen = 1;
    do
    {
        powerOfTen *= 10;
        yield return x % powerOfTen;
    }while(powerOfTen < lastPowerOfTen);
}

If you want to return the truncations in order of decreasing lengths, you can easily modify the last:

IEnumerable<long> LTR(long x)
{
    // if (x < 10) return ??;
    long bound = x/10, powerOfTen = 1;
    while(powerOfTen <= bound)
        powerOfTen *= 10;      // doesn't overflow
    }
    while(powerOfTen > 1)
    {
        x %= powerOfTen;
        powerOfTen /= 10;
        yield return x;
    }
}

Note: If the decimal representation of x contains at least one zero other than the last digit, these would all return the same value more than once. Since no (right or left) truncatable prime can have a zero digit in its decimal expansion, I did not care about that. If you do, in the last (decreasing length) version, you could easily avoid that by replacing

powerOfTen /= 10;

with a loop

do {
    powerOfTen /= 10;
}while(powerOfTen > 1 && powerOfTen > x);

In the increasing-length versions, that would not be as simple.

Further note:

long p = (long)Math.Pow(10, powerten);

Don't use floating point functions for integer arithmetic. In this case, if the called Math.Pow function operates on doubles [I'm not sure how that is in C#], it will not lead to wrong results, but it could produce wrong results if it operates on floats, and also in general (for other bases) even if double is used.


HTML page compiling LaTeX code to nice mathematics display - 2nd attempt I have this page that allows a user to quickly compile \$\LaTeX\$ code into a mathematical display. Whenever the user compiles the code, that page also generates an URL from which it is possible to view the output simply by visiting the URL:

index.html (Note that this Stack Snippet generates non-working URLs because it is on a different domain):

<!DOCTYPE html>

        <title>LaTeX console</title>
        <meta charset="UTF-8">

Type in the LaTeX code and press Compile!

                {06f672e0-065e-445d-8e16-0790142de8ab}

Input area

            <textarea id="latex_input" rows="5" cols="80">{bcd26d9d-9bcd-40d8-9357-ef17752144a1}</textarea>

            <button type="button" id="compile_button">Compile!</button>

URL to share

            <textarea id="encoded_url_textarea" rows="5" cols="80"></textarea>

As always, any critique is appreciated.

I'll just go through this as I find issues:

Don't declare your main dependency as async. In case the server goes down that makes your page look stupid. What you should do instead is check that the script was actually loaded before trying to use it.
JS standard naming convention for functions is camelCase. see the js-standard api as well as every major js framework. This is not python: get_url_parameter \$\rightarrow\$ getUrlParameter.
Same applies for variable names throughout your code.
Use 'use strict' to force compliance to new JS standards. And use it in the largest scope you control alone:

function getUrlParameter(param) {
    'use strict'
    let pageUrl =  // ...

I like how you encapsulated the parameter getting into it's own method. I also like how you use standard functions to URI-encode correctly instead of rolling your own encoding
Last time I checked (which is a while ago) it was common to wrap top-level code into a self-executing function like this:

(function() {
    // ...
})();

the main reason why is outlined here on SO
use let over var to communicate mutability intents to other developers (and the script engine). Also don't rely on implicit variable declaration. that's cheap :)
I never understood the point of aligning an assignment operation's right hand side :/
You can make things a tad easier when creating the encodedUrlTextarea.value by using template interpolation:

encodedUrlTextarea.value = `${location.protocol}//${location.host}${location.pathname}?code=${encodeURIComponent(latexInput.value)}`;

This can be made a bit cleaner with some extraction:

let baseUrl = `${location.protocol}//${location.host}${location.pathname}`;
let code = encodeURIComponent(latexInput.value);
encodedUrlTextarea.value = `${baseUrl}?code=${code}`;

As we can see, the creation of baseUrl can be pulled out of the function. It won't change during the script lifetime. Take advantage of that. It's definitely debatable whether the call to encodeURIComponent now really needs to be outside the template string


Unit testing for a blog post model on Google App Engine It is my second day into software testing and I am currently testing my database, handlers, and functions. I am wondering if I am taking the right approach as this is the first time testing my web app. I'd love to get some advice or feedback to improve my tests!

Below is the code for data model.

class Post(ndb.Model):
    content = ndb.StringProperty(required = True)
    user_key = ndb.KeyProperty(User)
    created = ndb.DateTimeProperty(auto_now_add = True)
    last_modified = ndb.DateTimeProperty(auto_now = True)
    user = ndb.StringProperty(required = True)
    city = ndb.StringProperty()
    coords = ndb.GeoPtProperty()

    def render(self):
        self._render_text = self.content.replace('\n', '<br>')
        params = dict(p = self)

        return render_str("post.html", **params)

    def as_dict(self):
        time_fmt = '%c'
        if self.coords is None:
            coords = "None"
        else:
            coords = self.coords.lat, self.coords.lon
            lat, lon = coords[0], coords[1]
            coords = {'latitude': lat, 'longitude': lon}

        d = {'user': self.user,
             'content': self.content,
             'city': self.city,
             'coords': coords,
             'created': self.created.strftime(time_fmt),
             'last_modified': self.last_modified.strftime(time_fmt)}
        return d

Here's the function.

def make_secure_val(val):
    return '%s|%s' % (val, hmac.new(secret, val).hexdigest())

Here's my tests

def test__PostModel(self):
    post_content = "test content"
    user = "testUser"
    city = "test city"
    u_key = ndb.Key('User', int("123"), parent=users_key())

    test_model = Post
    p = test_model(parent=post_key(), content=post_content, user=user, user_key=u_key, city=city)
    key = p.put()

    p2 = key.get()

    self.assertEqual('test content', p2.content)
    self.assertEqual(u_key, p2.user_key)
    self.assertEqual('testUser', p2.user)
    self.assertEqual('test city', p2.city)

    try:
        p = test_model()
        key = p.put()
        result = True
    except:
        result = False

    self.assertFalse(result)

def test__get_coords(self):
    coords = "43.653226,-79.383184"
    lat, lon = coords.split(",")
    coords = {"latitude": lat, "longitude": lon}
    coords = json.dumps(coords)
    return coords

    assertIs({"latitude": "43.653226", "longitude": "-79.383184"}, coords)

def test__make_secure_val(self):
    val = "test"
    secret ="testsecret"
    test_result = '%s|%s' % (val, hmac.new(secret, val).hexdigest())

    self.assertEqual("test|e91f3aaeb48b71fc68bddc8ea34dda24", test_result)

try:
    p = test_model()
    key = p.put()
    result = True
except:
    result = False

self.assertFalse(result)

This except clause catches any exception. What you probably intend to test was whether a specific exception occured, so catch that exception and none else.

If you want to make full use of the unittest library (which is what I assume you're using), you can rewrite this block with assertRaises as follows:

with self.assertRaises(YourExpectedException):
    p = test_model()
    key = p.put()

Now, the test__get_coords method doesn't make any sense to me. It unconditionally returns some JSON before it has any chance to assert anything at all. And even after removing the return statement, the code still asserts that some JSON is a dict object. Something is really finshy here!

test__make_secure_val is along the same lines. You're not actually testing the function but implementing it again, which is then what you test. You should probably change it to something like this:

def test__make_secure_val(self):
    test_result = make_secure_val(val)

    self.assertEqual("test|e91f3aaeb48b71fc68bddc8ea34dda24", test_result)

If you want to pass the secret to make_secure_val, add it to the parameter list:

def make_secure_val(val, secret):
    return '%s|%s' % (val, hmac.new(secret, val).hexdigest())


Use filter_var_array on whole array, or inside iteration I have a MySQL result set in a PHP multidimensional array.

I want to iterate through it and output it sanitized. Considering the two alternatives below, are there any performance differences?

filter_var_array on the whole result set before iterating:

$sql_rows = filter_var_array($sql_rows, FILTER_SANITIZE_FULL_SPECIAL_CHARS);
foreach ($sql_rows as $sql_row) {
    print '
' . $sql_row['first_name'] . $sql_row['last_name'] . '
';
}

filter_var_array on each row inside the iteration:

foreach ($sql_rows as $sql_row) {
    $sql_row = filter_var_array($sql_row, FILTER_SANITIZE_FULL_SPECIAL_CHARS);
    print '
' . $sql_row['first_name'] . $sql_row['last_name'] . '
';
}

Is this really the bottleneck in you application? If not choose the readable one. See Effective Java, 2nd Edition, Item 55: Optimize judiciously I know that this is a Java book but this chapter (and many others) is useful for every developer.

Some other notes:

I'd call the filtered variable something which shows that the data has been filtered, like $filtered_row. It would improve readability and help maintainers.
The code prints the first name and the last name without any separator character. I'd consider printing a separator there:

print '<p>' . $sql_row['first_name'] . ' ' . $sql_row['last_name'] . '</p>';

If you use only the first_name and the last_name maybe you should only filter these fields not the whole array. (I don't know whether it contains other values or not.)
Consider memory usage too. If I'm right the first one have to store the input and output array in the memory during the filtering which needs two times of the array size of memory temporarily. In the second case it's much smaller since it filters only one row at a time so it needs memory for the array once and two times of the size of a row during the filtering.