Permalänk

Feedback på min kod (Färdig)

blev precis klar med uppgift 14.1 i arbetsboken om programmering. Tänkte se ifall det var någon som har feedback att ge då jag sitter hemma och knappar helt ensam och vet inte alltid om min kod är så snygg den kan vara.

Uppgifts anvisning: Skapa en klass som hanterar personer med namn och personnummer. Skapa sedan en lista med själp av klassen list. användaren ska i programmet kunna lägga till personer till listan, samt söka på personnummer i listan. När användaren söker ska namnet och index för personern skrivas ut.

class Program { static void Main(string[] args) { List<PersonDatabas> Databas = new List<PersonDatabas>(); while (true) { System.Console.WriteLine("===Person databas==="); System.Console.WriteLine("Ange svars allternativ"); System.Console.WriteLine("________________________"); System.Console.WriteLine("1: Lägg till person i databasen"); System.Console.WriteLine("2: Sök på ett person nummer"); System.Console.WriteLine("3: Sök på person med namn"); System.Console.WriteLine("4: Skriv ut alla medlemmar i databasen"); int svar = System.Convert.ToInt32(System.Console.ReadLine()); switch (svar) { case 1: { System.Console.Write("Ange personens namn: "); string namn = System.Console.ReadLine(); System.Console.Write("Ange personens person nummer:"); long personNr = System.Convert.ToInt64(System.Console.ReadLine()); PersonDatabas personDatabas = new PersonDatabas(namn, personNr); Databas.Add(personDatabas); break; } case 2: { System.Console.Write("Sök på en person med personnummer: "); long sökning = System.Convert.ToInt64(System.Console.ReadLine()); foreach (var element in Databas) { if (sökning == element.personNr) { int index = (Databas.IndexOf(element) + 1); System.Console.WriteLine(index + ": " + element.name + ": " + element.personNr); } else { System.Console.WriteLine("Personen är inte inlaggd"); } } break; } case 3: { System.Console.Write("Ange personens namn: "); string namn = System.Console.ReadLine(); foreach (var person in Databas) { if (string.Compare(namn, person.name) == 0) { System.Console.WriteLine(person.name + ": " + person.personNr); } else { System.Console.WriteLine("Personen är ej inlaggd"); } } break; } case 4: { System.Console.WriteLine("Skriver ut alla medlemmar: "); foreach (var element in Databas) { System.Console.WriteLine(element.name + ": " + element.personNr); } break; } } } } } class PersonDatabas { public string name; public long personNr; public PersonDatabas(string name, long personNr) { this.name = name; this.personNr = personNr; } }

Permalänk
Hedersmedlem

Har du testat att göra en sökning på personnummer eller namn som inte finns?
Eller till och med, hur ser outputen ut när du söker efter en person?

Permalänk

@Shimonu: Ojdå! Bättre?

System.Console.Write("Ange personens personnummer: "); long sökning = System.Convert.ToInt64(System.Console.ReadLine()); foreach (var element in Databas) { if (sökning == element.personNr) { int index = (Databas.IndexOf(element) + 1); System.Console.WriteLine(index + ": " + element.name + " med person nr " + element.personNr); } } if (Databas.Count == 0) { System.Console.WriteLine("Databasen är tom"); } else { System.Console.WriteLine("Personen är inte inlaggd"); } break;

Permalänk

Några bonus uppgifter.

Söker man på "peter" ska det matcha en "Peter".

Vad händer om man skriver in fel värden när matar in. T.ex. "abc" när man söker efter personnummer?

Permalänk

Koden börjar bli riktigt lång och stökig nu märker jag. Måste jobba på att implementera koden i olika metodrar istället för att ha allt i main metoden. Nåväl. Detta är vad jag lyckades knappa ihop efter parametrarna ni gav mig.

