Two filters for products on a page I'm using two category filters from this demo. I give the filters two different sets of classes so they won't affect each other. Since I'm very new to jQuery, all I can come up with is repeating the same code with a different set of classes. I wonder if there's a shorter way to write this:

$(function(){

    /* filter1 */
    $(".category-menu ul li").click(function(){
        var CategoryID = $(this).data('category');
        $('.category-menu ul li').removeClass('cat-active');
        $(this).addClass('cat-active');

        $('.prod-cnt').each(function(){
            if(($(this).hasClass(CategoryID)) == false){
               $(this).css({'display':'none'});
            };
        });
        $('.'+CategoryID).fadeIn();

    });

    $('.category-menu select').change(function(){
        var CategoryID = $(this).find('option:selected').data('category');
        $('.prod-cnt').each(function(){
            if(($(this).hasClass(CategoryID)) == false){
               $(this).css({'display':'none'});
            };
        });
        $('.'+CategoryID).fadeIn();
    });

    /* filter2 */
    $(".category-menu2 ul li").click(function(){
        var CategoryID = $(this).data('category');
        $('.category-menu2 ul li').removeClass('cat-active2');
        $(this).addClass('cat-active2');

        $('.prod-cnt2').each(function(){
            if(($(this).hasClass(CategoryID)) == false){
               $(this).css({'display':'none'});
            };
        });
        $('.'+CategoryID).fadeIn();

    });

    $('.category-menu2 select').change(function(){
        var CategoryID = $(this).find('option:selected').data('category');
        $('.prod-cnt2').each(function(){
            if(($(this).hasClass(CategoryID)) == false){
               $(this).css({'display':'none'});
            };
        });
        $('.'+CategoryID).fadeIn();
    });

});

Please check out this demo.

When you see that you're basically copy-pasting the code and changing a few values, you can usually encapsulate all the common stuff inside a function. Here:

var registerStuff = function(category, active, prod, select) {
    $(category).click(function(){
        var CategoryID = $(this).data('category');
        $(category).removeClass(active);
        $(this).addClass(active);

        $(prod).each(function(){
            if(($(this).hasClass(CategoryID)) == false){
               $(this).css({'display':'none'});
            };
        });
        $('.'+CategoryID).fadeIn();
    });

    $(select).change(function(){
        var CategoryID = $(this).find('option:selected').data('category');
        $(prod).each(function(){
            if(($(this).hasClass(CategoryID)) == false){
                $(this).css({'display':'none'});
            };
        });
        $('.'+CategoryID).fadeIn();
    });
};

registerStuff(".category-menu  ul li", 'cat-active',  '.prod-cnt',  '.category-menu  select');
registerStuff(".category-menu2 ul li", 'cat-active2', '.prod-cnt2', '.category-menu2 select');

We can now clean up the inside of that function further. For example, some things could be cached, and the second $(category) is uneccessary. The == false is a code smell, rather use a negation. The jQuery hide() method is better than spelling out the css(). Blending out the prods of the wrong category is common code, and can be factored out as well.

var registerStuff = function(category, active, prod, select) {
    var displayOnlyWantedCategory = function($prods, categoryId) {
        $prods.each(function() {
            var $this = $(this);
            if (! $this.hasClass(categoryId)) {
                $this.hide();
            }
        });
        $('.' + categoryId).fadeIn();
    };

    var $category = $(category);
    $category.click(function() {
        var $this = $(this);
        $category.removeClass(active);
        $this.addClass(active);

        displayOnlyWantedCategory($(prod), $this.data('category'));
    });

    $(select).change(function() {
        displayOnlyWantedCategory($(prod), $(this).find('option:selected').data('category'));
    });
};

The next thing to consider is your usage of classes. Your category-menu and category-menu2 are visually and functionally identical, they're only two instances of the same idea. What happens when you want to have fifteen category-menus on the same page? I wouldn't want to see the CSS for that:

 .category-menu,
 .category-menu2,
 .category-menu3,
   ...
 .category-menu14,
 .category-menu15 {
   ...
 }

Your solution does not scale.  Instead, let's think about a product-collection element, which contains a few buttons to select a category, and a few items which can be displayed. We want to limit the effect of selecting a category to the current collection only.

The resulting HTML might look like:

All

Category 1

Category 2

Item 1 [cat 1]

Item 2 [cat 2]

Item 3 [cat 1]

The jQuery initialization must take care to install the handlers only in the elements of the current collection:

$(function() {
    $('.product-collection').each(function() {
        var $collection = $(this);
        var $selectors = $collection.find('.product-collection__selector');
        var $items     = $collection.find('.product-collection__item');

        $selectors.click(function() {
            var $selector = $(this);
            var cat = $selector.data('product-collection__category');

            $selectors.removeClass('product-collection__selector--active');
            $selector.addClass('product-collection__selector--active');

            if (cat) {
                $items.each(function() {
                    var $item = $(this);
                    if ($item.data('product-collection__category') == cat)
                        $item.fadeIn();
                    else
                        $item.hide();
                });
            }
            else {
                $items.fadeIn();
            }
        });
    });
});

See it in action here: http://jsfiddle.net/Qv6QE/3/

My class names may seem excessively verbose, but they avoid namespace conflicts by following the Block-Element-Modifier naming scheme.


Quicksort implementation in TASM assembly Is this assembly (TASM) implementation of quicksort efficient, and does this code follow common best practices?

IDEAL
MODEL small
STACK 100h
DATASEG
; --------------------------
; Your variables here
; --------------------------
arr db 3,1,2
var db ?
CODESEG
proc Swap
;swap two values
;1,2 values to be swaped
push bp
mov bp,sp
push di
push bx
push ax
push cx
xor ax,ax
xor cx,cx
mov bx,[bp+6];value to be swapped
mov di,[bp+4];
mov ah,[bx]
mov ch,[di]
mov [bx],ch
mov [di], ah
pop cx
pop ax
pop bx
pop di
pop bp
ret
endp Swap
proc Split
;sorting the array by valus greater then the pivot and values lower then the pivot
;1. start of the array
;2.end of the array
;3. pivot
push bp
mov bp,sp
push ax
push bx
push cx
push dx
push di
mov bx,[bp+8];start of the array(offset)
mov di,[bp+6]; end of the array(offset)
mov ax,[bp+4];pivot
push di ;offset to be swapped
add ax,bx
push ax ;offset to be swapped
call Swap
mov ax,bx
mov cx,di
sub cx,bx
inc cx
lpivot:
    mov dh,[byte ptr bx]
    cmp dh,[byte ptr di]
    jae greater
    push ax;offset to be swapped
    push bx;offset to be swapped
    call Swap
    inc ax
    greater:
    inc bx
loop lpivot
push ax;offset to be swapped
push di;offset to be swapped
call Swap
mov [bp+6],ax
pop di
pop dx
pop cx
pop bx
pop ax
pop bp
ret 2
endp Split
proc QuickSort
;sort an array(quicksort implention) usually around O(n log n) complexiy worst case O(n^2)
;1.strat of the array(offset)
;2.end of the array (offset)
push bp
mov bp,sp
push ax
push bx
push cx
push di
mov bx,[bp+6];start of the array(offset)
mov di,[bp+4];end of the array(offset)
cmp bx,di
jae return ;if the array size is 1 or 0 return
push bx;start of the array(offset)
push di;end of the array(offset)
call PickPivot
pop ax;pivot
push bx;start of the array(offset)
push di;end of the array(offset)
push ax;push pivot
call Split
pop ax;new pivot
push bx;start of the array(offset)
dec ax
push ax
call QuickSort
inc ax
inc ax
push ax
push di
call QuickSort
return:
pop di
pop cx
pop bx
pop ax
pop bp
ret
endp QuickSort
proc PickPivot
;1.strat of the array(offset)
;2.end of the array (offset)
push bp
mov bp,sp
push bx
push di
mov bx,[bp+6];start of the array(offset)
mov di,[bp+4];end of the array(offset)
add bx,di
shr bx,1
mov [bp+6],bx
pop di
pop bx
pop bp
ret 2
endp PickPivot
start:
mov ax, @data
mov ds, ax
xor ax,ax
mov bx,offset arr
mov di,offset var
dec di
push bx
push di
call QuickSort
exit:
mov ax, 4c00h
int 21h
END start

It feels like the first comment I make for every assembler code review is the same: Add comments.

This may seem silly to you, after all, you know exactly what the code does.  It's obvious, isn't it?  But 6 weeks from now or 6 months from now, even 6 years from now, will it still be obvious to you what this code does?  And that's just you, the person who wrote it.  Reading someone else's assembler is always a challenge.

You are asking for a code review, and I would never sign off on code (assembler or otherwise) that didn't have decent comments describing what it does.  If nothing else, performing that last pass thru the code describing what everything does sometimes turns up bugs or inefficiencies.

I realize this may just be a class project, or even just written for your own interest/education.  But it's a habit you should acquire.  Your future self (and the people you ask to read your code) will thank you.

So, let's start at the top with some file-scope comments.  Perhaps something like this:

; qs.asm - Routine to perform a quicksort on a hard-coded
;          list of numbers.
;
; Written for 16bit on i386.
;
; The list of numbers to sort is stored in the variable arr.

And looking at your first procedure, perhaps something like this:

; Swap - swap 2 variables in a list
;
; On input:
; sp + 4 - Pointer to first entry to swap
; sp + 2 - Pointer to second entry to swap
;
; On output:
; nothing
;
; Registers clobbered
; none

proc Swap

push bp     ; Create stack frame
mov bp, sp

push di     ; Preserve all used registers
push bx
push ax
push cx

xor ax, ax   ; Zero register (not sure why you do this?)
xor cx, cx

mov bx, [bp+6]   ; Get pointer to first entry
mov di, [bp+4]   ; Get pointer to second entry

mov ah, [bx]     ; Read first value
mov ch, [di]     ; Read second value

mov [bx], ch     ; Write second value to first location
mov [di], ah     ; Write first value to second location

pop cx     ; Restore used registers
pop ax
pop bx
pop di

pop bp     ; Restore stack
ret

endp Swap

This is the same code as you posted above, just with comments and spacing.  It makes the code drastically easier to understand and follow.  And it's not like it makes the .exe file any bigger.  It also highlights one of those inefficiencies I mentioned (why are you zeroing ax and cx?).

There are several other changes I would make to this routine.

You do standard stack-frame code (push bp mov bp, sp), but why?  There is no benefit to doing this here, and it will make (what is likely to be) the busiest part of your code that much slower.
Using xchg, you can use 1 fewer register.
Your routine uses pascal (or cdecl) calling convention (who pops the registers that are passed to Swap?).  There is nothing 'wrong' with calling functions this way, but there are alternatives.  If you wanted to, you could follow the 'fastcall' convention, which passes parameters in registers instead of on the stack.  Not having to read/write memory is typically faster, and requires fewer instructions (which is also faster).  The fastcall rules also allow you to use ax without having to preserve it (since it's intended to be the return value).  The downside is that if you call a routine that uses this convention, you need to make sure you don't mind having it stomp on the values of any of these registers.

I don't have a 16bit environment to test this, but given the comments, I expect you'll understand the intent, even if I muck up the syntax somewhere.

Implementing 1 & 2:

proc Swap

push ax     ; Preserve all used registers
push bx
push cx

mov bx, [sp+4]   ; Get pointer to first entry
mov cx, [sp+2]   ; Get pointer to second entry

mov al, [bx]     ; Read first value
xchg [cx], al    ; xchg first value and second value
mov [bx], al     ; Write second value back to first location

pop cx     ; Restore used registers
pop bx
pop ax

ret

endp Swap

Note that xchg doesn't allow you to use 2 memory addresses, so you still need ax.  But this still uses fewer registers, so needs fewer push/pops.

How about implementing #3?  Instead of pushing the two pointers to be swapped onto the stack, what if we pass them in cx and dx?  This is the normal 'fastcall' method on 32bit (IIRC, the standard for 16bit was a bit different).  Likewise, ax is used to return the 'result.'  In our case, there is no result, so we can use the register however we want.

Is changing the way functions are called kosher?  Well... kinda.  Assembler allows you to pass parameters however you want (as long as both the 'caller' and the 'callee' agree upon the rules), and the CPU certainly isn't going to care.  On the other hand, if every routine you write uses a different method, it's going to make reading your code a nightmare (even with comments).

Generally what you want to do is pick 1 standard and use it for most or all of your code.  Picking one of the industry standards makes sense.  Obviously if your code needs to be callable from C, you must pick a standard that C supports.  But since your entire program is asm, that doesn't apply here.

In this example, I'm using the Microsoft Fastcall standard for 32bit code.  It's one I'm familiar with (although most of my recent code is 64bit).  Obviously before you can use this, you'd need to change the code that calls Swap.

So, that gives us:

; Swap - swap 2 variables in a list
;
; On input:
; cx - Pointer to first entry to swap
; dx - Pointer to second entry to swap
;
; On output:
; nothing
;
; Registers clobbered
; ax

proc Swap

mov al, [cx]     ; Read first value
xchg [dx], al    ; xchg first value and second value
mov [cx], al     ; Write second value back to first location

ret

endp Swap

You know what?  The code is getting pretty small here.  In fact, does it still make sense to have a routine for this?  Or should you just put this code in Split?  Putting this code 'inline' would make Split a bit bigger (since you call it 3 times), but avoiding the call/ret instructions can make things even faster.  That's a judgement call.

I realize I've only covered the first routine, but this answer is already 5 pages long.  That seems like enough to get you started.

Main takeaways should be:

Comments.
Either remove stack frames, or document why they are needed.
Decide what calling convention to use.  They each have benefits (preserve all registers, faster, callable from C, etc), so decide what is important to your program.
Consider inlining smaller routines.

While Cody never responded to my request for clarification, he does tend to be right about these things.

Still, I'm not going to remove the xchg from this answer.  Why?  Because making this routine small makes you start thinking about inlining it.  And once you think about inlining it, you look again at this code:

mov dh,[byte ptr bx]
cmp dh,[byte ptr di]
jae greater
push ax;offset to be swapped
push bx;offset to be swapped
call Swap

So before calling Swap, we not only had the pointers, we also had the values and knew they needed to be swapped.  Instead of pushing the pointers, preserving registers, re-loading the pointers re-reading the values, and restoring the registers, we could simply have written the swapped values:

mov dh,[byte ptr bx]   ; Read the first value
mov dl,[byte ptr di]   ; Read the second value
cmp dh, dl
jae greater            ; Are they correctly ordered?
mov [byte ptr bx], dl  ; Nope.  Swap them.
mov [byte ptr di], dh

So the question of how expensive xchg is drops out of the question.  It was useful as a step along the way, even if ultimately it doesn't get used.

BTW, using a Swap routine in a higher-level language like C still makes sense.  The C compiler can optimize this code and does the inlining for you.  That way you get the benefit of clearer code (breaking out the Swap function makes the intent clear) while still getting the best performance.

Since assemblers aren't going "optimize" or "inline" for us, we as assembler programmers have to decide for ourselves where to make the trade-off between discrete functions and performance.


Get the value of the precedent day in a pivot table I have a pivot table of approximately 2 millions lines coming from a dataframe with the same structure as below:
raw = pd.DataFrame([[123456,datetime(2020,7,1),'A',10 ],
                   [123456,datetime(2020,7,1),'B',25 ],
                   [123456,datetime(2020,7,1),'C',0 ],
                   [123456,datetime(2020,7,2),'A',17 ],
                   [123456,datetime(2020,7,2),'B',23 ],
                   [123456,datetime(2020,7,2),'C',float('NaN') ],
                   [789012,datetime(2020,7,2),'A',11 ],
                   [789012,datetime(2020,7,2),'B',19 ],
                   [789012,datetime(2020,7,3),'A',8 ],
                   [789012,datetime(2020,7,3),'B',21 ]],
                   columns=['GROUP_ID','DATE', 'NAME', 'VALUE'])

    GROUP_ID    DATE    NAME VALUE
0   123456  2020-07-01   A   10.0
1   123456  2020-07-01   B   25.0
2   123456  2020-07-01   C    0.0
3   123456  2020-07-02   A   17.0
4   123456  2020-07-02   B   23.0
5   123456  2020-07-02   C    NaN
6   789012  2020-07-02   A   11.0
7   789012  2020-07-02   B   19.0
8   789012  2020-07-03   A    8.0
9   789012  2020-07-03   B   21.0

As you can see, the VALUE column can be Nan.
The pivot table is created like this:
pt = raw.pivot_table(index=['GROUP_ID', 'DATE'], columns=['NAME'], values=['VALUE'])

                      VALUE
            NAME      A      B       C
GROUP_ID    DATE
123456  2020-07-01  10.0    25.0    0.0
        2020-07-02  17.0    23.0    NaN
789012  2020-07-02  11.0    19.0    NaN
        2020-07-03   8.0    21.0    NaN

The idea is to create a level 0 column VALUE_PREV where I can have the value of C for the day before.
I first did this, and it took 10 seconds:
dfA = pt.stack().unstack(level='DATE').shift(1, axis=1).stack(level='DATE')
dfA = dfA[dfA.index.get_level_values('NAME') == 'C']
dfA = dfA.unstack(level='NAME').rename(columns={'VALUE':'VALUE_PREV'})
ptA = pt.merge(dfA, how='outer', on=['GROUP_ID', 'DATE'])

                     VALUE                 VALUE_PREV
            NAME     A         B     C     C
GROUP_ID    DATE
123456  2020-07-01  10.0    25.0    0.0   NaN
        2020-07-02  17.0    23.0    NaN   0.0
789012  2020-07-02  11.0    19.0    NaN   NaN
        2020-07-03   8.0    21.0    NaN   NaN

So I was wondering if there is a quicker way to do this or at least something less heavy to write / understand?
Edit : if the VALUE C is NaN at t, VALUE_PREV C at t+1 MUST be NaN and not 0

IIUC you can check for your conditions with pd.Series.where:
pt = raw.pivot_table(index=['GROUP_ID', 'DATE'], columns=['NAME'], values=['VALUE'])

s = pt["VALUE"].groupby(level=0)["C"].shift()

pt[("VALUE_PREV","C")] = s.where(s.notnull(), np.NaN)

print (pt)

                    VALUE            VALUE_PREV
NAME                    A     B    C          C
GROUP_ID DATE
123456   2020-07-01  10.0  25.0  0.0        NaN
         2020-07-02  17.0  23.0  NaN        0.0
789012   2020-07-02  11.0  19.0  NaN        NaN
         2020-07-03   8.0  21.0  NaN        NaN


Comparing a string that can hold a numeric value I am refactoring a part of big old system developed in Java. I came across a situation where we are migrating some data types, and there will be use of both types. For compatibility, as all the old part will remain using id as an int, I simply overloaded the constructor, and the old getter now parse a integer value.

The thing is, how is the proper way to compare this data? Does this way make any sense, or is there a simpler way to compare it, considering that id can be both, a number or a word.

import java.util.ArrayList;
import java.util.List;
import org.apache.commons.lang3.StringUtils;

public class SomeItem implements Comparable<SomeItem> {
    private String id;

    public SomeItem(String id) {
        this.id = id;
    }

    public SomeItem(int no) {
        this(String.valueOf(no));
    }

    public String getId() {
        return id;
    }

    public int getNo() {
        return Integer.parseInt(id);
    }

    @Override
    public int compareTo(SomeItem that) {
        if (StringUtils.isNumeric(this.getId()) && StringUtils.isNumeric(that.getId())) {
            return this.getNo() - that.getNo();
        } else {
            return this.getId().compareTo(that.getId());
        }
    }
}

this.getNo() - that.getNo()

may violate the contract of compareTo() if the subtraction overflows or underflows which is a likely event if the compared values are integer IDs of a large set of elements.

Instead you should explicitly compare the two:

return Integer.compare(this.getNo(), that.getNo());

Another way to avoid numerical comparison altogether is to format numbers as fixed-width strings with a DecimalFormat and compare the resulting strings (including some example code at the end):

import java.text.DecimalFormat;

public class SomeItem implements Comparable<SomeItem> {

    private static final DecimalFormat fmt = new DecimalFormat();

    static {
        int len = 19; // = String.valueOf(Long.MAX_VALUE).length();
        fmt.setMinimumIntegerDigits(len);
        fmt.setMaximumIntegerDigits(len);
        fmt.setPositivePrefix("+");
        fmt.setGroupingUsed(false);
    }

    private String id;

    public SomeItem(String id) {
        this.id = id;
    }

    public SomeItem(int no) {
        this(fmt.format(no));
    }

    public String getId() {
        return id;
    }

    public int getNo() {
        return Integer.parseInt(id);
    }

    @Override
    public int compareTo(SomeItem that) {
        return this.getId().compareTo(that.getId());
    }

    public static void main(String[] args)
    {
        java.io.PrintStream o = System.out;

        SomeItem[] items = new SomeItem[] {
            new SomeItem(1),
            new SomeItem(-1),
            new SomeItem(Integer.MAX_VALUE),
            new SomeItem(Integer.MIN_VALUE)
        };

        for (SomeItem i: items)
            System.out.format("id=%s, no=%+020d%n", i.getId(), i.getNo());
    }
}

Additionally, I based the amount of digits on long instead of int to provide room for (future) growth. I would have replaced the latter by the former entirely if Java converted the return value of #getNo() silently from long to int.


Can anyone take the for loop out of this code? Can anyone refactor this code without the for loop?

<?php

function lorem($paragraph_count = 1, $times = 1, $content = '')
{
    $paragraph = array(
        "Lorem ipsum dolor sit amet ......... Fusce a ante.",
        "Praesent sit amet est. Vestibulum quis elit ......... consectetuer quis, quam.",
        "Suspendisse ac eros. Morbi ......... feugiat at, tincidunt eget, ante.",
        "Proin sit amet ......... nisi lacinia tristique.",
        "Vivamus eu nisl sit amet tortor euismod venenatis ......... Proin tortor. Integer pulvinar."
    );

    for($i = 0; $i <= $paragraph_count; $i++) $content .= "<p>{$paragraph[$i]}</p>\n";

    return str_repeat($content, $times);
}
?>

<?= lorem(2,2); ?>

The following replaces the loop using higher-order functions:

$para = function($p) { return "<p>$p</p>\n"; };
$content = implode('', array_map($para, array_slice($paragraphs, 0,  $paragraph_count)));


Implementation of Strategy Pattern (photo gallery example) I'm learning design patterns and I thought that it would be a good idea to make my examples of each pattern. This is my example of the Strategy Pattern. Can you tell me is it good and what to improve?

This is the Gallery class

    public class PhotoGallery
{
    private ICollection<string> photosNames;
    private IShareStrategy shareStrategy;

    public PhotoGallery()
    {
        this.photosNames = new List<string>();
    }

    public void AddPhoto(string photoName)
    {
        this.photosNames.Add(photoName);
    }

    private void SetShareStrategy(ShareStrategy strategy)
    {
        switch (strategy)
        {
            case ShareStrategy.onFacebook:
                this.shareStrategy = new ShareOnFacebook();
                break;
            case ShareStrategy.onInstagram:
                this.shareStrategy = new ShareOnInstagram();
                break;
            case ShareStrategy.onTwitter:
                this.shareStrategy = new ShareOnTwitter();
                break;
        }
    }

    public void SharePhoto(string photoName, ShareStrategy strategy)
    {
        if (!this.photosNames.Contains(photoName))
        {
            Console.WriteLine("Invalid photo name.");
            return;
        }

        this.SetShareStrategy(strategy);
        shareStrategy.Share(photoName);
    }
}

This is The Share Strategy Enum

public enum ShareStrategy
{
    onFacebook,
    onInstagram,
    onTwitter
}

And the Strategies classes

public class ShareOnFacebook : IShareStrategy
{
    public void Share(string photoName)
    {
        Console.WriteLine($"You shared {photoName} on Facebook.");
    }
}

public class ShareOnInstagram : IShareStrategy
{
    public void Share(string photoName)
    {
        Console.WriteLine($"You shared {photoName} on Instagram.");
    }
}

public class ShareOnTwitter : IShareStrategy
{
    public void Share(string photoName)
    {
        Console.WriteLine($"You shared {photoName} on Twitter.");
    }
}

I would use dependency injection. Something like

serviceCollection
   .AddTransient<IShareStrategy, ShareOnFacebook>()
   .AddTransient<IShareStrategy, ShareOnTwitter>()
   .AddTransient<IShareStrategyProvider, ShareStrategyProvider>();

public class ShareStrategyProvider : IShareStrategyProvider
{
   private readonly IEnumerable<IShareStrategy> _strategies;
   public ShareStrategyProvider(IEnumerable<IShareStrategy> strategies)
   {
      _strategies = strategies;
   }

   public GetStrategy(ShareStrategy sharingStrategy);
   {
      return _strategies.Single(s => s.SharingStrategy == sharingStrategy);
   }
}

edit: Read up on open/closed principle, its related


Admin method in CakePHP to add a venue to the database I have a VenuesController and an EventsController. My venues controller has an admin_add method that, well, allows an administrator to add a venue to the database. It looks like this:

public function admin_add() {
    if ($this->request->is('post')) {
        if ($this->Venue->save($this->request->data)) {
            $message = 'Venue has been created.';
            $this->response->statusCode(201);
            if ($this->request->params['ext'] == 'json') {
                $this->set('id', $this->Venue->id);
                $this->set('_serialize', array('id'));
            }
            else {
                $this->Session->setFlash($message, 'flash', array('class' => 'success'));
                $this->redirect(array('action' => 'index'));
            }
        }
        else {
            if ($this->request->params['ext'] == 'json') {
                $this->set('validationErrors', $this->Venue->validationErrors);
                $this->set('_serialize', array('validationErrors'));
                $this->response->statusCode(400);
            }
        }
    }
}

As you can see, there are a lot of if () statements which seem like code smells to me. But they’re there because in my CMS’s events section, you can add a venue inline via a modal, which sents the request to this function via AJAX.

So my question is: can I improve the above VenuesController::admin_add() method, make it more intelligent without the need for the if () statements, and just generally make it more CakePHP-ier?

Why do you have the following lines repeating:

    if ($this->Venue->save($this->request->data)) {
        if ($this->Venue->save($this->request->data)) {

I think you may try to merge them to one and the associated logic too.


Multithreaded host pinging application I'm working on a network monitoring application, that pings an unknown number of hosts. I've made a class PingHost with a function zping and I called it with the help of a timer once every 2 seconds to let the 2 pings to finish, even if one of them gets TimedOut. But I think a better solution is to generate a new thread for every ping, so that the ping of every host would be independent.

Can anyone give me a hint how to do this?

namespace pinguin
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
        }

        private void timer1_Tick(object sender, EventArgs e)
        {
            PingHost caca = new PingHost();
            PingHost caca1 = new PingHost();
            this.label1.Text = caca.zping("89.115.14.160");
            this.label2.Text = caca1.zping("89.115.14.129");
        }

    }

    public class PingHost
    {
        public string zping(string dest)
        {
            Application.DoEvents();
            Ping sender = new Ping();
            PingOptions options = new PingOptions();
            options.DontFragment = true;

            string data = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
            byte[] buffer = Encoding.ASCII.GetBytes(data);
            int timeout = 50;
            int failed = 0;
            int pingAmount = 5;
            string stat = "";
            PingReply reply = sender.Send(dest, timeout, buffer, options);
            if (reply.Status == IPStatus.Success)
            {
                stat = "ok";
            }
            else
            {
                stat = "not ok!";
            }
            return stat;
        }
    }
}