class Program { static void Main(string[] args) { List<PersonDatabas> Databas = new List<PersonDatabas>(); long sökning = 0; while (true) { System.Console.WriteLine("===Person databas==="); System.Console.WriteLine("Ange svarsallternativ"); System.Console.WriteLine("________________________"); System.Console.WriteLine("1: Lägg till person i databasen"); System.Console.WriteLine("2: Sök på ett personnummer"); System.Console.WriteLine("3: Sök på person med namn"); System.Console.WriteLine("4: SKriv ut alla medlemmar i databasen"); int svar = 0; try { svar = System.Convert.ToInt32(System.Console.ReadLine()); } catch { System.Console.WriteLine("Ange enbart siffror enligt menyvalen."); } switch (svar) { case 1: { System.Console.Write("Ange personens namn: "); string namn = System.Console.ReadLine(); System.Console.Write("Ange personens personnummer: "); long personNr = System.Convert.ToInt64(System.Console.ReadLine()); PersonDatabas personDatabas = new PersonDatabas(namn, personNr); Databas.Add(personDatabas); System.Console.WriteLine("Du lade till: " + namn + " med personnummer " + personNr); break; } case 2: { System.Console.Write("Ange personens personnummer: "); try { sökning = System.Convert.ToInt64(System.Console.ReadLine()); } catch { System.Console.WriteLine("Ange endast siffror i personnummer"); continue; } foreach (var element in Databas) { if (sökning == element.personNr) { int index = (Databas.IndexOf(element) + 1); System.Console.WriteLine(index + ": " + element.name + " med person nr " + element.personNr); } else if (sökning != element.personNr) { System.Console.WriteLine("Personen är inte inlaggd"); } break; } if (Databas.Count == 0) { System.Console.WriteLine("Databasen är tom"); } break; } case 3: { System.Console.Write("Ange personens namn: "); string namn = System.Console.ReadLine(); foreach (var person in Databas) { if (string.Compare(namn, person.name, System.StringComparison.OrdinalIgnoreCase) == 0) { int index = (Databas.IndexOf(person) + 1); System.Console.WriteLine(index + ":" + person.name + " med person nr " + person.personNr); } else if (string.Compare(namn, person.name) == -1 || string.Compare(namn, person.name) == 1) { System.Console.WriteLine("Personen är inte inlaggd"); } } if (Databas.Count == 0) { System.Console.WriteLine("Databasen är tom"); } break; } case 4: { System.Console.WriteLine("Skriver ut alla medlemmar: "); int index = 0; foreach (var element in Databas) { index++; System.Console.WriteLine(index + ": " + element.name + ": " + element.personNr); } break; } } } } } class PersonDatabas { public string name; public long personNr; public PersonDatabas(string name, long personNr) { this.name = name; this.personNr = personNr; } }

Permalänk

@Zero_Digits: Kanon!

Precis som du säger, snart borde du flytta till metoder och börja använda namespace för att få bort alla System.*

Att kolla: Jämför TryParse från string, int och long och jämför med try/catch.

En annan bonus. För att se skillnad mot foreach(..) vs for(;;)
Vad händer om du matchar 2 personer med samma namn när du söker på namn när det gäller vilket index du får ut?

Permalänk
Medlem

Du bör lägga till en "default:" sist i din switch sats och tala om att det alternativet inte är aktivt, om man skriver in ett tal annat än de i menyn.

Dessutom, om man ska hårddra det så bör alla funktioner som manipulerar listan ligga som statiska metoder i klassen och som tar listan som ett "ref" indata argument.

Visa signatur

"Om man arbetar tillräckligt länge med att förbättra ett föremål går det sönder. "

Hjälp oss göra världen lite snällare! www.upphittat.nu

Permalänk

@Zero_Digits:

Såg nu även att detta

else if (sökning != element.personNr) { System.Console.WriteLine("Personen är inte inlaggd"); } break;

verkar udda. Vad händer vid break?

[edit]
Och,

else if (string.Compare(namn, person.name) == -1 || string.Compare(namn, person.name) == 1)

varför en else if istället för en else.. antingen så hittar du .. eller inte?

[edit2]
Samma sak gäller

else if (sökning != element.personNr)

När skrivs felmeddelandet ut? Per sökning eller per element i listan?

Permalänk
Medlem

@Zero_Digits: Är PersonDatabas verkligen ett lämpligt namn på klassen? Fundera på vad klassen egentligen representerar.

Sen behöver du fortfarande jobba lite med case 2, som koden ser ut nu kommer den skriva ut "Personen är inte inlagd" för varje element som inte matchar. Använd t.ex. en bool för att hålla reda på om någon person hittats istället, och om du hittar en träff i listan så kan du även avsluta loopen direkt med break. Det kan också vara lämpligt att kolla om databasen är tom före sökningen istället för efter.

Dina if-satser är lite tillkrånglade också. Detta:

if (sökning == element.personNr) { .. } else if (sökning != element.personNr) { ... }

kan istället skrivas som:

if (sökning == element.personNr) { .. } else { ... }