You can use SendAsync:

sender.PingCompleted += new PingCompletedEventHandler (PingCompletedCallback);
sender.SendAsync(dest, timeout, buffer, options, textLabel);

And the callback:

private static void PingCompletedCallback (object label, PingCompletedEventArgs e)
{
   if (e.Reply.Status == IPStatus.Success)
   {
      label.Text = "ok";
   }
   else
   {
      label.Text = "not ok!";
   }
}

Disclaimer: I haven't run this code. I believe the delegate will be able to access the label, but I've had issues with it in the past.


Advent of Code Day 6: toggling lights in a grid with Powershell I am using the "Advent of Code" series to help with my PowerShell education. The Day 6 puzzle has a 1000 × 1000 grid of lights. After processing instructions to turn on, turn off, or toggle rectangles of lights in the grid, how many lights are on at the end?

I am confident that this script will produce the right answer. However, it is going way too slow. I am interested in optimizing this script for performance. I am not against coming up with an alternative way to solve this problem with PowerShell.

$lightson = New-Object Collections.Generic.HashSet[string]
$strings = Get-Content C:\test\lights.txt

foreach ($string in $strings){
    $numbers = [regex]::matches($string,"\d+")
    $minX = [convert]::ToInt32($numbers[0],10)
    $minY = [convert]::ToInt32($numbers[1],10)
    $maxX = [convert]::ToInt32($numbers[2],10)
    $maxY = [convert]::ToInt32($numbers[3],10)
    $x = $minX
    $y = $miny

    write-host $string
    write-host $x $y

    if ($string -like "turn on*"){
            write-host "turn on"
            while ($x -le $maxX){
                while ($y -le $maxY){
                    $light = "$x,$y"
                    if ($light -notin $lightson){
                        [void]$lightson.Add("$x,$y")
                        #write-host $lightson.Count
                    }
                    $y +=1
                }
                $x += 1
                $y = $miny
            }
            write-host $lightson.Count "`n"
        }

    if ($string -like "turn off*"){
        write-host "turn off"
        while ($x -le $maxX){
            while ($y -le $maxY){
                $light = "$x,$y"
                if ($light -in $lightson){
                    [void]$lightson.Remove("$x,$y")
                    #write-host $lightson.Count
                }
                $y +=1
            }
            $x += 1
            $y = $miny
        }
        write-host $lightson.Count "`n"
    }

    if ($string -like "toggle*"){
        write-host "toggle`n"
        while ($x -le $maxX){
            while ($y -le $maxY){
                $light = "$x,$y"
                if ($light -in $lightson){
                    $lightson.Remove("$x,$y")
                    #write-host $lightson.Count
                }
                else{[void]($lightson.Add("$x,$y"))}
                $y +=1
            }
            $x += 1
            $y = $miny
        }
        write-host $lightson.Count "`n"
    }
}

There are a million different lights that can be on or off. This massive number is obviously causing the long process time. At the moment, I can not think of any alternatives.

I assume the Write-Host commands are to view progress/status of the script? I would remove these if you are confident the script is reliable and accurate. Maybe one status (write-host $lightson.Count) after every instruction line.

When you add a light to the list, you don't need to check if it's already in the list. Add will return True/False for its success. Either way, you won't need to check before or after. Just know that if it was on the list already, it will simply return False and carry on. Likewise when removing it from the list.

while ($x -le $maxX){
    while ($y -le $maxY){
        $light = "$x,$y"
        [void]$lightson.Add("$x,$y")
        $y +=1
    }
    $x += 1
    $y = $miny
}

You will also no longer need $light.

When toggling a light, you can again use the return value of Add or Remove:

while ($x -le $maxX){
    while ($y -le $maxY){
        If ($lightson.Remove("$x,$y")) { }
        Else {[void]$lightson.Add("$x,$y")}
        $y +=1
    }
    $x += 1
    $y = $miny
}

Every loop of ForEach is evaluating all three If conditions. You can combine these into an If/ElseIf/ElseIf so subsequent conditions are not evaluated when the first has already succeeded.

For slightly less verbose loops, you could change While loops to For loops. This didn't make a large difference in times though.

I entered the number into the website you linked, and your algorithm did give the correct answer.

$lightson = New-Object Collections.Generic.HashSet[string]
$strings = Get-Content C:\test\lights.txt

foreach ($string in $strings){
    $numbers = [regex]::matches($string,"\d+")
    $minX = [convert]::ToInt32($numbers[0],10)
    $minY = [convert]::ToInt32($numbers[1],10)
    $maxX = [convert]::ToInt32($numbers[2],10)
    $maxY = [convert]::ToInt32($numbers[3],10)

    if ($string -like "turn on*"){
        For ($x = $minX; $x -le $maxX; $x++) {
            For ($y = $minY; $y -le $maxY; $y++) {
                [void]$lightson.Add("$x,$y")
            }
        }
    } ElseIf ($string -like "turn off*"){
        For ($x = $minX; $x -le $maxX; $x++) {
            For ($y = $minY; $y -le $maxY; $y++) {
                [void]$lightson.Remove("$x,$y")
            }
        }
    } ElseIf ($string -like "toggle*"){
        For ($x = $minX; $x -le $maxX; $x++) {
            For ($y = $minY; $y -le $maxY; $y++) {
                If ($lightson.Remove("$x,$y")) { }
                Else {[void]$lightson.Add("$x,$y")}
            }
        }
    }
    write-host $lightson.Count
}