För om sökning == element.personNr inte är sant så måste ju sökning != element.personNr vara sant. D.v.s. det finns ingen mening med att kolla det i en else if. Samma gäller i case 3 där du jämför namn. Fast du bör som sagt ändå inte ha en else-sats i detta fall, eftersom du inte vill göra en utskrift för varje element som inte matchar.

På alla ställen där du behöver ett elements index så skulle det vara bättre att bara använda en vanlig for-loop istället för foreach. Att loopa igenom listan och använda IndexOf som du gör i t.ex. case 2 är väldigt oeffektivt, eftersom IndexOf också kommer loopa igenom listan för att hitta indexet på elementet.

Permalänk

[edit] fel användare.
@ZecretW: offtopic. Då @Zero_Digits aldrig skapar en ny lista som pekar på den gamla (pointern) så kan vi ignorera ref alt. out.

Permalänk
Medlem

@zoomster2: Nu fattade jag inte, snälla förklara, det lät intressant.

Normalt skapas ju en "enkelriktad" koppling till indata i en metod om man inte använder "ref". Är det inte samma med List objekt?

Visa signatur

"Om man arbetar tillräckligt länge med att förbättra ett föremål går det sönder. "

Hjälp oss göra världen lite snällare! www.upphittat.nu

Permalänk
Medlem

Jag skummade lite snabbt och fastnade först och främst på stavfel, och många dessutom Det är inte bara koden som skall göra rätt, utan det skall se snyggt och prydligt ut också! Har du inte stavningskontroll i din utvecklingsmiljö?

Utöver detta så har du fått många bra förslag i tråden!
Uppdelning till funktioner skulle nog behövas för att bevara läsligheten.

Permalänk
Medlem

Flera bra synpunkter i tråden, men har inte sett någon som kommenterat valet av "long" för personnummer? Int/heltal överlag är väl rätt dumt för ett personnummer... Tänkte då ett tal inte kan börja på 0, vilket innebär att samtliga födda mellan 00 och 09 inte får vara med

Personnummer sparas bättre i en sträng (string) och kan då även formateras lättare med fyra sista siffrorna m.m.
Bara en liten tanke för bättre kod

Visa signatur

NZXT H510 Flow MSI B450 Tomahawk MAX
AMD Ryzen 5800X3D RX 7900XTX Kingston Fury 64GB
LG C2 42" 4K@120Hz AOC Q27G2U 1440P@144Hz

Permalänk

@ZecretW:
[offtopic] Value type vs reference type

namespace Zoomster.Example303 { using System; using System.Collections.Generic; internal static class Program { private static void Main() { var pets = new List<string> {"dog","cat"}; // change 'content' because is a reference type AddPet(pets, "hamster"); PrintPets(pets); // change 'content' because is a reference type but allow pets = new List<string>() if needed AddPetWithRef(ref pets, "fish"); PrintPets(pets); var value = "abc"; // not changing value because it is a value type ChangeValue(value, "cba"); Console.WriteLine(value); // changing the pointer to the string ChangeValueWithRef(ref value, "aaa"); Console.WriteLine(value); } private static void AddPet(ICollection<string> pets, string pet) { pets.Add(pet); } private static void AddPetWithRef(ref List<string> pets, string pet) { pets.Add(pet); } private static void PrintPets(IEnumerable<string> pets) { foreach (var pet in pets) { Console.WriteLine(pet); } } private static void ChangeValue(string value, string newValue) { value = newValue; } private static void ChangeValueWithRef(ref string value, string newValue) { value = newValue; } } }

Permalänk
Skrivet av zoomster2:

@Zero_Digits: Kanon!

En annan bonus. För att se skillnad mot foreach(..) vs for(;;)
Vad händer om du matchar 2 personer med samma namn när du söker på namn när det gäller vilket index du får ut?

Hejsan, tack för feedbacken, har vart på 1 vecka semester men är nu tillbaka vid skolbänken. Varför jag använder foreach är på grund av att jag har en lista som jag utgår ifrån och att efter vad jag har lärt mig är foreach smidigare med listor med tanke på att du aldrig kan komma out of bounds. Söker jag på ett namn som matchar med ett annat så får jag ut båda namnen, följt av vardera personnummer och med korrekt index placering enligt vad jag har testat mig till. vad tänkte du?

Skrivet av ZecretW:

Du bör lägga till en "default:" sist i din switch sats och tala om att det alternativet inte är aktivt, om man skriver in ett tal annat än de i menyn.

Dessutom, om man ska hårddra det så bör alla funktioner som manipulerar listan ligga som statiska metoder i klassen och som tar listan som ett "ref" indata argument.

Detta får jag kolla närmare på, tack för tipset

Permalänk
Skrivet av Dalton Sleeper:

Jag skummade lite snabbt och fastnade först och främst på stavfel, och många dessutom Det är inte bara koden som skall göra rätt, utan det skall se snyggt och prydligt ut också! Har du inte stavningskontroll i din utvecklingsmiljö?

Ingen stavningskontroll aktiverad verkar det som. Men jag ska kika på detta. Brukar alltid slarva till stavningen när jag bara leker runt i koden. Detta är såklart en dålig vana, tur att detta inte är en inlämningsuppgift Tack för feedback!

Permalänk
Skrivet av Pamudas:

Flera bra synpunkter i tråden, men har inte sett någon som kommenterat valet av "long" för personnummer? Int/heltal överlag är väl rätt dumt för ett personnummer... Tänkte då ett tal inte kan börja på 0, vilket innebär att samtliga födda mellan 00 och 09 inte får vara med

Personnummer sparas bättre i en sträng (string) och kan då även formateras lättare med fyra sista siffrorna m.m.
Bara en liten tanke för bättre kod

Ahh spännande. Googlade runt lite och antar att det är "TryParse" man får använda då för att se att strängen innehåller siffror. Vad jag inte kan klura ut just nu är hur man gör ett undantag för "-" om användaren skulle mata in till exempel: 19750416-7587. Har du några tips på bra läsningar om detta?

Permalänk
Medlem
Skrivet av Zero_Digits:

Ahh spännande. Googlade runt lite och antar att det är "TryParse" man får använda då för att se att strängen innehåller siffror. Vad jag inte kan klura ut just nu är hur man gör ett undantag för "-" om användaren skulle mata in till exempel: 19750416-7587. Har du några tips på bra läsningar om detta?

RegEx är ett perfekt användningsområde för detta. Du kollar då om de första 8 tecknen är siffror, som eventuellt följs av en skiljetecken och sedan fyra siffror.
Typ "^[12]{1}[90]{1}[0-9]{6}-{0,1}[0-9]{4}$"
Det betyder alltså: siffran 1 eller 2 måste förekomma EN gång. Efteråt måste antingen 9 eller 0 förekomma EN gång. Efter detta måste 0 till och med 9 förekomma exakt 6 gånger följt av ett valfritt skiljetecken och sedan 4 siffror från 0 till och med 9.
Notera att jag inte är någon expert på RegEx och det är eventuellt överkurs just nu
Skrev även på mobilen så ursäkta indentering och eventuellt felstavning

var reg = new Regex(@^[12]{1}[90]{1}[0-9]{6}-{0,1}[0-9]{4}$); var input = "19750416-7587"; // 197504167587 bör gå igenom. var matches = reg.Matches(input); if(matches.Count > 0){ Console.WriteLine(matches[0].Value); }else{ Console.WriteLine("fel input"); }

Lite snabbt sådär som det kan användas.

Visa signatur

NZXT H510 Flow MSI B450 Tomahawk MAX
AMD Ryzen 5800X3D RX 7900XTX Kingston Fury 64GB
LG C2 42" 4K@120Hz AOC Q27G2U 1440P@144Hz

Permalänk
Medlem

Som @Pamudas skriver, "RegEx" (Regular Expressions, reguljära uttryck) är väldigt kraftfulla för att matcha format, extrahera information ur ostrukturerad text, validering m.m.

För att göra livet lite lättare kan jag rekommendera att använda ett hjälpmedel för att skapa sin regex. Jag brukar köra https://regex101.com/. Man kan enkelt testa ett uttryck mot valfri text och även se hur det tolkas i en trädvy.

Det går att förenkla (eller iallafall förkorta) personnummer-uttrycket ovan något, testa i verktyget för att få en detaljerad förklaring!

^(19|20)?\d{6}[-+]?\d{4}$

Skickades från m.sweclockers.com

Permalänk

Tack för tipset! Jag tror dock detta är som ni säger lite överkurs just nu. Jag körde en if sats som bara tillät 10 eller 12 siffror istället. Får duga sålänge. Men kommer absolut komma tillbaka till detta efter mina studier

Förövrigt har jag en till fråga angående att jämföra och söka i strängar. Jag har i min kod en funktion som ska tillåta användaren att söka efter namn. Den simplaste formen är ju att köra en "==" mellan användarens inmatning och det namnet i listan man vill åt. Men då måste namnen vara identiska. Nu har jag hittat metoden "String.Contains()" som söker i en sträng efter den angivna inmatningen. Problemet med String.Contains() är att den är case sensetive. Det blir ganska dumt när man ska söka på carl och det inte funkar för att det ska vara Carl... Min fråga är då ifall det finns någon smidig lösning på detta? String.compare() & String.equals() har jag provat men dom funkar också bara om namnet är exakt samma, alltså det räcker inte bara med förnamn eller efternamn som det hade gjort i string.Contains()

Permalänk
Skrivet av Zero_Digits:

Problemet med String.Contains() är att den är case sensetive. Det blir ganska dumt när man ska söka på carl och det inte funkar för att det ska vara Carl... Min fråga är då ifall det finns någon smidig lösning på detta?

Att String.Contains() är case sensitive löser du genom att använda dig av String.ToLower(). Tänk dock på att använda denna både på den inmatade strängen och strängarna du jämför med.

Förutom särskrivningarna och nån felstavning som andra redan påpekat skulle jag rekommendera dig att skippa svenskan i koden och låta alla variabelnamn och metoder ha engelska namn.

Permalänk
Medlem
Skrivet av Zero_Digits:

Tack för tipset! Jag tror dock detta är som ni säger lite överkurs just nu. Jag körde en if sats som bara tillät 10 eller 12 siffror istället. Får duga sålänge. Men kommer absolut komma tillbaka till detta efter mina studier

Förövrigt har jag en till fråga angående att jämföra och söka i strängar. Jag har i min kod en funktion som ska tillåta användaren att söka efter namn. Den simplaste formen är ju att köra en "==" mellan användarens inmatning och det namnet i listan man vill åt. Men då måste namnen vara identiska. Nu har jag hittat metoden "String.Contains()" som söker i en sträng efter den angivna inmatningen. Problemet med String.Contains() är att den är case sensetive. Det blir ganska dumt när man ska söka på carl och det inte funkar för att det ska vara Carl... Min fråga är då ifall det finns någon smidig lösning på detta? String.compare() & String.equals() har jag provat men dom funkar också bara om namnet är exakt samma, alltså det räcker inte bara med förnamn eller efternamn som det hade gjort i string.Contains()

Strängar vill du inte jämföra med ==, detta för att en sträng är ett objekt.
Ska du jämföra strängar gör du det med equals() metoden. Dock är även denna case-sensitive.
Finns dock en variant för just vad du försöker göra, equalsIgnoreCase.

Ang. Ditt contains problem finns det lite olika sätt att lösa det på.
https://stackoverflow.com/questions/14018478/string-contains-...
Här hittar du lite inspiration.

Med risk för att låta otrevlig så rekommenderar jag att du vässar på dina google-skills, det är det bästa verktyget en programmerare har.
Är sannolikt att någon annan stött på samma problem med en lösning, Google kommer att bli din bästa vän.

Lite övriga synpunkter är på din personklass.
Att ha fälten publika är generellt dålig praxis, du vill förhindra att de ändras hur som helst.
Sätt de istället som privata och använd getters och setters.
T.ex:

private String namn; public string getNamn() { return this.namn; } public void setString(String namn) { this.namn = namn; }

Lycka till med studierna!

Visa signatur

Stationär: Core i9 13900k | Asus X790 ROG Strix Gaming-F | 32GB DDR5 | GeForce RTX 5090 | Lian Li PC-O11 dynamic evo
Laptop: Macbook Pro 16" | Apple M1 Max

Permalänk
Medlem

@Zero_Digits: Se också Microsofts dokumentation för många exempel på hur man kan jämföra strängar.

@MaxieTheHatter: Du tänker nog på fel språk, TS använder C# och inte Java.

Permalänk
Medlem

@perost:
Se där, kunde ha svurit att det var Java när jag läste på telefonen.
Ser nu att det är System.console och inte System.out

My bad!

Ska väl erkänna att jag inte har kodat i C#, men syntaxen är såpass lik att jag inte ens tänkte mig för innan jag antog att det var java

Visa signatur

Stationär: Core i9 13900k | Asus X790 ROG Strix Gaming-F | 32GB DDR5 | GeForce RTX 5090 | Lian Li PC-O11 dynamic evo
Laptop: Macbook Pro 16" | Apple M1 Max