String-formatting and printing functions As a small exercise I have written some string formatting and printing functions in C++11. I would like a bit of code review, not so much on the merits of using this over something like std::stringstream or boost::format, but simply whether or not my implementation makes sense. I realize that simply performing string replacements isn't that robust and a more sophisticated version would actually parse the format string. I especially would like to know if my use of things like rvalue references, std::forward, std::move and variadic templates are correct, as these are the main features I was focusing on in this exercise.

#include <boost/algorithm/string/replace.hpp>
#include <boost/lexical_cast.hpp>
#include <iostream>
#include <string>
#include <utility>
#include <cstdio>

namespace detail
{
    template <unsigned N, class T>
    void format_impl(std::string& str, T&& item)
    {
        char find[8];
        snprintf(find, 8, "{%d}", N);
        auto replace = boost::lexical_cast<std::string>(std::forward<T>(item));
        boost::replace_all(str, find, replace);
    }

    template <unsigned N, class Head, class... Tail>
    void format_impl(std::string& str, Head&& head, Tail&&... tail)
    {
        detail::format_impl<N>(str, std::forward<Head>(head));
        detail::format_impl<N+1>(str, std::forward<Tail>(tail)...);
    }
}

template <class... Args>
std::string format(const std::string& fmt, Args&&... args)
{
    std::string temp = fmt;
    detail::format_impl<0>(temp, std::forward<Args>(args)...);
    return temp;
}

template <class... Args>
std::string format(std::string&& fmt, Args&&... args)
{
    std::string temp = std::move(fmt);
    detail::format_impl<0>(temp, std::forward<Args>(args)...);
    return temp;
}

template <class... Args>
void print(const std::string& fmt, Args&&... args)
{
    std::cout << format(fmt, std::forward<Args>(args)...);
}

template <class... Args>
void print(std::string&& fmt, Args&&... args)
{
    std::cout << format(std::forward<std::string>(fmt), std::forward<Args>(args)...);
}

int main()
{
    print("Hello, {0}! The answer is {1}.\n", "World", 42);
}

As you mention, the simple replacing is susceptible to problems if a replacement string includes a format specifier. But setting that aside, there are still a few things I would rather do differently.

Document. You should be sure to document your formatting language in comments; without reading the function, or the example in main, it's impossible to know that the formatting language uses curly braces around indices starting at zero, whether you can repeat or skip indices, or what happens if you provide an out-of-bounds index.

Avoid C functions? This may be personal style, but I would prefer to avoid snprintf in format_impl. I would instead probably use something like "{" + std::to_string(N) + "}". The resulting local could then be marked const, or the expression could go directly in the call to boost::replace_all.

Remove redundancy. Jeffrey's comment about std::string& and std::string&& overloads is half right. It should be relatively rare that you need to provide both overloads. In places that are doing template type deduction on parameter T, T&& can be deduced as either type. Here there is no deduction so that cannot happen. (That's the part of the comment that was half wrong - it didn't apply here.)

Instead we have to look at how fmt is being used. In format(const std::string&, ...) this immediately makes a mutable copy called temp. In format(std::string&&, ...), this moves the local to another mutable copy called temp. From there, both functions are identical. I assert that there should only be one function here.

What function? Well, you want to modify the parameter, so it needs to be mutable. If the parameter is not mutable, you'll have to make a copy. So that rules out a const std::string&. And as your example shows, you can't always convert to a std::string&&. So what's the right option? Plan vanilla std::string. If you call it with a const, the caller makes a copy; the function receives a copy that it is free to mutate. If you call it with an rvalue, the caller moves it and the function receives a copy that it is free to mutate. So the result?

template <class... Args>
std::string format(std::string fmt, Args&&... args)
{
    detail::format_impl<0>(fmt, std::forward<Args>(args)...);
    return fmt;
}

The same argument applies to print very similarly. The only question is if you'll want to explicitly mark the string it passes to format with a std::move, or trust your compiler to figure it out. (In this case it's simple enough I'd probably leave it out. But it does serve as implicit documentation to a future reader, so I'll show it here.)

template <class... Args>
void print(std::string fmt, Args&&... args)
{
    std::cout << format(std::move(fmt), std::forward<Args>(args)...);
}

Brush up on r-value references: Note that your implementation of print(std::string&& fmt, ...) was incorrect. In particular, one of the two uses of std::forward is misleading. Per Scott Meyers, it's useful to think of std::forward as a “conditional” std::move. It's useful in conjunction with the deduced T&& parameters, which he calls “universal references.” You use this correctly in the code phrase std::forward<Args>(args)..., which moves each movable sub-args, determined by the type of the corresponding Args, as deduced from the call site.

However the similar looking phrase std::forward<std::string>(fmt) is not conditional at all. It will never move fmt, as it is always casting fmt to an lvalue type. If you were to implement this function (which I recommended against in the previous point), you would want std::forward<std::string&&>(fmt) which is really just a long and hard to read way of writing std::move(fmt). This is correct because print's fmt is an l-value that was moved in, and now you want to move it onward.

The upshot This was a good exercise that ended up focusing a lot on std::move, std::forward, and r-value references in general. There don't appear to have been any functionality problems in your implementation, although it's always good to make sure with more than one test case. If you expand on this function's robustness and capability set (say fixing the limitations from the replace_all approach, or adding a way to escape braced indices so that they aren't substituted) that could make another interesting review. Thanks!


Collect maximum fruits in two baskets of at most 2 types of fruits The task

is taken from LeetCode

  In a row of trees, the i-th tree produces fruit with type tree[i].
  You start at any tree of your choice, then repeatedly perform the
  following steps:

  Add one piece of fruit from this tree to your baskets.  If you cannot, stop.
  Move to the next tree to the right of the current tree.  If there is no tree to the right, stop.

  Note that you do not have any choice after the initial choice of
  starting tree: you must perform step 1, then step 2, then back to step
  1, then step 2, and so on until you stop.

  You have two baskets, and each basket can carry any quantity of fruit,
  but you want each basket to only carry one type of fruit each. What is
  the total amount of fruit you can collect with this procedure?

  Example 1:

Input: [1,2,1]
Output: 3
// Explanation: We can collect [1,2,1].

  Example 2:

Input: [0,1,2,2]
Output: 3
// Explanation: We can collect [1,2,2].
// If we started at the first tree, we would only collect [0, 1].

  Example 3:

Input: [1,2,3,2,2]
Output: 4
// Explanation: We can collect [2,3,2,2].
// If we started at the first tree, we would only collect [1, 2].

  Example 4:

Input: [3,3,3,1,2,1,1,2,3,3,4]
Output: 5
// Explanation: We can collect [1,2,1,1,2].
// If we started at the first tree or the eighth tree, we would only collect 4 fruits.

  Note:

  1 <= tree.length <= 40000
  0 <= tree[i] < tree.length

My solution:

has time and space complexity of \$O(n)\$. At first I thought it was easy. But then, I got confused with the one of the test cases (i.e. I = [3,3,3,1,2,1,1,2,3,3,4];) and everything inside the else-block is a bit hacky afterwards. Maybe there is a more elegant solution to that.

/**
 * @param {number[]} tree
 * @return {number}
 */
var totalFruit = function(tree) {
    const set = new Set(tree);
    if (set.size <= 2) {
        return tree.length;
    }

    const fruits = new Set();
    let i = 0;
    let j = 0;
    let max = 0;
    let count = 0;
    while (j < tree.length) {
        if (fruits.size <= 2 && !fruits.has(tree[j])) {
            fruits.add(tree[j]);
        }

        if (fruits.size <= 2) {
            count++;
            max = Math.max(max, count);
            j++;
        } else {
            fruits.delete(tree[i]);
            const lastIndex = tree.slice(i, j - 1).lastIndexOf(tree[i]);
            i += lastIndex + 1;
            count-= lastIndex + 1;
        }
    }
    return max;
};

let I = [1,2,1];
I = [0,1,2,2];
I = [3,3,3,1,2,1,1,2,3,3,4];
console.log(totalFruit(I));

Update: I made a mistake. This should be the accurate solution:

/**
 * @param {number[]} tree
 * @return {number}
 */
var totalFruit = function(tree) {
    let max = 0, count = 0;
    for (let i = 0, first = 0, second = -1; i < tree.length; i++) {
        count++;
        if (tree[i] === tree[first]) {
            first = i;
        } else if (second === -1 || tree[i] === tree[second]) {
            second = i;
        } else {
            max = Math.max(count - 1, max);
            count = Math.abs(first - second) + 1;
            first = i - 1;
            second = i;
        }
    }
    return Math.max(count, max);
};

Time complexity \$O(n)\$ and space complexity \$O(1)\$

Your second solution is good and fast, but you asked about more elegant solution, so I am suggesting mine. Firstly, I solved this task by Python, then convert all logic into Javascript. It is a little slower, than your (20 ms) and use more memory, but I think it more straightforward and understandable.

The code only answers do not liked on this site, so I add some comparisons:

Algorithm. Both algorithms are similar but:

Mine: keeps track of every number changing positions. The start1 position changes every time the number was changed, so I always know the position, where the previous number was started. The start2 position changes only when third number occurs, so I just subtract the start2 from the current index and get the needed two number sequence length.
Your: keeps track of last occurrences the first and second numbers, so you miss their start positions, and thus, you should use the count variable for storing the length of the current two number sequence. When the third number appears, you need to calculate the value of the last uninterruptible one number sequence by Math.abs(first - second). Also, you don't know which number was last - first or second, so the Math.abs function is needed.

The way of access to array items.

Mine: uses the iterator - for...of statement and Array.entries(). It relieves us from tree[i] and tree[second] like constructions.
Your: Uses counter and array indexes to access the needed item.

The Javascript code:

var totalFruit = function(tree) {
    let n1 = -1;
    let n2 = -1;

    let start1 = 0;
    let start2 = 0;

    let maxim = 1;
    # Add extra element in the end of array (which is not occured in array)
    # to get rid of the second 'Math.max(maxim, k - start2)' call
    tree.push(-2);

    for (let [k, num] of tree.entries()) {
        if (num !== n1) {
            if (num !== n2) {
                maxim = Math.max(maxim, k - start2);
                start2 = start1;
            }

            n2 = n1;
            n1 = num;

            start1 = k;
        }
    }

    return maxim;
}

The original Python code:

class Solution:
    def totalFruit(self, tree):
        n1 = -1
        n2 = -1

        start1 = 0
        start2 = 0

        maxim = 1
        tree.append(-2)

        for k, num in enumerate(tree):
            if num != n1:
                if num != n2:
                    maxim = max(maxim, k - start2)

                    start2 = start1

                n2 = n1
                n1 = num

                start1 = k

        return maxim


Python3 - merge sort, O(n) space efficiency Any critique of my implementation of Merge sort would be much appreciated!  I tested it using a driver function (shown below), and everything works.  However, it still feels unwieldy, I am a beginner so I really want to hear any criticisms, constructive or not :)
def inplace_merge_sort( lst, start = 0 , end = None ):
  def inplace_merge( lst1_start, lst1_end , lst2_start, lst2_end ): #needs to take in two sets of unsorted indices
    start, end = lst1_start, lst2_end
    for _ in range( (end - start) ):
      if(lst[lst1_start] < lst[lst2_start]):
        lst1_start += 1

      else:
        lst.insert(lst1_start , lst[lst2_start])
        del lst[lst2_start + 1]
        lst1_start += 1
        lst2_start += 1

      if( lst1_start == lst2_start or lst2_start == lst2_end):
        break

    return start, end #returns indices of sorted newly sublist

  if( len(lst) == 1 or len(lst) == 0): #catches edge cases
    return lst

  if end is None: end = len(lst) #so I don't have to input parameters on first call

  length_sublist = end - start

  if( length_sublist > 1):
    start1, end1 = inplace_merge_sort( lst, start, (end + start) // 2  )
    start2, end2 = inplace_merge_sort( lst, (end + start) // 2 , end  )

    return inplace_merge(start1, end1, start2, end2)

  else:
    return start, end

Here is the test function
def inplace_driver_helper(f_n):
  def modified_list_returner( lst ):
    f_n(lst)
    return lst
  return modified_list_returner

def driver(f_n):
  # NICK I added these two test cases to catch some popular edge cases.
    assert f_n([]) == []
    assert f_n([4]) == [4]

    assert f_n([1,2,3]) == [1,2,3]

    assert f_n([3,2,1]) == [1,2,3]

    assert f_n([1,2,3,1,2,3]) == [1,1,2,2,3,3]

    assert f_n([1,2,3,1,1,2,3]) == [1,1,1,2,2,3,3]

    assert f_n([-1,0,46,2,3,1,2,3]) == [-1,0,1,2,2,3,3,46]

and when we run this,
if __name__ == '__main__':
    driver(inplace_driver_helper(inplace_merge_sort))

    print('done')

The output is 'done'!

Merge is usually O(m) time, where m is the number of elements involved in the merge. Due to your insertions and deletions, it's rather O(mn), where n is the length of the entire list. That makes your whole sort O(n^2 log n) time instead of mergesort's usual O(n log n).
You call it inplace sort, which suggests it doesn't return anything, but you do return the list if it's short and you return some start/end indices otherwise. Rather inconsistent and confusing. I'd make it not return anything (other than the default None).
Your function offers to sort only a part of the list, but you don't test that.
You use quite a few rather long variable names. I'd use shorter ones, especially i and j for the main running indices.
You insert before you delete. This might require the entire list to be reallocated and take O(n) extra space if it doesn't have an extra spot overallocated. Deleting (or popping) before inserting reduces that risk and thus increases the chance that you only take O(log n) extra space.
Mergesort ought to be stable. Yours isn't, as in case of a tie, your merge prefers the right half's next value. For example, you turn [0, 0.0] into [0.0, 0].

A modified version:
def inplace_merge_sort(lst, start=0, stop=None):
    """Sort lst[start:stop]."""

    def merge(i, j, stop):
        """Merge lst[i:j] and lst[j:stop]."""
        while i < j < stop:
            if lst[j] < lst[i]:
                lst.insert(i, lst.pop(j))
                j += 1
            i += 1

    if stop is None:
        stop = len(lst)

    middle = (start + stop) // 2
    if middle > start:
        inplace_merge_sort(lst, start, middle)
        inplace_merge_sort(lst, middle, stop)
        merge(start, middle, stop)

Oh, I renamed end to stop, as that's what Python mostly uses, for example:
>>> help(slice)
Help on class slice in module builtins:

class slice(object)
 |  slice(stop)
 |  slice(start, stop[, step])

>>> help(list.index)
Help on method_descriptor:

index(self, value, start=0, stop=9223372036854775807, /)

>>> help(range)
Help on class range in module builtins:

class range(object)
 |  range(stop) -> range object
 |  range(start, stop[, step]) -> range object
```


A more efficient/cleaner way of scrolling a div's contents in HTML My current code for scrolling an image contained within a div is:

This was designed some time ago, and I was thinking that there must be a better way of writing it now.

Since my project requires performance over looks, I want to have a high performing page, but still with little animations like this.

Is there a more efficient way of creating something like this?

Here are a couple of recommendations on the JS side if you choose not to use the CSS-only solution:

Use private vars with the var statement instead of instance vars
with the this statement unless you need to actually expose this
properties for public inspection or use by another object.
Don't make vScroll into a global - it is only used in the scope of setup so it can be defined inside of setup, and if it isn't global, you can pass vScroll.move by reference to setInterval instead of using setInterval's quote syntax, which requires an extra parsing step.
Use a cross-browser compatible document.ready like jQuery's, or in the very least use DOMContentLoaded. The onload event can wait for the entire document to load, including CSS, but DOMContentLoaded occurs when the DOM is available (will increase load speed).


Is there any flaws in my code which let me to save settings to a file? This code allows me to save some settings from my small app to the text file which later I can retrieve by using substring and delimiter.

I wonder if there's any better way of doing this. If not, how could I make my code better or more efficient?

This is my code:

using System;
using System.IO;

namespace NameSpace
{
    class LastUsed
    {
        private static string dir = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles) + @"\Folder\";
        private static string file = dir + @"\Settings.txt";
        private string text;

        public void CheckFileStatus()
        {
            if (!Directory.Exists(dir))
            {
                DirectoryInfo directory = Directory.CreateDirectory(dir);
            }
            if (!File.Exists(file))
            {
                using (FileStream fileStream = File.Create(file))
                {
                }
            }
        }

        public bool EmptyFile()
        {
            FileInfo fileInfo = new FileInfo(file);

            if (fileInfo.Length == 0) return true;
            else return false;
        }

        private void SetFileText(string writeText)
        {
            using (StreamWriter streamWriter = new StreamWriter(file))
            {
                streamWriter.Write(writeText);
            }
        }

        private string GetFileText()
        {
            using (StreamReader streamReader = File.OpenText(file))
            {
                return streamReader.ReadLine();
            }
        }

        public string Text
        {
            set
            {
                text = value;
                SetFileText(text);
            }
            get
            {
                return GetFileText();
            }
        }
    }
}

LastUsed is an awful name for a class.  The name of a class should always be a noun.  No exceptions.

Read-only fields should be marked as read-only using the readonly keyword.

CheckFileStatus is an awful name for a function. It does not give any hints as to what it means by 'File Status', nor any hints as to what it means by 'Checking' it.  Looking inside, I see that it performs operations that change the state of the system, so 'Check' is an awful choice of a name for what it does.  When a function begins with 'Check' it is supposed to return bool and to perform absolutely nothing that would change any state anywhere at all. No exceptions.

EmptyFile is not a good name for a function that checks whether a file is empty, because it may be confused with a function that actually empties the file.  Call it IsEmpty instead, and make it a property, not a function.

This is lame:

if (fileInfo.Length == 0) return true;
else return false;

Replace it with this:

return fileInfo.Length == 0;

In the SetFileText/GetFileText functions, streamWriter.Write() is not symmetrical to streamReader.ReadLine(). Consider either using streamReader.ReadToEnd(), (the preferred solution,) or using streamWriter.WriteLine() and also asserting that the text to be written does not already contain any Eolns, because if it does, then GetFileText() will not return the same string that was given on SetFileText().

The functions CheckFileStatus() and EmptyFile() expose implementation details of the class. These implementation details should not have to be exposed for a class which simply acts as a repository for a single string. It should be possible to rewrite the class to read/write the string from the Windows Registry, or from application.config, without having to change the interface of the class. So, replace those methods with some method which abstracts their usage.

And, by the way, why are you doing all this instead of just using the Windows Registry or application.config ?


Adding two roman-numeral inputs This is the first coding class I've taken and I'm not getting much feedback from the instructor. The function of this program is to take two roman numeral inputs, convert them to arabic, add them together, then convert the sum to roman numerals. I wanted to see if any of you code ninjas could give me some feedback on the solution that I've come up with for this.

I would like to note that in the class I'm taking we haven't learned about dictionaries and we have only briefly covered lists.

This is only the functional part of the code, not including the loops to catch improper inputs.

print('Enter two roman numeral number to be added together')
print()

#converting first roman numeral to arabic

user1 = input('Enter first roman numeral: ')

print()

total1 = 0

for i in user1:
    if i == 'I':
        total1 += 1
    elif i == 'V':
        total1 += 5
    elif i == 'X':
        total1 += 10
    elif i == 'L':
        total1 += 50
    elif i == 'C':
        total1 += 100
if 'IV' in user1:
    total1 -= 2
if 'IX' in user1:
    total1 -= 2
if 'XL' in user1:
    total1 -= 20

print('The decimal value of', user1, 'is', total1)
print()
#converting second roman numeral to arabic

user2 = input('Enter second roman numeral: ')

print()

total2 = 0

for i in user2:
    if i == 'I':
        total2 += 1
    elif i == 'V':
        total2 += 5
    elif i == 'X':
        total2 += 10
    elif i == 'L':
        total2 += 50
    elif i == 'C':
        total2 += 100
if 'IV' in user2:
    total2 -= 2
if 'IX' in user2:
    total2 -= 2
if 'XL' in user2:
    total2 -= 20

print('The decimal value of', user2, 'is', total2)
print()

totalSum = total1 + total2

print('The decimal sum is:', totalSum)
print()

numeral = []

# converting from arabic to roman numeral
# splits the number into integers and appends roman numeral characters to a list

while totalSum > 1:
    for i in range(len(str(totalSum))): #this loop to identify the one, tens, hundreds place
        if i == 0:
            digit = totalSum % 10
            totalSum //= 10
            if digit == 9:
                numeral.append('IX')
            if 5 < digit < 9:
                numeral.append('I' * (digit % 5))
                digit -= digit % 5
            if digit == 5:
                numeral.append('V')
            if digit == 4:
                numeral.append('IV')
            if digit < 4:
                numeral.append('I' * digit)
        if i == 1:
            digit = totalSum % 10
            totalSum //= 10
            if digit == 9:
                numeral.append('XC')
            if 5 < digit < 9:
                numeral.append('X' * (digit % 5))
                digit -= digit % 5
            if digit == 5:
                numeral.append('L')
            if digit == 4:
                numeral.append('XL')
            if digit < 4:
                numeral.append('X' * digit)
        if i == 2:
            digit = totalSum % 10
            totalSum //= totalSum
            numeral.append('C' * digit)

numeral.reverse()

print('The roman sum is: ', ''.join(numeral))

There are definitely a few things you could improve here.  First, you should create a method to convert a Roman numeral to an Arabic numeral and vice-versa:

def roman_to_arabic(number):

    total = 0

    for i in number:
        if i == 'I':
            total += 1
        elif i == 'V':
            total += 5
        elif i == 'X':
            total += 10
        elif i == 'L':
            total += 50
        elif i == 'C':
            total += 100
    if 'IV' in number:
        total -= 2
    if 'IX' in number:
        total -= 2
    if 'XL' in number:
        total -= 20

    return total

Then, you do not need to write this twice, but can call it like this:

total1 = roman_to_arabic(user1)

Another thing you could do to improve the code is use better names.  You have the inputs named as user1 and user2.  They do not represent users, but Roman numerals as strings, maybe roman_numeral1 would be a better name.  total1 also does not represent a total, but an Arabic numeral converted from a Roman numeral: arabic_numeral1.

Finally, you do not support 'D' (500) or 'M' (1000).  If you support these, you should probably add support for "CM" (900) too.  Otherwise, your code looks pretty good to me.


Basic spellchecker with File I/O This is a basic spell checking program that I wrote earlier today. It accomplishes the tasks that I had in mind, but was curious about improving my coding style and improving efficiency of my program. Any advice is appreciated. I have read through the PEP-8 style guide and some others as background information. The hard-coded wordlist.txt is a text file containing some 60 odd thousand English words that are spelled correctly.

#---------------------------------------------------------
# The "spellCheck" function determines whether the input
# from the inputFile is a correctly spelled word, and if not
# it will return the word and later be written to a file
# containing misspelled words
#---------------------------------------------------------
def spell_check(word, english):
    if word in english:
        return None
    else:
        return word

#---------------------------------------------------------
# The main function will include all of the code that will
# perform actions that are not contained within our other
# functions, and will generally call on those other functions
# to perform required tasks
#---------------------------------------------------------
def main():
    # Grabbing user input
    inputFile = input('Enter the name of the file to input from: ')
    outputFile = input('Enter the name of the file to output to: ')
    english = {}  # Will contain all available correctly spelled words.
    wrong = []  # Will contain all incorrectly spelled words.
    num = 0  # Used for line counter.

    # Opening, Closing, and adding words to spell check dictionary
    with open('wordlist.txt', 'r') as c:
        for line in c:
            (key) = line.strip()
            english[key] = ''

    # Opening, Closing, Checking words, and adding wrong ones to wrong list
    with open(inputFile, 'r') as i:
        for line in i:
            line = line.strip()
            fun = spell_check(line, english)
            if fun is not None:
                wrong.append(fun)

    # Opening, Closing, and Writing to output file
    with open(outputFile, 'w') as o:
        for i in wrong:
            o.write('%d %s\n' % (num, i))
            num += 1

main()

In python your spell_check function could, and should be written like this:

def spell_check(word, english):
    return word if word in english else None

Dict and sets has there differences, one being that membership testing is faster with sets, they also consume less memory, which will be important for the efficiency of your program.

So a pythonic way of defining "english" would be something inline with:

with open("wordlist", "r") as f:
    english = {word.strip() for word in f}

you could still define the set above for readability.

Now that you have a set that you could use for membership testing, I think it's remove the function spell check, since it adds a line of code and somewhat obscures the program. If it did something more then checked for membership, sure, bur right now, no.

In that case, you could remove the third for loop and create a generator to let the program deal with only one word at the time and further reduce memory usage.

with open("check", "r") as f:
    with open("output", "w") as output:
        wrong = (x.strip() for x in f if x.strip() not in english)
        for key, word in enumerate(wrong):
            output.write("{}: {}\n".format(key, word))

what happens here is that the two files are open at the same time, and since wrong is a generator, it will read one line, in this case word, at the time. If the currently written word is in english it yield it and write it to the file.

Pythons enumerate builtins purpose is for these kinda situations, where you want to iterate over something while still keeping track of the rounds.

You should also make it a habit to protect your main function by,

if __name__ == '__main__':
     main()

to avoid executing self written programs when importing them.

Putting it all together:

def main():

    with open("wordlist", "r") as f:
        english = {word.strip() for word in f}

    with open("check", "r") as f:
        with open("output", "w") as output:
            wrong = (x.strip() for x in f if x.strip() not in english)
            for key, word in enumerate(wrong):
                output.write("{}: {}\n".format(key, word))

if __name__ == '__main__':
    main()


Using Python to search a sorted 2D Matrix Code

Problem

class Solution:
    def searchMatrix(self, matrix, target):
        """
        :type matrix: List[List[int]]
        :type target: int
        :rtype: bool
        """

        if len(matrix) == 0 or len(matrix[0]) == 0:
            return False

        height = len(matrix)
        width = len(matrix[0])

        row = height - 1
        col = 0

        while col < width and row >= 0:
            if matrix[row][col] > target:
                row -= 1
            elif matrix[row][col] < target:
                col += 1
            else:
                return True

        return False

Any improvements you could make to this? Runtime is O(Len(row) + len(col)), space is constant.

I looked at other solutions and a lot of them used binary search, but that is O(len(col) * log(len(row)) so I don't think that's an improvement.

Unfortunately, using binary search both for rows and columns is not possible here. It fails if the last element of the first row is too small, i.e. for this input:

matrix = [[ 0,  1,  2,  3,  4,  5],
          [ 6,  7,  8,  9, 10, 11],
          [12, 13, 14, 15, 16, 17],
          [18, 19, 20, 21, 22, 23],
          [24, 25, 26, 27, 28, 29]]
target = 10

However, you could use binary search within each row for \$\mathcal{O}(n\log m)\$ runtime, compared to your \$\mathcal{O}(n + m)\$ runtime, as you noted in the question. While this is nominally worse, for the actual testcases being tested by Leetcode it performs better.

from bisect import bisect_left

class Solution:
    def searchMatrix(self, matrix, target):
        """
        :type matrix: List[List[int]]
        :type target: int
        :rtype: bool
        """
        if not matrix or not matrix[0]:
            return False
        for row in matrix:
            i = bisect_left(row, target)
            if i < len(row) and row[i] == target:
                return True
        return False

Your code: 36ms, faster than 85.35%

Binary search: 28 ms, faster than 98.39%

This is probably due to the testcases being small enough (and  if \$m = n\$, \$2n > n\log_b n\$ for \$n < b^2\$).


Copy and bit-invert ints in an std::vector I want a second copy of an std::vector in which all elements are inverted, e.g. 0x01 in the first vector should be 0xFE in the second.

Is this a good solution?

std::vector<uint8_t> first_list = {0x01, 0x10, 0x32, 0x1A };
std::vector<uint8_t> second_list = first_list;
for(std::vector<uint8_t>::iterator it = second_list.begin(); it != second_list.end(); ++it)
    *it = ~*it;

The for loop sticks out to me as unnecessarily verbose.

You can utilize std::transform here (I'll assume C++11 for the lambda):

int main()
{
    std::vector<uint8_t> first_list = {0x01, 0x10, 0x32, 0x1A};
    std::vector<uint8_t> second;
    std::transform(first_list.begin(), first_list.end(), std::back_inserter(second),
                   [](uint8_t u) { return ~u; });
}

You also don't need to make a copy of the first vector to do this; better to simply use push_back (or back_inserter here, since we're working with iterators).


Making numbers palindromic This is a challenge I found here. You're suppose to list how many steps it takes to convert a number to a palindromic number, where each step consists of adding number to the reverse of itself. For example, 196196871 takes 151 steps and gives me 3588816724276188853.

import java.util.ArrayList;
import java.util.Scanner;

public class Main {

    private Scanner in = new Scanner(System.in);

    public Main() {
        String input = in.nextLine();
        ArrayList< Long > targets = new ArrayList< Long >();

        // Checks if the ENTER key was pressed. ENTER is the exit key of this program.
        while(!input.equals(""))
        {
            targets.add(Long.parseLong(input));
            input = in.nextLine();
        }

        for(long target : targets)
            System.out.println(target + " gets palindromic after " + getPalSteps(target) + " steps: " + convertToPal(target));
    }

    public long reverse(long number) {
        long reverse = 0;
        long n = number;
        while (n != 0)
        {
            reverse *= 10;
            reverse = reverse + n % 10;
            n /= 10;
        }
        return reverse;
    }

    public boolean palindrome(long number) {
        return reverse(number) == number;
    }

    public long convertToPal(long number) {
        if (number == 196)
            return -1;

        while (!palindrome(number))
        {
            number = reverse(number) + number;
        }

        return number;
    }

    public long getPalSteps(long number) {
        int steps = 0;

        if (number == 196)
            return -1;

        while (!palindrome(number))
        {
            number = reverse(number) + number;
            steps++;
        }

        if(steps > 10000)
            System.out.println("Lychrel number!");

        return steps;
    }

    public static void main(String[] args) {
        new Main();
    }

}

When you add a number to its reverse, its value approximately doubles, on average.  If you do it 10000 times, you should get a result that is very roughly 210000 times the original. That will certainly overflow a long, which only holds values below 263.  You need to either

lower the iteration limit
check for overflow
use BigInteger
reinvent arithmetic for large numbers using strings or arrays

The use of OOP is awkward and inappropriate. Constructing an object should just involve just allocating and initializing some memory to model something. Your Main() constructor, though, is the entire input, calculation, and output loop of the program.

Your program isn't object-oriented at all. Just make everything static, and merge the constructor code into the main() function.


Remove nth node from last position in linked list Description:

Given a linked list, remove the nth node from the end of list and return its head.

  Given linked list: 1->2->3->4->5, and n = 2.

  After removing the second node from the end, the linked list becomes
  1->2->3->5.

Note:

Given n will always be valid.
Try to do this in one pass.

Code:

class Solution {
    public ListNode removeNthFromEnd(ListNode head, int n) {
        int total = 0;
        ListNode current = head;
        while (current != null) {
            total++;
            current = current.next;
        }
        // current must be null here

        // 1 based index
        int indexToRemove = total - n + 1;
        if (indexToRemove < 0) {
            return null;
        }
        if (1 == total && n == 1) {
            return null;
        }
        if (indexToRemove == 1) {
            return head.next;
        }

        current = head;
        ListNode prev = null;
        for (int i = 0; i < indexToRemove - 1; i++) {
            prev = current;
            current = current.next;
        }
        // i > indexToRemove

        if (current == null) { // last
            prev.next = null;
        }
        else {
            prev.next = current.next;
        }
        return head;
    }
}

Questions:

I managed to run the code and pass all the tests but I still feel confused while dealing with off-by-one errors and NullPointerException. The question felt quite easy at first but implementation took a lot of time because of above two errors and correcting them, it was like I was shooting in the dark.

I still feel I have written a lot of extra code which could have been avoided.

What mental model or basic rules I can learn to visualise or understand program states and avoid such errors?

You failed to solve the "try to do this in one pass" part, as you iterate through the list once to find the total size and a second time to find the nth element from the start.

The algorithmic idea is, to use two pointers into the list:

Initialize poiner1 to head, advance n times (if n is the distance from the tail you seek)
Then, initialize pointer2 to head, advance both pointers, until pointer1 is null, i.e. reached the end of the list.
Now, pointer2 should point to the end-nth element

Removal as usual.


A One-Pass Hash Table Solution to twoSum I read the guiding solution to twoSum in leetcodes ;

  Given an array of integers, return indices of the two numbers such that they add up to a specific target.

  You may assume that each input would have exactly one solution, and you may not use the same element twice.

  Example:

Given nums = [2, 7, 11, 15], target = 9,

Because nums[0] + nums[1] = 2 + 7 = 9,
return [0, 1].

  Approach 3: One-pass Hash Table
  It turns out we can do it in one-pass. While we iterate and inserting elements into the table, we also look back to check if current element's complement already exists in the table. If it exists, we have found a solution and return immediately.

and mimic a python solution

class Solution:
    def twoSum(self, nums, target) -> List[int]:
        """
        :type nums: List[int]
        :type target: int
        """
        nums_d = {}

        for i in range(len(nums)):
            complement = target - nums[i]

            if nums_d.get(complement) != None: #Check None not True Value
                return [i, nums_d.get(complement)]
            nums_d[nums[i]] = i    #produce a map
        return []

Unfortunately, my solution is only faster than 81%, which I thought was a best solution.

  Runtime: 40 ms, faster than 81.00% of Python3 online submissions for Two Sum.
  Memory Usage: 14.3 MB, less than 5.08% of Python3 online submissions for Two Sum.
  Next challenges:

How could continue to improve the code, and I am curious about the approaches of the top 20%.

Don't leave white space at the end of lines.
Use enumerate.
When comparing with None use is and is not.
It'd be cleaner to just use in rather than get.
If you want to use nums_d.get then you should use it outside the if so you don't have to use it a second time when you enter the if.
This however makes the code messy for not much of a benefit IMO.
Unless the site forces you to return lists returning a tuple would be more Pythonic.
Your comments aren't helpful, if anything they make the code harder to read for me.
The variable name nums is easier to read then nums_d, the _d is useless.
When returning it would be better to either:

Raise an exception, as the lookup failed.
Return a tuple where both values are None. This is so you can tuple unpack without errors.

Getting the code:

def test_peil(nums: List[int], target: int) -> Tuple[int, ...]:
    lookup = {}
    for i, v in enumerate(nums):
        if target - v in lookup:
            return i, lookup[target - v]
        lookup[v] = i
    raise ValueError('Two sums target not in list.')

You can further improve performance by including Alain T.'s change for small numbers.

def test_peil_alain(nums: List[int], target: int) -> Tuple[int, ...]:
    if len(nums) <= 100:
        for i, n in enumerate(nums):
            if target - n in nums:
                return i, nums.index(target - n)
    else:
        lookup = {}
        for i, v in enumerate(nums):
            if target - v in lookup:
                return i, lookup[target - v]
            lookup[v] = i
    raise ValueError('Two sums target not in list.')

And has a performance improvement: (the graph is volatile due to only using one input sample)


ButterKnife.bind with super class I am using ButterKnife in my project, and as we know we have to write ButterKnife.bind(this); in all the activities, so I have just created BaseActivity like this:

public class BaseActivity extends AppCompatActivity {

    protected void onCreate(Bundle savedInstanceState, int layout) {
        super.onCreate(savedInstanceState);
        setContentView(layout);
        ButterKnife.bind(this);
    }
}

and I am calling it from other activity like this

public class SplashActivity extends BaseActivity {

    @BindView(R.id.txtName)
    TextView txtName;

    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState, R.layout.activity_splash);
    }
}

I want to know whether it is good practice to follow this approach.

Override setContentView instead of a different onCreate that receives the layout id.

public class BaseActivity {

    protected void setContentView(Int layoutResID) {
        super.setContentView(layoutResID);
        ButterKnife.bind(this);
    }

}

Overriding this makes it more consistent (no custom onCreate) with the android style, I think.

I dont see any problem of using it, its cool since everytime you create a new activity its already there so just add @BindView and it works.

Other alternative is not using ButterKnife but Kotlin and Kotlin Extensions, take a look.

https://kotlinlang.org/docs/tutorials/android-plugin.html


A function for parsing words from a string without using whitespace I'm trying to parse words from a badly garbled text file that contains many repeats. It's about 100k characters in length and was formed from joining many substrings in alphabetical order.

I'm curious about other methods for finding words without using whitespace.

def unique_words(string):
    words = dict()
    p1 = 0 # String slice position 1
    p2 = 1 # String slice position 2
    len_string = len(string)
    while p2 < len_string:
        p2 += 1
        sub1 = string[p1:p2] # A shorter sub
        sub2 = string[p1:(p2 + 1)] # A longer sub
        sub1_count = string.count(sub1) # Counts the frequency of the shorter sub
        sub2_count = string.count(sub2) # Counts the frequency of the longer sub
        if sub2_count * len(sub2) < sub1_count * len(sub1): # True if the frequency of sub1 * its length is greater
            words[sub1] = ('') # Add
            p1 = p2
    return words

The above code works when the number of unique words is small but fails when it is large. I've used the website TextMechanic to generate a random string like

'updownleftupdowndownleftupleftrightupdownleftup'

and the above code returns a dictionary exactly as desired:

{'up': '', 'down': '', 'left': '', 'right': ''}

Here's the problem:

When the number of unique words increases, there is a point where the occurrence of single letters out numbers the total character count of any word in a string.

My current solution uses the algorithm on short slices of the original string, but this involves trial-and-error and has artifacts.

You should give your variables better names. For example, while you might know what the variable p1, or p2 does, people reading your code don't. Giving your variables better names also reduces the need for inline comments like # String slice position 2.

Rather than using the dict function to initialize an empty dictionary, you can just type the following: words = {}.

Finally, you don't need the parentheses around the '' in words[sub1] = (''). It can be changed to the following: words[sub1] = ''.


Errno-like C global error handling I'm thinking about a unified error handling interface.

I've come to conclusion that the errno global variable style isn't so bad, and decided to implement an improved version of it: every function called sets an error code to a global error variable through the get/set functions.

The difference with the errno pattern is that the code is set even if there's no actual error (ERR_OK enum value), so we get a status of an operation after each function call. This allows us to not reserve a special "error" return value. The global error variable is of thread local storage type.

What pitfalls could be met with such approach?

err.h:

#ifndef ERR_H
#define ERR_H

enum err {
    ERR_OK
    ERR_SOME,
};

void err_set(enum err e);
int err_get(void);

#define ERR (err_get() != ERR_OK)

#endif /* ERR_H */

err.c:

#include "err.h"

_Thread_local static int error;

void err_set(enum err e) {
    error = e;
}

int err_get(void) {
    return error;
}

main.c:

#include <stdio.h>
#include "err.h"

static size_t get_size(void *arg) {
    if (!arg) {
        err_set(ERR_SOME);
        return 0;
    }

    err_set(ERR_OK);
    return sizeof(*arg);
}

int main(void) {
    void *bad = NULL;

    size_t size = get_size(bad);
    if (ERR) {
        printf("Bad pointer!\n");
        return 1;
    }

    printf("size == %zu\n", size);

    return 0;
}

Error handling is indeed an issue in C with a multitude of potential handling mechanisms.  Good to see your take on it.

Allow for atomic error handling.  In addition to getting the error, it would be useful to have a test and clear to allow simpler error testing.  The point is that once upper level code tests positive for an error (the error is caught), the global test status should be immediately cleared (atomic) to allow for subsequent setting without a chance of intervening code messing up things.

bool err_test_and_clear(int *err) {
  // Simplistic code.  See also "C11 7.17 Atomics <stdatomic.h>"
  if (err) {
    *err = error;
  }
  bool e = error != ERR_OK;
  error = ERR_OK;
  return e;
}

// usage
foo();
int err;
if (err_test_and_clear(&err)) {
  foo_clean_up();  // code may cause an error
  fprintf(stderr, "Error: %d\n", err);  // report original error
}

vs.

// usage
foo();
if (ERR) {
  foo_clean_up(); // code may cause error - oops this should have been after below 2
  int err = err_get();
  err_set(ERR_OK);
  fprintf(stderr, "Error: %d\n", err);  // report maybe original error
}

Not a fan of ERR, which looks like a constant or object and it is not.  Perhaps make it look like a function. Or be emphatic that ERR_OK is 0.

// #define ERR (err_get() != ERR_OK)

// better
#define ERR() (err_get() != ERR_OK)

// or best no define, just use `if(err_get())`

If code like if (err_get() != ERR_OK) is preferred over if (err_get()), then code should use

// _Thread_local static int error;
_Thread_local static int error = ERR_OK;

Missing ,

enum err {
    ERR_OK    , // add ,
    ERR_SOME,
};

I'm leaning that _Thread_local static int error may benefit with being volatile.  Have to ponder that more.
For easier to read semantics, consider a clear function.  err_clear(); has more clarity than err_set(ERR_OK); ("setting the error" of "OK error").  Further ERR_OK looks contradictory.  Perhaps ERR_NONE?

void err_clear(void) {
  error = ERR_OK;
}

Mixed "types".  I'd expect both int or both enum err

             vvvvvvvv
void err_set(enum err e);
int err_get(void);
^^^


Look! Smoke! Who do we blame? Recently, I found a way to generate specific blame for SmokeDetector. That was cool, so I decided to make a C++ program for it. I also have it on GitHub. This program is called Blame Bot.

Blame Bot does not need any flags to compile, so you can use make blame to compile it.

How it works:

Blame Bot takes an input of a number. This input is your chat id. This number is taken from your chat link. Mine would be: https://chat.stackexchange.com/users/301688/grant-garrison, and my number would be 301688. Then, the number is converted to base 7. A switch statement then outputs the characters. (Note: the characters must be in double quotes.)

#include <iostream>

int main(){
    int digit;
    std::string stringDigit, answer, instring;
    std::cin>>instring;
    int in = std::stoi(instring,nullptr);
    while (in != 0){
        digit = in % 7;
        if (digit < 10){
            stringDigit = '0' + digit;
        }else{
            stringDigit = digit - 10 + 'A';
        }
            answer = stringDigit + answer;
            in /= 7;
    }
    std::cout<<"!!/blame\u180E ";
    int i = 0;
    while(answer.length()>=i){
        switch (answer[i]-'0'){
            case 0: std::cout<<"\u180E";
                break;
            case 1: std::cout<<"\u200B";
                break;
            case 2: std::cout<<"\u200C";
                break;
            case 3: std::cout<<"\u200D";
                break;
            case 4: std::cout<<"\u2060";
                break;
            case 5: std::cout<<"\u2063";
                break;
            case 6: std::cout<<"\uFEFF";
                break;
        }
        i++;
    }
    std::cout<<"a\n";
}

What I'm looking for:

I'm looking for efficiency and syntax improvements. I know the program is fast already, but any extra computation cut out is a bonus. I do want to leave the { and } on the if statements though, as I think it improves readability for me. I also do not want to use any libraries.

What I think can be improved:

I think that switching back and forth from std::string and int is inefficient, and I'm sure there is a better way that doesn't need that. I also think that the base conversion algorithm might have a better replacement.

Spacing

The usage of (horizontal) space is not consistent, you have a space
after if and switch, but not after else and while.
I would generally use more whitespace to increase the readability, in particular after keywords, around operators, etc. For example

while (answer.length() >= i) {

Scope

It is a good habit to declare variables at the narrowest scope where they are used, and not at the top of the function. This applies for example
to digit and stringDigit, which are only used inside the while-loop.

Input validation

std::stoi(instring, nullptr) silently ignores leading whitespace
and trailing non-digits, that might or might not be wanted.
The pos parameter could be used to check if the complete string
has been converted successfully.

Also std::stoi(instring, nullptr) throws an std::invalid_argument
exception if the input string contains no valid digits at all,
you might want to catch that.

Iterating too far

In

int i = 0;
while(answer.length()>=i){
    switch (answer[i]-'0'){
        // ...
    }
    i++;
}

the last iteration accesses answer[answer.length()], which is
the null character. Even if this causes no harm (answer[i]-'0'
then matches none of the cases) it is probably unintended and you'll
want answer.length() > i instead. (See below for further improvements
of this part.)

Simplifications

The remainder modulo 7 cannot be >= 10, so the while loop
simplifies to

while (in != 0) {
    int digit = in % 7;
    std::string stringDigit;
    stringDigit = '0' + digit;
    answer = stringDigit + answer;
    in /= 7;
}

which can further be simplified to

while (in != 0) {
    char digit = '0' + in % 7;
    answer.insert(0, 1, digit);
    in /= 7;
}

making the stringDigit variable obsolete, and mutating answer
instead of concatenating strings.

Iterating over all valid string indices is better done with a for loop:

for (int i = 0; i < answer.length(); ++i) {
    // ... use `answer[i]` ...
}

But actually you don't need the index, only the characters, and
iterating over all characters in a string can be done more simply
with a range-based for loop (added in C++ 11):

for (char& c : answer)
    // ...
}

The switch statement can be replaced by an array lookup:

std::string blameStrings[] = {
    "\u180E", "\u200B", "\u200C", "\u200D", "\u2060", "\u2063", "\uFEFF"
};
for (char& c : answer) {
    std::cout << blameStrings[c - '0'];
}

(You can probably choose a better name for the array, but I have no
idea what those strings stand for.)

Get rid of the intermediate string storage

Your code computes answer as the base-7 representation of the
input number, and then iterates over the characters in that string.

The conversion from digits to characters in a string and back to
digits can be avoided by storing the integer digits in a vector
instead. And instead of prepending additional digits, we can append
them (to avoid that memory needs to be moved repeatedly) and then iterate
over the result in reverse order:

#include <vector>

// ...

int in = std::stoi(instring, nullptr);
std::vector<int> digits;
while (in != 0) {
    digits.push_back(in % 7);
    in /= 7;
}

std::cout<<"!!/blame\u180E ";
for (auto it = digits.rbegin(); it != digits.rend(); ++it) {
    std::cout << blameStrings[*it];

}
std::cout<<"a\n";


Get array slices from list of lengths I have a list of segment lengths and I want to convert it into array slices:

segment_lens = [2, 5, 5]

last_end = 0
start = 0
end = 0

for s_i in range(len(segment_lens)):
    start = last_end
    end += segment_lens[s_i]
    print("Slice: [%s,%s]" %(start, end,))
    last_end = end

Here is the intended output, which in my application I would use to slice a list:

Slice: [0,2]
Slice: [2,7]
Slice: [7,12]

Is there a better way to do this? I feel like there should be a python built-in for this, but I'm not sure.

I would probably use iter for this:

def get_chunks(l, chunk_lengths):
    it = iter(l)
    for chunk_length in chunk_lengths:
        yield [next(it) for _ in range(chunk_length)]

Usage:

>>> get_chunks(range(100), [1,2,5])
<generator object get_chunks at 0x7f2744f68910>

>>> list(get_chunks(range(100), [1,2,5]))
[[0], [1, 2], [3, 4, 5, 6, 7]]

Review:

I would make your code at least a function that yeilds the slices and give it a nice name, like I did in the function above.

Note that it is frowned upon in python to iterate over the indices of a list, if all you need are the elements. So prefer this:

for item in l:
    print(item)

over

for i in range(len(l)):
    print(l[i])

Also note that using "%s" % var style formatting is the old way of doing it. It is recommended to use str.format, which offers a lot more possibilities (none of which you need here, though).

print("Slice: [{},{}]".format(start, end,))


Security of the login code on the website I wrote the code to log in php to the admin panel. Everything works as it should. But I am not sure if it is well written code and if it is safe. I care about security. I have read a lot about it but I am still not sure. Please, help me.
<form method="post">
   <input type="email" name="email" placeholder="Email">
   <input type="password" name="password" placeholder="Password">
   <button type="submit" name="submit">Submit</button>
</form>

LOGIN PAGE:
session_start();
session_regenerate_id();
if(isset($_POST['submit'])){
   unset($_POST['submit']);
      if(in_array('', $_POST)){
         //Errors for empty input
      } else {
         if(filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)){
            if(Check if the user exists in the database){
               if(password_verify($_POST['password'], Password field in db)){
                  $_SESSION['loggedin'] = 1;
                  header('Location: welcome');
                  exit();
               } else {
                  //Wrong login credentials
               }
            } else {
               //Wrong login credentials
            }
         } else {
            //Wrong email
         }
      }
   }
}

PAGE AFTER LOGIN:
session_start();
session_regenerate_id();
if(empty($_SESSION['loggedin'])){
   header('Location: login');
   exit();
}

The devil is in the details, and you are omitting a lot of details from your code that usually go wrong. That said, I at least saw the following issues in your code:
session_regenerate_id regenerates the... id... of the session. I am not sure why you do this, but it is not necessary and will likely cause issues where people are logged out unexpectedly on unstable networks where a page might not load for any reason. Your logged in session is also valid indefinitely. If an attacker can get their hands on a session identifier, they can keep their session alive indefinitely. You might want to add an expires_on timestamp to your session that is checked to limit the lifetime of any session.
While not wrong perse, unset($_POST['submit']); does not add anything either. It does not do anything for you security-wise at least.
Your check in_array('', $_POST) is a bit convoluted. It uses that in_array checks values, even if you pass it an array with key-value pairs instead of one with numeric indexes. It is probably the reason why you unset the submit button, but at least in the documentation it is only used with numeric indexed arrays. You might want to check if password and email are set instead with empty($_POST['password']) || empty($_POST['email']). This also guards against warnings when password is not set. A second quirk of in_array is that your check will return true with the following array due to type conversion: [ 'remember_me' => 0 ].
I am not sure why you use filter_var($_POST['email'], FILTER_VALIDATE_EMAIL), since the email is already in the database and you should be using prepared queries that are resistent against sql injection through any user input. FILTER_VALIDATE_EMAIL lets through a lot more than you expect, and if you are relying on that function to prevent sql injection, you might be vulnerable. Since you didn't share that part of the code I cannot comment on if your code is vulnerable in that regard.
The second argument of password_verify($_POST['password'], Password field in db) is a hash, not a database field name. If you meant that you retrieve it from the database, it is fine.
You do not store who is logged in and instead only set if someone is logged in $_SESSION['loggedin'] = 1;. If you do not care who is logged in, the email part of the login is redundant and you should just remove it. It does not really add any security.
All else-es in your code should be combined into one message. You do not want to give a would-be attacked any information on what part they should improve to get into your website. Just give a generic message "your email or password is incorrect".

That said, the functions that you do use do not return anything else than the expected booleans as far as I know. You correctly exit after sending your location header. I also don't see a way to set being logged in outside sending a username and password through the login page.


More efficient update macro in Excel I have a macro that makes comparisons and then this macro exports all of the changes based on if the information doesn't match. I have it so that each column gets their own worksheet in the new workbook. I am using 7 different counting integers and it takes a very long time because I am exporting over 60k rows.

Question: is there a faster way to execute this code? Can a UDF be used? if so how?

Dim ws As Worksheet
Dim wb2 As Workbook
Set wb = Application.Workbooks("Total Database Update_WORKING.xlsm")
Set ws = wb.Worksheets("Results")
Set wb2 = Application.Workbooks.Open("C:\Import Update.xlsx")
    i = 2
    ii = 2
    iii = 2
    iiii = 2
    iiiii = 2
    iiiiii = 2
    iiiiii = 2
    k = 2
    wb2.Activate
    Do While ws.Cells(k, 1) <> ""
        If ws.Cells(k, 4) = "No Match" Then
            wb2.Worksheets("AD UPDATE").Cells(i, 1) = ws.Cells(k, 1)
            wb2.Worksheets("AD UPDATE").Cells(i, 2) = ws.Cells(k, 2)
            i = i + 1
        End If
        If ws.Cells(k, 7) = "No Match" Then
            wb2.Worksheets("SENIOR UPDATE").Cells(ii, 1) = ws.Cells(k, 1)
            wb2.Worksheets("SENIOR UPDATE").Cells(ii, 2) = ws.Cells(k, 5)
            ii = ii + 1
        End If
        If ws.Cells(k, 10) = "No Match" Then
            wb2.Worksheets("ID UPDATE").Cells(iii, 1) = ws.Cells(k, 1)
            wb2.Worksheets("ID UPDATE").Cells(iii, 2) = ws.Cells(k, 8)
            iii = iii + 1
        End If
        If ws.Cells(k, 13) = "No Match" Then
            wb2.Worksheets("MINOR UPDATE").Cells(iiii, 1) = ws.Cells(k, 1)
            wb2.Worksheets("MINOR UPDATE").Cells(iiii, 2) = ws.Cells(k, 11)
        End If
        If ws.Cells(k, 16) = "No Match" Then
            wb2.Worksheets("MAJOR UPDATE").Cells(iiii, 1) = ws.Cells(k, 1)
            wb2.Worksheets("MAJOR UPDATE").Cells(iiii, 2) = ws.Cells(k, 14)
            iiii = iiii + 1
        End If
        If ws.Cells(k, 19) = "No Match" Then
            wb2.Worksheets("CAP UPDATE").Cells(iiiii, 1) = ws.Cells(k, 1)
            wb2.Worksheets("CAP UPDATE").Cells(iiiii, 2) = ws.Cells(k, 17)
            iiiii = iiiii + 1
        End If
        If ws.Cells(k, 22) = "No Match" Then
            wb2.Worksheets("PL UPDATE").Cells(iiiiii, 1) = ws.Cells(k, 1)
            wb2.Worksheets("PL UPDATE").Cells(iiiiii, 2) = ws.Cells(k, 20)
            iiiiii = iiiiii + 1
        End If
        k = k + 1
    Loop

    wb2.Save
    Sleep (1000)
    wb2.Close SaveChanges:=True
    wb.Activate
End Sub

What are the chances that more than one of your if statements would be true for each row?  It looks like you may risk overwriting some of your data if that is the case.

Dim ws As Worksheet
Dim wb2 As Workbook
Set wb = Application.Workbooks("Total Database Update_WORKING.xlsm")
Set ws = wb.Worksheets("Results")

Using variant your array will be able to size to whatever range you give it, but it will be 1 based.

Dim rng as Variant
Set rng = wb.worksheetS("Results").Range("B2:Your last column/row goes here")
Set wb2 = Application.Workbooks.Open("C:\Import Update.xlsx")
    i = 2
    ii = 2
    iii = 2
    iiii = 2
    iiiii = 2
    iiiiii = 2
    iiiiii = 2
    k = 2
    wb2.Activate
    dim row as long
    dim col as long
    For row = 1 to UBound(rng, 1)
        If rng(row, 4) = "No Match" Then
            wb2.Worksheets("AD UPDATE").Cells(i, 1) = rng(row, 1)
            wb2.Worksheets("AD UPDATE").Cells(i, 2) = rng(row, 2)
            i = i + 1
        End If
        k = k + 1
    Next Row

    wb2.Save
    Sleep (1000)
    wb2.Close SaveChanges:=True
    wb.Activate
End Sub